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

app/controllers/diary_comments_controller.rb

@@ -1,7 +1,4 @@
 class DiaryCommentsController < ApplicationController
-  include UserMethods
-  include PaginationMethods
-
   layout "site"
 
   before_action :authorize_web
@@ -10,23 +7,9 @@ class DiaryCommentsController < ApplicationController
 
   authorize_resource
 
-  before_action :lookup_user, :only => :index
+  before_action :check_database_writable
-  before_action :check_database_writable, :only => [:create, :hide, :unhide]
 
-  allow_thirdparty_images :only => [:index, :create]
+  allow_thirdparty_images :only => :create
-
-  def index
-    @title = t ".title", :user => @user.display_name
-
-    comments = DiaryComment.where(:user => @user)
-    comments = comments.visible unless can? :unhide, DiaryComment
-
-    @params = params.permit(:display_name, :before, :after)
-
-    @comments, @newer_comments_id, @older_comments_id = get_page_items(comments, :includes => [:user])
-
-    render :partial => "page" if turbo_frame_request_id == "pagination"
-  end
 
   def create
     @entry = DiaryEntry.find(params[:id])

app/controllers/users/changeset_comments_controller.rb

@@ -0,0 +1,14 @@
+module Users
+  class ChangesetCommentsController < CommentsController
+    def index
+      @title = t ".title", :user => @user.display_name
+
+      comments = ChangesetComment.where(:author => @user)
+      comments = comments.visible unless current_user&.moderator?
+
+      @params = params.permit(:display_name, :before, :after)
+
+      @comments, @newer_comments_id, @older_comments_id = get_page_items(comments, :includes => [:author])
+    end
+  end
+end

app/controllers/users/comments_controller.rb

@@ -0,0 +1,18 @@
+module Users
+  class CommentsController < ApplicationController
+    include UserMethods
+    include PaginationMethods
+
+    layout "site"
+
+    before_action :authorize_web
+    before_action :set_locale
+    before_action :check_database_readable
+
+    authorize_resource
+
+    before_action :lookup_user
+
+    allow_thirdparty_images
+  end
+end

app/controllers/users/diary_comments_controller.rb

@@ -0,0 +1,16 @@
+module Users
+  class DiaryCommentsController < CommentsController
+    def index
+      @title = t ".title", :user => @user.display_name
+
+      comments = DiaryComment.where(:user => @user)
+      comments = comments.visible unless can? :unhide, DiaryComment
+
+      @params = params.permit(:display_name, :before, :after)
+
+      @comments, @newer_comments_id, @older_comments_id = get_page_items(comments, :includes => [:user])
+
+      render :partial => "page" if turbo_frame_request_id == "pagination"
+    end
+  end
+end

app/models/changeset_comment.rb

@@ -25,6 +25,8 @@ class ChangesetComment < ApplicationRecord
   belongs_to :changeset
   belongs_to :author, :class_name => "User"
 
+  scope :visible, -> { where(:visible => true) }
+
   validates :id, :uniqueness => true, :presence => { :on => :update },
                  :numericality => { :on => :update, :only_integer => true }
   validates :changeset, :associated => true

app/views/diary_comments/index.html.erb

@@ -1,10 +0,0 @@
-<% content_for :heading do %>
-  <h1><%= t ".heading", :user => @user.display_name %></h1>
-  <p><%= t ".subheading_html", :user => link_to(@user.display_name, @user) %></p>
-<% end %>
-
-<% if @comments.empty? %>
-  <h4><%= t ".no_comments" %></h4>
-<% else %>
-  <%= render :partial => "page" %>
-<% end -%>

app/views/users/changeset_comments/_page.html.erb

@@ -0,0 +1,22 @@
+<turbo-frame id="pagination" target="_top" data-turbo="false">
+  <table class="table table-striped" width="100%">
+    <thead>
+      <tr>
+        <th width="25%"><%= t ".changeset" %></th>
+        <th width="25%"><%= t ".when" %></th>
+        <th width="50%"><%= t ".comment" %></th>
+      </tr>
+    </thead>
+    <% @comments.each do |comment| -%>
+    <tr>
+      <td width="25%" class="<%= "text-muted" unless comment.visible? %>"><%= link_to comment.changeset.id, changeset_path(comment.changeset) %></td>
+      <td width="25%" class="<%= "text-muted" unless comment.visible? %>"><span title="<%= l comment.created_at, :format => :friendly %>"><%= time_ago_in_words(comment.created_at, :scope => :"datetime.distance_in_words_ago") %></span></td>
+      <td width="50%" class="richtext text-break<%= " text-muted" unless comment.visible? %>"><%= comment.body.to_html %></td>
+    </tr>
+    <% end -%>
+  </table>
+
+  <%= render "shared/pagination",
+             :newer_id => @newer_comments_id,
+             :older_id => @older_comments_id %>
+</turbo-frame>

app/views/users/comments/index.html.erb

@@ -0,0 +1,19 @@
+<% content_for :heading_class, "pb-0" %>
+
+<% content_for :heading do %>
+  <h1><%= t ".heading_html", :user => link_to(@user.display_name, @user) %></h1>
+  <ul class="nav nav-tabs">
+    <li class="nav-item">
+      <%= link_to t(".diary_entries"), user_diary_comments_path, :class => ["nav-link", { "active" => controller_name == "diary_comments" }] %>
+    </li>
+    <li class="nav-item">
+      <%= link_to t(".changesets"), user_changeset_comments_path, :class => ["nav-link", { "active" => controller_name == "changeset_comments" }] %>
+    </li>
+  </ul>
+<% end %>
+
+<% if @comments.empty? %>
+  <h4><%= t ".no_comments" %></h4>
+<% else %>
+  <%= render :partial => "page" %>
+<% end -%>

app/views/users/show.html.erb

@@ -26,7 +26,7 @@
               <span class='badge count-number'><%= number_with_delimiter(current_user.diary_entries.size) %></span>
             </li>
             <li>
-              <%= link_to t(".my comments"), diary_comments_path(current_user) %>
+              <%= link_to t(".my comments"), user_diary_comments_path(current_user) %>
               <span class='badge count-number'><%= number_with_delimiter(current_user.diary_comments.size) %></span>
             </li>
             <li>
@@ -78,7 +78,7 @@
               <span class='badge count-number'><%= number_with_delimiter(@user.diary_entries.size) %></span>
             </li>
             <li>
-              <%= link_to t(".comments"), diary_comments_path(@user) %>
+              <%= link_to t(".comments"), user_diary_comments_path(@user) %>
               <span class='badge count-number'><%= number_with_delimiter(@user.diary_comments.size) %></span>
             </li>
             <% if current_user %>

config/locales/en.yml

@@ -622,15 +622,6 @@ en:
       heading: Unsubscribe from the following diary entry discussion?
       button: Unsubscribe from discussion
   diary_comments:
-    index:
-      title: "Diary Comments added by %{user}"
-      heading: "%{user}'s Diary Comments"
-      subheading_html: "Diary Comments added by %{user}"
-      no_comments: "No diary comments"
-    page:
-      post: Post
-      when: When
-      comment: Comment
     new:
       heading: Add a comment to the following diary entry discussion?
   doorkeeper:
@@ -1948,6 +1939,9 @@ en:
       preview: Preview
       help: Help
     pagination:
+      changeset_comments:
+        older: Older Comments
+        newer: Newer Comments
       diary_comments:
         older: Older Comments
         newer: Newer Comments
@@ -2882,6 +2876,26 @@ en:
       user:
         summary_html: "%{name} created from %{ip_address} on %{date}"
         summary_no_ip_html: "%{name} created on %{date}"
+    comments:
+      index:
+        heading_html: "%{user}'s Comments"
+        changesets: "Changesets"
+        diary_entries: "Diary entries"
+        no_comments: "No comments"
+    changeset_comments:
+      index:
+        title: "Changeset Comments added by %{user}"
+      page:
+        changeset: Changeset
+        when: When
+        comment: Comment
+    diary_comments:
+      index:
+        title: "Diary Comments added by %{user}"
+      page:
+        post: Post
+        when: When
+        comment: Comment
     suspended:
       title: Account Suspended
       heading: Account Suspended

config/routes.rb

@@ -246,8 +246,6 @@ OpenStreetMap::Application.routes.draw do
   get "/user/:display_name/diary/rss" => "diary_entries#rss", :defaults => { :format => :rss }
   get "/diary/:language/rss" => "diary_entries#rss", :defaults => { :format => :rss }
   get "/diary/rss" => "diary_entries#rss", :defaults => { :format => :rss }
-  get "/user/:display_name/diary/comments/:page", :page => /[1-9][0-9]*/, :to => redirect(:path => "/user/%{display_name}/diary/comments")
-  get "/user/:display_name/diary/comments" => "diary_comments#index", :as => :diary_comments
   get "/user/:display_name/diary" => "diary_entries#index"
   get "/diary/:language" => "diary_entries#index"
   scope "/user/:display_name" do
@@ -269,12 +267,15 @@ OpenStreetMap::Application.routes.draw do
   resources :users, :path => "user", :param => :display_name, :only => [:new, :create, :show] do
     resource :role, :controller => "user_roles", :path => "roles/:role", :only => [:create, :destroy]
     scope :module => :users do
+      resources :diary_comments, :only => :index
+      resources :changeset_comments, :only => :index
       resource :issued_blocks, :path => "blocks_by", :only => :show
       resource :received_blocks, :path => "blocks", :only => [:show, :edit, :destroy]
       resource :status, :only => :update
     end
   end
   get "/user/:display_name/account", :to => redirect(:path => "/account/edit")
+  get "/user/:display_name/diary/comments(/:page)", :page => /[1-9][0-9]*/, :to => redirect(:path => "/user/%{display_name}/diary_comments")
 
   resource :account, :only => [:edit, :update, :destroy] do
     scope :module => :accounts do

test/controllers/diary_comments_controller_test.rb

@@ -8,10 +8,6 @@ class DiaryCommentsControllerTest < ActionDispatch::IntegrationTest
   end
 
   def test_routes
-    assert_routing(
-      { :path => "/user/username/diary/comments", :method => :get },
-      { :controller => "diary_comments", :action => "index", :display_name => "username" }
-    )
     assert_routing(
       { :path => "/user/username/diary/1/comments", :method => :post },
       { :controller => "diary_comments", :action => "create", :display_name => "username", :id => "1" }
@@ -24,53 +20,6 @@ class DiaryCommentsControllerTest < ActionDispatch::IntegrationTest
       { :path => "/diary_comments/2/unhide", :method => :post },
       { :controller => "diary_comments", :action => "unhide", :comment => "2" }
     )
-
-    get "/user/username/diary/comments/1"
-    assert_redirected_to "/user/username/diary/comments"
-  end
-
-  def test_index
-    user = create(:user)
-    other_user = create(:user)
-    suspended_user = create(:user, :suspended)
-    deleted_user = create(:user, :deleted)
-
-    # Test a user with no comments
-    get diary_comments_path(:display_name => user.display_name)
-    assert_response :success
-    assert_template :index
-    assert_select "h4", :html => "No diary comments"
-
-    # Test a user with a comment
-    create(:diary_comment, :user => other_user)
-
-    get diary_comments_path(:display_name => other_user.display_name)
-    assert_response :success
-    assert_template :index
-    assert_dom "a[href='#{user_path(other_user)}']", :text => other_user.display_name
-    assert_select "table.table-striped tbody" do
-      assert_select "tr", :count => 1
-    end
-
-    # Test a suspended user
-    get diary_comments_path(:display_name => suspended_user.display_name)
-    assert_response :not_found
-
-    # Test a deleted user
-    get diary_comments_path(:display_name => deleted_user.display_name)
-    assert_response :not_found
-  end
-
-  def test_index_invalid_paged
-    user = create(:user)
-
-    %w[-1 0 fred].each do |id|
-      get diary_comments_path(:display_name => user.display_name, :before => id)
-      assert_redirected_to :controller => :errors, :action => :bad_request
-
-      get diary_comments_path(:display_name => user.display_name, :after => id)
-      assert_redirected_to :controller => :errors, :action => :bad_request
-    end
   end
 
   def test_create

test/controllers/users/changeset_comments_controller_test.rb

@@ -0,0 +1,37 @@
+require "test_helper"
+
+module Users
+  class ChangesetCommentsControllerTest < ActionDispatch::IntegrationTest
+    ##
+    # test all routes which lead to this controller
+    def test_routes
+      assert_routing(
+        { :path => "/user/username/changeset_comments", :method => :get },
+        { :controller => "users/changeset_comments", :action => "index", :user_display_name => "username" }
+      )
+    end
+
+    def test_index
+      user = create(:user)
+      other_user = create(:user)
+      changeset = create(:changeset, :closed)
+      create_list(:changeset_comment, 3, :changeset => changeset, :author => user)
+      create_list(:changeset_comment, 2, :changeset => changeset, :author => other_user)
+
+      get user_changeset_comments_path(user)
+      assert_response :success
+      assert_select "table.table-striped tbody" do
+        assert_select "tr", :count => 3
+      end
+
+      create(:changeset_comment, :changeset => changeset, :author => user)
+      create(:changeset_comment, :changeset => changeset, :author => user, :visible => false)
+
+      get user_changeset_comments_path(user)
+      assert_response :success
+      assert_select "table.table-striped tbody" do
+        assert_select "tr", :count => 4
+      end
+    end
+  end
+end

test/controllers/users/diary_comments_controller_test.rb

@@ -0,0 +1,70 @@
+require "test_helper"
+
+module Users
+  class DiaryCommentsControllerTest < ActionDispatch::IntegrationTest
+    def setup
+      super
+      # Create the default language for diary entries
+      create(:language, :code => "en")
+    end
+
+    ##
+    # test all routes which lead to this controller
+    def test_routes
+      assert_routing(
+        { :path => "/user/username/diary_comments", :method => :get },
+        { :controller => "users/diary_comments", :action => "index", :user_display_name => "username" }
+      )
+
+      get "/user/username/diary/comments/1"
+      assert_redirected_to "/user/username/diary_comments"
+
+      get "/user/username/diary/comments"
+      assert_redirected_to "/user/username/diary_comments"
+    end
+
+    def test_index
+      user = create(:user)
+      other_user = create(:user)
+      suspended_user = create(:user, :suspended)
+      deleted_user = create(:user, :deleted)
+
+      # Test a user with no comments
+      get user_diary_comments_path(user)
+      assert_response :success
+      assert_template :index
+      assert_select "h4", :html => "No comments"
+
+      # Test a user with a comment
+      create(:diary_comment, :user => other_user)
+
+      get user_diary_comments_path(other_user)
+      assert_response :success
+      assert_template :index
+      assert_dom "a[href='#{user_path(other_user)}']", :text => other_user.display_name
+      assert_select "table.table-striped tbody" do
+        assert_select "tr", :count => 1
+      end
+
+      # Test a suspended user
+      get user_diary_comments_path(suspended_user)
+      assert_response :not_found
+
+      # Test a deleted user
+      get user_diary_comments_path(deleted_user)
+      assert_response :not_found
+    end
+
+    def test_index_invalid_paged
+      user = create(:user)
+
+      %w[-1 0 fred].each do |id|
+        get user_diary_comments_path(user, :before => id)
+        assert_redirected_to :controller => "/errors", :action => :bad_request
+
+        get user_diary_comments_path(user, :after => id)
+        assert_redirected_to :controller => "/errors", :action => :bad_request
+      end
+    end
+  end
+end

test/controllers/users_controller_test.rb

@@ -220,7 +220,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
       assert_select "a[href^='/user/#{ERB::Util.u(user.display_name)}/history']", 1
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/traces']", 1
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary']", 1
-      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary/comments']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary_comments']", 1
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/account']", 0
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks']", 0
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks_by']", 0
@@ -236,7 +236,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
       assert_select "a[href^='/user/#{ERB::Util.u(blocked_user.display_name)}/history']", 1
       assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/traces']", 1
       assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/diary']", 1
-      assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/diary/comments']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/diary_comments']", 1
       assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/account']", 0
       assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/blocks']", 1
       assert_select "a[href='/user/#{ERB::Util.u(blocked_user.display_name)}/blocks_by']", 0
@@ -252,7 +252,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
       assert_select "a[href^='/user/#{ERB::Util.u(moderator_user.display_name)}/history']", 1
       assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/traces']", 1
       assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/diary']", 1
-      assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/diary/comments']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/diary_comments']", 1
       assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/account']", 0
       assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/blocks']", 0
       assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/blocks_by']", 1
@@ -269,7 +269,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
       assert_select "a[href^='/user/#{ERB::Util.u(user.display_name)}/history']", 1
       assert_select "a[href='/traces/mine']", 1
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary']", 1
-      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary/comments']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary_comments']", 1
       assert_select "a[href='/account/edit']", 1
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks']", 0
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks_by']", 0
@@ -287,7 +287,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
       assert_select "a[href^='/user/#{ERB::Util.u(user.display_name)}/history']", 1
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/traces']", 1
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary']", 1
-      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary/comments']", 1
+      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary_comments']", 1
       assert_select "a[href='/account/edit']", 0
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks']", 0
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks_by']", 0