From: Tom Hughes Date: Fri, 7 Mar 2025 17:53:32 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/5767' X-Git-Tag: live~5 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/284a142c51dcc7ce4566664ec4c0ae3912906137?hp=be4644db99f27402a337c3adc6b31c96a2a2c914 Merge remote-tracking branch 'upstream/pull/5767' --- diff --git a/app/controllers/issues/reporters_controller.rb b/app/controllers/issues/reporters_controller.rb new file mode 100644 index 000000000..b6513f443 --- /dev/null +++ b/app/controllers/issues/reporters_controller.rb @@ -0,0 +1,27 @@ +module Issues + class ReportersController < ApplicationController + layout "site" + + before_action :authorize_web + before_action :set_locale + before_action :check_database_readable + + authorize_resource :issue + + def index + @issue = Issue.visible_to(current_user).find(params[:issue_id]) + + user_ids = @issue.reports.order(:created_at => :desc).pluck(:user_id).uniq + @unique_reporters = { + @issue.id => { + :count => user_ids.size, + :users => User.in_order_of(:id, user_ids) + } + } + + render :partial => "reporters", :locals => { :issue => @issue } if turbo_frame_request? + rescue ActiveRecord::RecordNotFound + redirect_to :controller => "/errors", :action => "not_found" + end + end +end diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 298f7c374..640317ce7 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -45,11 +45,12 @@ class IssuesController < ApplicationController @issues, @newer_issues_id, @older_issues_id = get_page_items(@issues, :limit => @params[:limit]) + @unique_reporters_limit = 3 @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)) + :users => User.in_order_of(:id, user_ids.first(@unique_reporters_limit)) } end diff --git a/app/views/issues/_page.html.erb b/app/views/issues/_page.html.erb index 9681704a1..225517c9d 100644 --- a/app/views/issues/_page.html.erb +++ b/app/views/issues/_page.html.erb @@ -25,12 +25,7 @@ <%= link_to reportable_title(issue.reportable), reportable_url(issue.reportable) %> <%= link_to issue.reported_user.display_name, issue.reported_user if issue.reported_user %> - <% @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 %> -

<%= t ".more_reporters", :count => @unique_reporters[issue.id][:count] - 3 %>

- <% end %> + <%= render :partial => "issues/reporters/reporters", :locals => { :issue => issue } %> <% if issue.user_updated %> diff --git a/app/views/issues/reporters/_reporters.html.erb b/app/views/issues/reporters/_reporters.html.erb new file mode 100644 index 000000000..d426cad33 --- /dev/null +++ b/app/views/issues/reporters/_reporters.html.erb @@ -0,0 +1,13 @@ +<%= turbo_frame_tag "#{dom_id(issue)}_reporters", :data => { :turbo => false } do %> + <% @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_limit && @unique_reporters[issue.id][:count] > @unique_reporters_limit %> +

+ <%= link_to t(".more_reporters", :count => @unique_reporters[issue.id][:count] - @unique_reporters_limit), + issue_reporters_path(issue), + :class => "link-body-emphasis", + :data => { :turbo => true } %> +

+ <% end %> +<% end %> diff --git a/app/views/issues/reporters/index.html.erb b/app/views/issues/reporters/index.html.erb new file mode 100644 index 000000000..88b33946e --- /dev/null +++ b/app/views/issues/reporters/index.html.erb @@ -0,0 +1,5 @@ +<% content_for :heading do %> +

<%= t ".title", :issue_id => @issue.id %>

+<% end %> + +<%= render :partial => "reporters", :locals => { :issue => @issue } %> diff --git a/config/locales/en.yml b/config/locales/en.yml index fe1fdbe52..5cfb56062 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1538,7 +1538,6 @@ en: reports_count: one: "%{count} Report" other: "%{count} Reports" - more_reporters: "and %{count} more" reported_item: Reported Item states: ignored: Ignored @@ -1581,6 +1580,11 @@ en: reportable_title: diary_comment: "%{entry_title}, comment #%{comment_id}" note: "Note #%{note_id}" + reporters: + index: + title: "Issue #%{issue_id} Reporters" + reporters: + more_reporters: "and %{count} more" issue_comments: create: comment_created: Your comment was successfully created diff --git a/config/routes.rb b/config/routes.rb index 8d42db69b..ec78f6ffd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -375,6 +375,7 @@ OpenStreetMap::Application.routes.draw do # issues and reports resources :issues do resources :comments, :controller => :issue_comments + resources :reporters, :module => :issues, :only => :index member do post "resolve" post "assign" diff --git a/test/controllers/issues/reporters_controller_test.rb b/test/controllers/issues/reporters_controller_test.rb new file mode 100644 index 000000000..06cfba3c1 --- /dev/null +++ b/test/controllers/issues/reporters_controller_test.rb @@ -0,0 +1,98 @@ +require "test_helper" + +module Issues + class DataControllerTest < ActionDispatch::IntegrationTest + ## + # test all routes which lead to this controller + def test_routes + assert_routing( + { :path => "/issues/1/reporters", :method => :get }, + { :controller => "issues/reporters", :action => "index", :issue_id => "1" } + ) + end + + def test_index_missing_issue_as_moderator + session_for(create(:moderator_user)) + get issue_reporters_path(999111) + + assert_redirected_to :controller => "/errors", :action => :not_found + end + + def test_index_missing_issue_as_administrator + session_for(create(:administrator_user)) + get issue_reporters_path(999111) + + assert_redirected_to :controller => "/errors", :action => :not_found + end + + def test_index_assigned_to_moderator_as_unauthorized + issue = create(:issue, :assigned_role => "moderator") + + get issue_reporters_path(issue) + + assert_redirected_to login_path(:referer => issue_reporters_path(issue)) + end + + def test_index_assigned_to_moderator_as_regular_user + issue = create(:issue, :assigned_role => "moderator") + + session_for(create(:user)) + get issue_reporters_path(issue) + + assert_redirected_to :controller => "/errors", :action => :forbidden + end + + def test_index_assigned_to_moderator_as_administrator + issue = create(:issue, :assigned_role => "moderator") + + session_for(create(:administrator_user)) + get issue_reporters_path(issue) + + assert_redirected_to :controller => "/errors", :action => :not_found + end + + def test_index_assigned_to_moderator_as_moderator + issue = create(:issue, :assigned_role => "moderator") + + session_for(create(:moderator_user)) + get issue_reporters_path(issue) + + assert_response :success + end + + def test_index_assigned_to_administrator_as_unauthorized + issue = create(:issue, :assigned_role => "administrator") + + get issue_reporters_path(issue) + + assert_redirected_to login_path(:referer => issue_reporters_path(issue)) + end + + def test_index_assigned_to_administrator_as_regular_user + issue = create(:issue, :assigned_role => "administrator") + + session_for(create(:user)) + get issue_reporters_path(issue) + + assert_redirected_to :controller => "/errors", :action => :forbidden + end + + def test_index_assigned_to_administrator_as_moderator + issue = create(:issue, :assigned_role => "administrator") + + session_for(create(:moderator_user)) + get issue_reporters_path(issue) + + assert_redirected_to :controller => "/errors", :action => :not_found + end + + def test_index_assigned_to_administrator_as_administrator + issue = create(:issue, :assigned_role => "administrator") + + session_for(create(:administrator_user)) + get issue_reporters_path(issue) + + assert_response :success + end + end +end