[openstreetmap/openstreetmap-website] Fixed "Add note and comment counts to the user profile" described in #1643 (PR #4875)

[Tom Hughes]

@tomhughes requested changes on this pull request.

I don't think we need to delay this - the counts should reflect what the links currently do and if we change what they do then we can consider how to deal with the counts then.

Other than the specific comments I have added please adjust the commit message as per my comments on #4881.

> @@ -0,0 +1,6 @@
+class AddNotesAndDiaryCommentsCounterCaches < ActiveRecord::Migration[7.1]
+  def change
+    add_column :users, :diary_comments_count, :integer, :default => 0
+    add_column :users, :note_comments_count, :integer, :default => 0

These need to call `reset_counters` for any user that already has comments or the counts won't be correct - see `db/migrate/20121005195010_add_diary_entry_counter_caches.rb` for a way to to it reasonably efficiently.

There are less than 10k users with diary comments so that is probably fine but there are nearly half a million with not comments which may prove to be a problem in terms of time take to run the migration and populate the counters to we might have to do it with a rake task rather than in the migration.

> @@ -11,21 +11,23 @@
           <ul class='clearfix'>
             <li>
               <%= link_to t(".my edits"), :controller => "changesets", :action => "index", :display_name => current_user.display_name %>
-              <span class='badge count-number'><%= number_with_delimiter(current_user.changesets.size) %></span>
+              <span class='badge count-number'><%= number_with_delimiter(current_user.changesets_count) %></span>

Doesn't `changesets.size` already delegate to the counter? If it doesn't then this isn't working at the moment, which isn't good, and if it does why the change - what is the canonical rails way to do this?

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

Message ID: <openstreetmap/openstreetmap-website/pull/4875/review/2102746814 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20240606/20a7aada/attachment.htm>