From: Tom Hughes <tom@compton.nu>
Date: Wed, 29 Jan 2025 18:41:43 +0000 (+0000)
Subject: Merge remote-tracking branch 'upstream/pull/5575'
X-Git-Tag: live~132
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/68481fe7753432ffd3051ede15f7518b1c0d73c9?hp=b00dc40ff064f35b878c5416ae0cd96f7384097d

Merge remote-tracking branch 'upstream/pull/5575'
---

diff --git a/app/assets/javascripts/index/layers/data.js b/app/assets/javascripts/index/layers/data.js
index 3cc8f6135..dc522af41 100644
--- a/app/assets/javascripts/index/layers/data.js
+++ b/app/assets/javascripts/index/layers/data.js
@@ -73,10 +73,8 @@ OSM.initializeDataLayer = function (map) {
             $("<button type='button' class='btn-close'>")
               .attr("aria-label", I18n.t("javascripts.close"))
               .click(close))),
-        $("<div>").append(
-          $("<div class='d-flex'>").append(
-            $("<p class='alert alert-warning'>")
-              .text(I18n.t("browse.start_rjs.feature_error", { message: message }))))));
+        $("<p class='alert alert-warning'>")
+          .text(I18n.t("browse.start_rjs.feature_error", { message: message }))));
   }
 
   function getData() {
diff --git a/app/assets/javascripts/index/layers/notes.js b/app/assets/javascripts/index/layers/notes.js
index c0ba35d7e..24bf969b3 100644
--- a/app/assets/javascripts/index/layers/notes.js
+++ b/app/assets/javascripts/index/layers/notes.js
@@ -1,6 +1,7 @@
 OSM.initializeNotesLayer = function (map) {
-  var noteLayer = map.noteLayer,
-      notes = {};
+  let noteLoader;
+  const noteLayer = map.noteLayer;
+  let notes = {};
 
   var noteIcons = {
     "new": L.icon({
@@ -25,6 +26,8 @@ OSM.initializeNotesLayer = function (map) {
     map.on("moveend", loadNotes);
     map.fire("overlayadd", { layer: noteLayer });
   }).on("remove", () => {
+    if (noteLoader) noteLoader.abort();
+    noteLoader = null;
     map.off("moveend", loadNotes);
     noteLayer.clearLayers();
     notes = {};
@@ -40,9 +43,16 @@ OSM.initializeNotesLayer = function (map) {
     if (marker) {
       marker.setIcon(noteIcons[feature.properties.status]);
     } else {
+      let title;
+      const description = feature.properties.comments[0];
+
+      if (description?.action === "opened") {
+        title = description.text;
+      }
+
       marker = L.marker(feature.geometry.coordinates.reverse(), {
         icon: noteIcons[feature.properties.status],
-        title: feature.properties.comments[0].text,
+        title,
         opacity: 0.8,
         interactive: true
       });
@@ -56,8 +66,6 @@ OSM.initializeNotesLayer = function (map) {
     return marker.id;
   };
 
-  var noteLoader;
-
   function loadNotes() {
     var bounds = map.getBounds();
     var size = bounds.getSize();
diff --git a/app/assets/javascripts/leaflet.map.js b/app/assets/javascripts/leaflet.map.js
index bdc8532d1..10ce94715 100644
--- a/app/assets/javascripts/leaflet.map.js
+++ b/app/assets/javascripts/leaflet.map.js
@@ -115,19 +115,18 @@ L.OSM.Map = L.Map.extend({
   },
 
   updateLayers: function (layerParam) {
-    var layers = layerParam || "M";
+    const oldBaseLayer = this.getMapBaseLayer();
+    let newBaseLayer;
 
-    for (let i = this.baseLayers.length - 1; i >= 0; i--) {
-      if (layers.indexOf(this.baseLayers[i].options.code) === -1) {
-        this.removeLayer(this.baseLayers[i]);
+    for (const layer of this.baseLayers) {
+      if (!newBaseLayer || layerParam.includes(layer.options.code)) {
+        newBaseLayer = layer;
       }
     }
 
-    for (let i = this.baseLayers.length - 1; i >= 0; i--) {
-      if (layers.indexOf(this.baseLayers[i].options.code) >= 0 || i === 0) {
-        this.addLayer(this.baseLayers[i]);
-        return;
-      }
+    if (newBaseLayer !== oldBaseLayer) {
+      if (oldBaseLayer) this.removeLayer(oldBaseLayer);
+      if (newBaseLayer) this.addLayer(newBaseLayer);
     }
   },
 
diff --git a/app/assets/stylesheets/common.scss b/app/assets/stylesheets/common.scss
index 098370689..92b9f7240 100644
--- a/app/assets/stylesheets/common.scss
+++ b/app/assets/stylesheets/common.scss
@@ -816,6 +816,14 @@ tr.turn {
   }
 }
 
+/* Rules for the issues page */
+
+.issues.issues-index {
+  td.reporter_users {
+    max-width: 5rem;
+  }
+}
+
 /* Rules for the account confirmation page */
 
 .accounts-terms-show {
diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index 5cedb3c70..298f7c374 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -44,6 +44,15 @@ class IssuesController < ApplicationController
     end
 
     @issues, @newer_issues_id, @older_issues_id = get_page_items(@issues, :limit => @params[:limit])
+
+    @unique_reporters = @issues.each_with_object({}) do |issue, reporters|
+      user_ids = issue.reports.order(:created_at => :desc).pluck(:user_id).uniq
+      reporters[issue.id] = {
+        :count => user_ids.size,
+        :users => User.in_order_of(:id, user_ids.first(3))
+      }
+    end
+
     render :partial => "page" if turbo_frame_request_id == "pagination"
   end
 
diff --git a/app/views/issues/_page.html.erb b/app/views/issues/_page.html.erb
index f625f0e04..6b291e58d 100644
--- a/app/views/issues/_page.html.erb
+++ b/app/views/issues/_page.html.erb
@@ -13,6 +13,7 @@
           <th><%= t ".reports" %></th>
           <th><%= t ".reported_item" %></th>
           <th><%= t ".reported_user" %></th>
+          <th class="reporter_users"><%= t ".reporter_users" %></th>
           <th><%= t ".last_updated" %></th>
         </tr>
       </thead>
@@ -23,6 +24,14 @@
             <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 class="reporter_users text-truncate">
+              <% @unique_reporters[issue.id][:users].each do |reporter| %>
+                <%= link_to reporter.display_name, reporter, :class => "d-block text-truncate", :title => reporter.display_name %>
+              <% end %>
+              <% if @unique_reporters[issue.id][:count] > 3 %>
+                <p class="m-0"><%= t ".more_reporters", :count => @unique_reporters[issue.id][:count] - 3 %></p>
+              <% end %>
+            </td>
             <td>
               <% if issue.user_updated %>
                 <%= t ".last_updated_time_ago_user_html", :user => link_to(issue.user_updated.display_name, issue.user_updated),
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 38b810d1f..172c66182 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -1512,9 +1512,11 @@ en:
       reports: Reports
       last_updated: Last Updated
       last_updated_time_ago_user_html: "%{time_ago} by %{user}"
+      reporter_users: Reporter Users
       reports_count:
         one: "%{count} Report"
         other: "%{count} Reports"
+      more_reporters: "and %{count} more"
       reported_item: Reported Item
       states:
         ignored: Ignored
diff --git a/test/system/issues_test.rb b/test/system/issues_test.rb
index 327b888bc..a961ea80f 100644
--- a/test/system/issues_test.rb
+++ b/test/system/issues_test.rb
@@ -204,4 +204,48 @@ class IssuesTest < ApplicationSystemTestCase
       assert_no_content(/extra_#{n}[^\d]/i)
     end
   end
+
+  def test_single_issue_reporters
+    sign_in_as(create(:moderator_user))
+    issue = create(:issue, :assigned_role => "moderator")
+    issue.reports << create(:report, :user => create(:user, :display_name => "Test Name"))
+
+    visit issues_path
+    assert_content issue.reported_user.display_name
+    assert_content issue.reports.first.user.display_name
+  end
+
+  def test_multiple_issue_reporters
+    sign_in_as(create(:moderator_user))
+    issue = create(:issue, :assigned_role => "moderator")
+
+    create_list(:report, 5, :issue => issue)
+
+    visit issues_path
+    0.upto(1).each do |n|
+      assert_no_content issue.reports[n].user.display_name
+    end
+    2.upto(4).each do |n|
+      assert_content issue.reports[n].user.display_name
+    end
+  end
+
+  def test_ordering_issue_reporters
+    sign_in_as(create(:moderator_user))
+    issue = create(:issue, :assigned_role => "moderator")
+
+    create_list(:report, 5, :issue => issue)
+
+    4.downto(0).each do |n|
+      issue.reports << create(:report, :user => issue.reports[n].user)
+    end
+
+    visit issues_path
+    0.upto(2).each do |n|
+      assert_content issue.reports[n].user.display_name
+    end
+    3.upto(4).each do |n|
+      assert_no_content issue.reports[n].user.display_name
+    end
+  end
 end
diff --git a/test/system/note_layer_test.rb b/test/system/note_layer_test.rb
new file mode 100644
index 000000000..792c3416d
--- /dev/null
+++ b/test/system/note_layer_test.rb
@@ -0,0 +1,40 @@
+require "application_system_test_case"
+
+class NoteLayerTest < ApplicationSystemTestCase
+  test "note marker should have description as a title" do
+    position = (1.1 * GeoRecord::SCALE).to_i
+    create(:note, :latitude => position, :longitude => position) do |note|
+      create(:note_comment, :note => note, :body => "Note description")
+    end
+
+    visit root_path(:anchor => "map=18/1.1/1.1&layers=N")
+    all "img.leaflet-marker-icon", :count => 1 do |marker|
+      assert_equal "Note description", marker["title"]
+    end
+  end
+
+  test "note marker should not have a title if the note has no visible description" do
+    position = (1.1 * GeoRecord::SCALE).to_i
+    create(:note, :latitude => position, :longitude => position) do |note|
+      create(:note_comment, :note => note, :body => "Note description is hidden", :visible => false)
+      create(:note_comment, :note => note, :body => "Note comment visible", :event => "commented")
+    end
+
+    visit root_path(:anchor => "map=18/1.1/1.1&layers=N")
+    all "img.leaflet-marker-icon", :count => 1 do |marker|
+      assert_equal "", marker["title"]
+    end
+  end
+
+  test "note marker should not have a title if the note has no visible description and comments" do
+    position = (1.1 * GeoRecord::SCALE).to_i
+    create(:note, :latitude => position, :longitude => position) do |note|
+      create(:note_comment, :note => note, :body => "Note description is hidden", :visible => false)
+    end
+
+    visit root_path(:anchor => "map=18/1.1/1.1&layers=N")
+    all "img.leaflet-marker-icon", :count => 1 do |marker|
+      assert_equal "", marker["title"]
+    end
+  end
+end