[openstreetmap/openstreetmap-website] Add rate limiting for changeset comments (PR #4202)

[Andy Allan]

@gravitystorm commented on this pull request.



> @@ -395,6 +395,19 @@ def max_friends_per_hour
     max_friends.clamp(0, Settings.max_friends_per_hour)
   end
 
+  def max_changeset_comments_per_hour
+    if moderator?
+      36000
+    else
+      previous_comments = changeset_comments.limit(200).count
+      active_reports = issues.with_status(:open).sum(:reports_count)
+      max_comments = previous_comments / 200.0 * Settings.max_changeset_comments_per_hour
+      max_comments = max_comments.floor.clamp(Settings.min_changeset_comments_per_hour, Settings.max_changeset_comments_per_hour)
+      max_comments /= 2**active_reports
+      max_comments.floor.clamp(1, Settings.max_changeset_comments_per_hour)
+    end

That's an interesting algorithm! I do like relying on previous activity, rather than account age, so I think we could rework the existing limits along these lines too. And using reports as the main downward modifier is smart.

I suspect there might be non-moderator power users caught out by this, since a disagreement about reverts could lead to one person making ~10 reports and therefore severely rate-limiting the power user. Perhaps defending against that would be too much complexity for now.

Is there a naming issue here? The minimum number of comments per hour is actually 1, not `min_changeset_comments_per_hour`. I think an admin seeing the min and max settings might not realise that the min is more of a "default" or starter value, that can go both up and also go down lower.

I also think we need to move these rate limit calculations out of the user model, but I'm not yet sure where they should go. The model is already huge, and it almost feels like these are controller-level decisions rather than some property of the model in isolation.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4202#pullrequestreview-1602359627
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/4202/review/1602359627 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20230830/d443bbbf/attachment.htm>