[openstreetmap/openstreetmap-website] Show "reporting user" on "issues" screen (PR #4990)

[Tom Hughes]

@tomhughes requested changes on this pull request.

Although this is described as showing the "last three unique reporters" there isn't actually any ordering that I can see, and ordering on time is hard to combine with uniqification...

> @@ -60,6 +61,13 @@
           <td class="text-nowrap"><%= link_to t(".reports_count", :count => issue.reports_count), issue %></td>
           <td><%= link_to reportable_title(issue.reportable), reportable_url(issue.reportable) %></td>
           <td><%= link_to issue.reported_user.display_name, issue.reported_user if issue.reported_user %></td>
+          <td>
+            <% unique_reporters = issue.reports.uniq { |report| report.user.id } %>

This could probably be better done as `issue.reports.preload(:user).map(&:user).uniq` which would also get a list of users instead of a list of reports.

> @@ -60,6 +61,13 @@
           <td class="text-nowrap"><%= link_to t(".reports_count", :count => issue.reports_count), issue %></td>
           <td><%= link_to reportable_title(issue.reportable), reportable_url(issue.reportable) %></td>
           <td><%= link_to issue.reported_user.display_name, issue.reported_user if issue.reported_user %></td>
+          <td>
+            <% unique_reporters = issue.reports.uniq { |report| report.user.id } %>
+            <% unique_reporters[..2].each_with_index do |report, index| %>

This could be done more simply with something like:

```
safe_join(unique_reporters[..2].map { |u| link_to u.display_name, u }, ",")
```

That's assuming the previous edit to make the reporter list a list of users.

> @@ -60,6 +61,13 @@
           <td class="text-nowrap"><%= link_to t(".reports_count", :count => issue.reports_count), issue %></td>
           <td><%= link_to reportable_title(issue.reportable), reportable_url(issue.reportable) %></td>
           <td><%= link_to issue.reported_user.display_name, issue.reported_user if issue.reported_user %></td>
+          <td>
+            <% unique_reporters = issue.reports.uniq { |report| report.user.id } %>
+            <% unique_reporters[..2].each_with_index do |report, index| %>
+              <%= link_to(report.user.display_name, report.user) %><%= "," unless index == 2 || index == unique_reporters.size - 1 %>
+            <% end %>
+            <%= "..." if unique_reporters.size > 3 %>

Maybe instead of truncating the list here we should put the whole thing in but use CSS to limit the width and add an ellipsis? With a tooltip to show the follow value even?

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

Message ID: <openstreetmap/openstreetmap-website/pull/4990/review/2180762354 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20240716/3b740b4b/attachment-0001.htm>