<p></p>
<p><b>@gravitystorm</b> commented on this pull request.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/4202#discussion_r1310026589">app/models/user.rb</a>:</p>
<pre style='color:#555'>> @@ -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
</pre>
<p dir="auto">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.</p>
<p dir="auto">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.</p>
<p dir="auto">Is there a naming issue here? The minimum number of comments per hour is actually 1, not <code class="notranslate">min_changeset_comments_per_hour</code>. 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.</p>
<p dir="auto">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.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/4202#pullrequestreview-1602359627">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLL3YWKVDTIPLCVZ6DTXX4GBDANCNFSM6AAAAAA363PXKA">unsubscribe</a>.<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/AAK2OLOLSSP6HHARG6BYH2LXX4GBDA5CNFSM6AAAAAA363PXKCWGG33NNVSW45C7OR4XAZNRKB2WY3CSMVYXKZLTORJGK5TJMV32UY3PNVWWK3TUL5UWJTS7QIIUW.gif" height="1" width="1" alt="" /><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span><openstreetmap/openstreetmap-website/pull/4202/review/1602359627</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/openstreetmap-website/pull/4202#pullrequestreview-1602359627",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/4202#pullrequestreview-1602359627",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>