Merge 2b5af63895 into 432fa57e61

app/abilities/ability.rb

@@ -54,6 +54,7 @@ class Ability
         can [:read, :create, :destroy], UserMute
 
         if user.moderator?
+          can :manage, ChangesetTag
           can [:hide, :unhide], [DiaryEntry, DiaryComment]
           can [:read, :resolve, :ignore, :reopen], Issue
           can :create, IssueComment

app/controllers/changeset_tags_controller.rb

@@ -0,0 +1,40 @@
+class ChangesetTagsController < ApplicationController
+  layout "site"
+
+  before_action :authorize_web
+  before_action :set_locale
+  before_action :check_database_readable
+
+  authorize_resource
+
+  def show
+    @changeset = Changeset.find(params[:changeset_id])
+  rescue ActiveRecord::RecordNotFound
+    render :action => "changeset_not_found", :status => :not_found
+  end
+
+  def destroy
+    begin
+      @changeset = Changeset.find(params[:changeset_id])
+    rescue ActiveRecord::RecordNotFound
+      render :action => "changeset_not_found", :status => :not_found
+      return
+    end
+    begin
+      @key = Base64.urlsafe_decode64(params[:base64_key].to_s)
+    rescue ArgumentError
+      render :action => "invalid_tag", :status => :not_found
+      return
+    end
+    begin
+      @changeset_tag = ChangesetTag.find([params[:changeset_id], @key])
+    rescue ActiveRecord::RecordNotFound
+      render :action => "tag_not_found", :status => :not_found
+      return
+    end
+
+    @changeset_tag.delete
+    flash[:notice] = t ".success", :k => @changeset_tag.k, :v => @changeset_tag.v
+    redirect_to changeset_tags_path(@changeset)
+  end
+end

app/views/changeset_tags/_tag.html.erb

@@ -0,0 +1,12 @@
+<tr>
+  <th class='py-1 border-secondary-subtle table-secondary fw-normal' dir='auto'><%= format_key(tag[0]) %></th>
+  <td class='py-1 border-secondary-subtle border-start' dir='auto'><%= format_value(tag[0], tag[1]) %></td>
+  <td class='py-1 border-secondary-subtle text-end'>
+    <%= button_to t(".delete"),
+                  changeset_tags_path(@changeset),
+                  :method => :delete,
+                  :params => { :base64_key => Base64.urlsafe_encode64(tag[0]) },
+                  :data => { :confirm => t(".confirm_delete", :k => tag[0], :v => tag[1]) },
+                  :class => "btn btn-sm btn-danger" %>
+  </td>
+</tr>

app/views/changeset_tags/changeset_not_found.html.erb

@@ -0,0 +1,7 @@
+<% content_for :heading do %>
+  <h1><%= t(".heading") %></h1>
+<% end %>
+
+<div>
+  <p><%= t ".body", :id => params[:changeset_id] %>
+</div>

app/views/changeset_tags/invalid_tag.html.erb

@@ -0,0 +1,7 @@
+<% content_for :heading do %>
+  <h1><%= t(".heading") %></h1>
+<% end %>
+
+<div>
+  <p><%= t ".body" %>
+</div>

app/views/changeset_tags/show.html.erb

@@ -0,0 +1,19 @@
+<% content_for :heading do %>
+  <h1><%= t ".heading" %></h1>
+<% end %>
+
+<div class="alert alert-warning">
+  <%= t ".comment_warning" %>
+</div>
+
+<h2><%= t ".changeset_html", :id => link_to(@changeset.id, @changeset) %></h2>
+
+<p><%= changeset_details(@changeset) %></p>
+
+<% unless @changeset.tags.empty? %>
+  <div class='mb-3 border border-secondary-subtle rounded overflow-hidden'>
+    <table class='mb-0 table align-middle'>
+      <%= render :partial => "tag", :collection => @changeset.tags.sort %>
+    </table>
+  </div>
+<% end %>

app/views/changeset_tags/tag_not_found.html.erb

@@ -0,0 +1,7 @@
+<% content_for :heading do %>
+  <h1><%= t(".heading") %></h1>
+<% end %>
+
+<div>
+  <p><%= t ".body", :id => params[:changeset_id], :k => @key %>
+</div>

app/views/changesets/show.html.erb

@@ -111,9 +111,13 @@
 </div>
 
 <div class='secondary-actions'>
-  <%= link_to t(".changesetxml"), api_changeset_path(@changeset) %>
+  <%= link_to t(".changesetxml"), api_changeset_path(@changeset), :class => "text-nowrap" %>
   &middot;
-  <%= link_to t(".osmchangexml"), api_changeset_download_path(@changeset) %>
+  <%= link_to t(".osmchangexml"), api_changeset_download_path(@changeset), :class => "text-nowrap" %>
+  <% if can? :manage, ChangesetTag %>
+    &middot;
+    <%= link_to t(".manage_tags"), changeset_tags_path(@changeset), :class => "text-nowrap" %>
+  <% end %>
 </div>
 
 <% if @next_by_user || @prev_by_user %>

config/locales/en.yml

@@ -474,6 +474,25 @@ en:
         title_particular: "OpenStreetMap changeset #%{changeset_id} discussion"
       timeout:
         sorry: "Sorry, the list of changeset comments you requested took too long to retrieve."
+  changeset_tags:
+    show:
+      heading: Manage changeset tags
+      changeset_html: "Changeset: %{id}"
+      comment_warning: Remember to add a comment to the changeset after modifying its tags. This ensures that the changes you made are replicated.
+    tag:
+      delete: "Delete"
+      confirm_delete: "Delete the tag %{k}=%{v} ?"
+    changeset_not_found:
+      heading: Changeset does not exist
+      body: "Sorry, changeset #%{id} could not be found."
+    invalid_tag:
+      heading: Invalid changeset tag
+      body: "Sorry, the requested tag key cannot be decoded."
+    tag_not_found:
+      heading: Changeset tag does not exist
+      body: "Sorry, tag %{k} could not be found in changeset #%{id}."
+    destroy:
+      success: Tag %{k}=%{v} deleted successfully.
   changesets:
     changeset:
       comments:
@@ -521,6 +540,7 @@ en:
       comment: "Comment"
       changesetxml: "Changeset XML"
       osmchangexml: "osmChange XML"
+      manage_tags: "Manage changeset tags"
     paging_nav:
       nodes: "Nodes (%{count})"
       nodes_paginated: "Nodes (%{x}-%{y} of %{count})"

config/routes.rb

@@ -151,6 +151,8 @@ OpenStreetMap::Application.routes.draw do
     namespace :changeset_comments, :as => :comments, :path => :comments do
       resource :feed, :only => :show, :defaults => { :format => "rss" }
     end
+
+    resource :tags, :controller => "changeset_tags", :only => [:show, :destroy]
   end
   get "/changeset/:id/subscribe", :id => /\d+/, :to => redirect(:path => "/changeset/%{id}/subscription")
   get "/changeset/:id/unsubscribe", :id => /\d+/, :to => redirect(:path => "/changeset/%{id}/subscription")

test/controllers/changeset_tags_controller_test.rb

@@ -0,0 +1,206 @@
+require "test_helper"
+
+class ChangesetTagsControllerTest < ActionDispatch::IntegrationTest
+  def test_routes
+    assert_routing(
+      { :path => "/changeset/1/tags", :method => :get },
+      { :controller => "changeset_tags", :action => "show", :changeset_id => "1" }
+    )
+    assert_routing(
+      { :path => "/changeset/1/tags", :method => :delete },
+      { :controller => "changeset_tags", :action => "destroy", :changeset_id => "1" }
+    )
+  end
+
+  def test_show_success
+    changeset = create(:changeset)
+    moderator_user = create(:moderator_user)
+
+    session_for(moderator_user)
+
+    get changeset_tags_path(changeset)
+    assert_response :success
+
+    assert_dom ".content-body" do
+      assert_dom "h2", :text => "Changeset: #{changeset.id}" do
+        assert_dom "a[href='#{changeset_path(changeset)}']"
+      end
+
+      assert_dom "a[href='#{user_path(changeset.user)}']"
+    end
+  end
+
+  def test_show_success_1_tag
+    changeset = create(:changeset)
+    create(:changeset_tag, :changeset => changeset, :k => "tested-tag-key", :v => "tested-tag-value")
+    moderator_user = create(:moderator_user)
+
+    session_for(moderator_user)
+
+    get changeset_tags_path(changeset)
+    assert_response :success
+
+    assert_dom ".content-body" do
+      assert_dom "tbody tr", :count => 1 do |rows|
+        check_tag_table_row rows[0], changeset, "tested-tag-key", "tested-tag-value"
+      end
+    end
+  end
+
+  def test_show_success_2_tags
+    changeset = create(:changeset)
+    create(:changeset_tag, :changeset => changeset, :k => "tested-1st-tag-key", :v => "tested-1st-tag-value")
+    create(:changeset_tag, :changeset => changeset, :k => "tested-2nd-tag-key", :v => "tested-2nd-tag-value")
+    moderator_user = create(:moderator_user)
+
+    session_for(moderator_user)
+
+    get changeset_tags_path(changeset)
+    assert_response :success
+
+    assert_dom ".content-body" do
+      assert_dom "tbody tr", :count => 2 do |rows|
+        check_tag_table_row rows[0], changeset, "tested-1st-tag-key", "tested-1st-tag-value"
+        check_tag_table_row rows[1], changeset, "tested-2nd-tag-key", "tested-2nd-tag-value"
+      end
+    end
+  end
+
+  def test_show_success_empty_tag
+    changeset = create(:changeset)
+    create(:changeset_tag, :changeset => changeset, :k => "", :v => "")
+    moderator_user = create(:moderator_user)
+
+    session_for(moderator_user)
+
+    get changeset_tags_path(changeset)
+    assert_response :success
+
+    assert_dom ".content-body" do
+      assert_dom "tbody tr", :count => 1 do |rows|
+        check_tag_table_row rows[0], changeset, "", ""
+      end
+    end
+  end
+
+  def test_show_fail_no_changeset
+    moderator_user = create(:moderator_user)
+
+    session_for(moderator_user)
+
+    get changeset_tags_path(999111)
+    assert_response :not_found
+  end
+
+  def test_show_fail_not_logged_in
+    changeset = create(:changeset)
+
+    get changeset_tags_path(changeset)
+    assert_redirected_to login_path(:referer => changeset_tags_path(changeset))
+  end
+
+  def test_show_fail_not_moderator
+    changeset = create(:changeset)
+    user = create(:user)
+
+    session_for(user)
+
+    get changeset_tags_path(changeset)
+    assert_redirected_to :controller => :errors, :action => :forbidden
+  end
+
+  def test_destroy_success
+    changeset = create(:changeset)
+    create(:changeset_tag, :changeset => changeset, :k => "tested-1st-tag-key", :v => "tested-1st-tag-value")
+    create(:changeset_tag, :changeset => changeset, :k => "tested-2nd-tag-key", :v => "tested-2nd-tag-value")
+    other_changeset = create(:changeset)
+    create(:changeset_tag, :changeset => other_changeset, :k => "tested-1st-tag-key", :v => "tested-1st-tag-value")
+    moderator_user = create(:moderator_user)
+
+    session_for(moderator_user)
+
+    assert_difference "ChangesetTag.count", -1 do
+      delete changeset_tags_path(changeset, :base64_key => Base64.urlsafe_encode64("tested-1st-tag-key"))
+      assert_redirected_to changeset_tags_path(changeset)
+    end
+    assert_equal({ "tested-2nd-tag-key" => "tested-2nd-tag-value" }, changeset.tags)
+    assert_match(/tested-1st-tag-key=tested-1st-tag-value deleted successfully/, flash[:notice])
+  end
+
+  def test_destroy_success_empty_tag
+    changeset = create(:changeset)
+    create(:changeset_tag, :changeset => changeset, :k => "", :v => "")
+    moderator_user = create(:moderator_user)
+
+    session_for(moderator_user)
+
+    assert_difference "ChangesetTag.count", -1 do
+      delete changeset_tags_path(changeset, :base64_key => Base64.urlsafe_encode64(""))
+      assert_redirected_to changeset_tags_path(changeset)
+    end
+    assert_empty changeset.tags
+  end
+
+  def test_destroy_fail_no_changeset
+    moderator_user = create(:moderator_user)
+
+    session_for(moderator_user)
+
+    delete changeset_tags_path(999111, :base64_key => Base64.urlsafe_encode64("nope"))
+    assert_response :not_found
+  end
+
+  def test_destroy_fail_invalid_key_encoding
+    changeset = create(:changeset)
+    moderator_user = create(:moderator_user)
+
+    session_for(moderator_user)
+
+    delete changeset_tags_path(changeset, :base64_key => "ZnJvbV9jb")
+    assert_response :not_found
+  end
+
+  def test_destroy_fail_no_key
+    changeset = create(:changeset)
+    moderator_user = create(:moderator_user)
+
+    session_for(moderator_user)
+
+    delete changeset_tags_path(changeset, :base64_key => Base64.urlsafe_encode64("tested-missing-tag"))
+    assert_response :not_found
+
+    assert_dom ".content-body", :text => /tested-missing-tag could not be found/
+  end
+
+  def test_destroy_fail_not_logged_in
+    changeset = create(:changeset)
+    create(:changeset_tag, :changeset => changeset, :k => "tested-tag-key", :v => "tested-tag-value")
+
+    delete changeset_tags_path(changeset, :base64_key => Base64.urlsafe_encode64("tested-tag-key"))
+    assert_response :forbidden
+  end
+
+  def test_destroy_fail_not_moderator
+    changeset = create(:changeset)
+    create(:changeset_tag, :changeset => changeset, :k => "tested-tag-key", :v => "tested-tag-value")
+    user = create(:user)
+
+    session_for(user)
+
+    delete changeset_tags_path(changeset, :base64_key => Base64.urlsafe_encode64("tested-tag-key"))
+    assert_redirected_to :controller => :errors, :action => :forbidden
+  end
+
+  private
+
+  def check_tag_table_row(row, changeset, key, value)
+    assert_dom row, "th", :text => key
+    assert_dom row, "td", :text => value
+    assert_dom row, "td form[action='#{changeset_tags_path(changeset)}']" do
+      assert_dom "input[type='hidden'][name='base64_key']" do
+        assert_dom "> @value", Base64.urlsafe_encode64(key)
+      end
+      assert_dom "button", :text => "Delete"
+    end
+  end
+end

test/controllers/changesets_controller_test.rb

@@ -334,6 +334,33 @@ class ChangesetsControllerTest < ActionDispatch::IntegrationTest
     assert_dom "a[href='#{changeset_path changeset5}']", :count => 1
   end
 
+  def test_show_no_manage_tags_link_for_anonymous_users
+    changeset = create(:changeset)
+
+    sidebar_browse_check :changeset_path, changeset.id, "changesets/show"
+    assert_dom ".secondary-actions a[href='#{changeset_tags_path changeset}']", :count => 0
+  end
+
+  def test_show_no_manage_tags_link_for_regular_users
+    changeset = create(:changeset)
+    user = create(:user)
+
+    session_for(user)
+
+    sidebar_browse_check :changeset_path, changeset.id, "changesets/show"
+    assert_dom ".secondary-actions a[href='#{changeset_tags_path changeset}']", :count => 0
+  end
+
+  def test_show_manage_tags_link
+    changeset = create(:changeset)
+    moderator_user = create(:moderator_user)
+
+    session_for(moderator_user)
+
+    sidebar_browse_check :changeset_path, changeset.id, "changesets/show"
+    assert_dom ".secondary-actions a[href='#{changeset_tags_path changeset}']", :count => 1
+  end
+
   ##
   # This should display the last 20 non-empty changesets
   def test_feed