From 8cce21cfd7a363c31c351e78e6ffc931784d9c96 Mon Sep 17 00:00:00 2001
From: Anton Khorev <tony29@yandex.ru>
Date: Thu, 7 Nov 2024 13:33:04 +0300
Subject: [PATCH 01/12] Add blank page for managing changeset tags

---
 app/abilities/ability.rb                      |  1 +
 app/controllers/changeset_tags_controller.rb  | 11 ++++++
 app/views/changeset_tags/show.html.erb        |  3 ++
 config/locales/en.yml                         |  3 ++
 config/routes.rb                              |  2 +
 .../changeset_tags_controller_test.rb         | 37 +++++++++++++++++++
 6 files changed, 57 insertions(+)
 create mode 100644 app/controllers/changeset_tags_controller.rb
 create mode 100644 app/views/changeset_tags/show.html.erb
 create mode 100644 test/controllers/changeset_tags_controller_test.rb

diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb
index 7f47b578c..d306f51b9 100644
--- a/app/abilities/ability.rb
+++ b/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
diff --git a/app/controllers/changeset_tags_controller.rb b/app/controllers/changeset_tags_controller.rb
new file mode 100644
index 000000000..bd617e676
--- /dev/null
+++ b/app/controllers/changeset_tags_controller.rb
@@ -0,0 +1,11 @@
+class ChangesetTagsController < ApplicationController
+  layout "site"
+
+  before_action :authorize_web
+  before_action :set_locale
+  before_action :check_database_readable
+
+  authorize_resource
+
+  def show; end
+end
diff --git a/app/views/changeset_tags/show.html.erb b/app/views/changeset_tags/show.html.erb
new file mode 100644
index 000000000..21da774c7
--- /dev/null
+++ b/app/views/changeset_tags/show.html.erb
@@ -0,0 +1,3 @@
+<% content_for :heading do %>
+  <h1><%= t ".heading" %></h1>
+<% end %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index b56fb6410..1fb6a1dd6 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -473,6 +473,9 @@ 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
   changesets:
     changeset:
       no_edits: "(no edits)"
diff --git a/config/routes.rb b/config/routes.rb
index 3f4a12bd7..2187bbd79 100644
--- a/config/routes.rb
+++ b/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
   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")
diff --git a/test/controllers/changeset_tags_controller_test.rb b/test/controllers/changeset_tags_controller_test.rb
new file mode 100644
index 000000000..c3189959d
--- /dev/null
+++ b/test/controllers/changeset_tags_controller_test.rb
@@ -0,0 +1,37 @@
+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" }
+    )
+  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
+  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
+end

From 3d52fa21e207129519ca6dc2660f0be32cdd4d1c Mon Sep 17 00:00:00 2001
From: Anton Khorev <tony29@yandex.ru>
Date: Thu, 7 Nov 2024 13:51:37 +0300
Subject: [PATCH 02/12] Add 'not found' page for managing changeset tags

---
 app/controllers/changeset_tags_controller.rb       | 6 +++++-
 app/views/changeset_tags/not_found.html.erb        | 7 +++++++
 config/locales/en.yml                              | 3 +++
 test/controllers/changeset_tags_controller_test.rb | 9 +++++++++
 4 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 app/views/changeset_tags/not_found.html.erb

diff --git a/app/controllers/changeset_tags_controller.rb b/app/controllers/changeset_tags_controller.rb
index bd617e676..0f9b38547 100644
--- a/app/controllers/changeset_tags_controller.rb
+++ b/app/controllers/changeset_tags_controller.rb
@@ -7,5 +7,9 @@ class ChangesetTagsController < ApplicationController
 
   authorize_resource
 
-  def show; end
+  def show
+    Changeset.find(params[:changeset_id])
+  rescue ActiveRecord::RecordNotFound
+    render :action => "not_found", :status => :not_found
+  end
 end
diff --git a/app/views/changeset_tags/not_found.html.erb b/app/views/changeset_tags/not_found.html.erb
new file mode 100644
index 000000000..26e801b8b
--- /dev/null
+++ b/app/views/changeset_tags/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>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 1fb6a1dd6..d466cadcb 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -476,6 +476,9 @@ en:
   changeset_tags:
     show:
       heading: Manage changeset tags
+    not_found:
+      heading: Changeset does not exist
+      body: "Sorry, changeset #%{id} could not be found."
   changesets:
     changeset:
       no_edits: "(no edits)"
diff --git a/test/controllers/changeset_tags_controller_test.rb b/test/controllers/changeset_tags_controller_test.rb
index c3189959d..c85c40be5 100644
--- a/test/controllers/changeset_tags_controller_test.rb
+++ b/test/controllers/changeset_tags_controller_test.rb
@@ -18,6 +18,15 @@ class ChangesetTagsControllerTest < ActionDispatch::IntegrationTest
     assert_response :success
   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)
 

From 74b1560099d9ccd428d3e0c9d62e63c806ff39fd Mon Sep 17 00:00:00 2001
From: Anton Khorev <tony29@yandex.ru>
Date: Thu, 7 Nov 2024 14:02:28 +0300
Subject: [PATCH 03/12] Add changeset link to changeset tags page

---
 app/controllers/changeset_tags_controller.rb       | 2 +-
 app/views/changeset_tags/show.html.erb             | 2 ++
 config/locales/en.yml                              | 1 +
 test/controllers/changeset_tags_controller_test.rb | 6 ++++++
 4 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/app/controllers/changeset_tags_controller.rb b/app/controllers/changeset_tags_controller.rb
index 0f9b38547..a836c1f48 100644
--- a/app/controllers/changeset_tags_controller.rb
+++ b/app/controllers/changeset_tags_controller.rb
@@ -8,7 +8,7 @@ class ChangesetTagsController < ApplicationController
   authorize_resource
 
   def show
-    Changeset.find(params[:changeset_id])
+    @changeset = Changeset.find(params[:changeset_id])
   rescue ActiveRecord::RecordNotFound
     render :action => "not_found", :status => :not_found
   end
diff --git a/app/views/changeset_tags/show.html.erb b/app/views/changeset_tags/show.html.erb
index 21da774c7..22d4bac2b 100644
--- a/app/views/changeset_tags/show.html.erb
+++ b/app/views/changeset_tags/show.html.erb
@@ -1,3 +1,5 @@
 <% content_for :heading do %>
   <h1><%= t ".heading" %></h1>
 <% end %>
+
+<h2><%= t ".changeset_html", :id => link_to(@changeset.id, @changeset) %></h2>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index d466cadcb..81ec66896 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -476,6 +476,7 @@ en:
   changeset_tags:
     show:
       heading: Manage changeset tags
+      changeset_html: "Changeset: %{id}"
     not_found:
       heading: Changeset does not exist
       body: "Sorry, changeset #%{id} could not be found."
diff --git a/test/controllers/changeset_tags_controller_test.rb b/test/controllers/changeset_tags_controller_test.rb
index c85c40be5..953755f51 100644
--- a/test/controllers/changeset_tags_controller_test.rb
+++ b/test/controllers/changeset_tags_controller_test.rb
@@ -16,6 +16,12 @@ class ChangesetTagsControllerTest < ActionDispatch::IntegrationTest
 
     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
+    end
   end
 
   def test_show_fail_no_changeset

From 6bd2477e12f0b2bcb34125946dfddb941ad644da Mon Sep 17 00:00:00 2001
From: Anton Khorev <tony29@yandex.ru>
Date: Thu, 7 Nov 2024 14:08:20 +0300
Subject: [PATCH 04/12] Add date/user to changeset tags page

---
 app/views/changeset_tags/show.html.erb             | 2 ++
 test/controllers/changeset_tags_controller_test.rb | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/app/views/changeset_tags/show.html.erb b/app/views/changeset_tags/show.html.erb
index 22d4bac2b..b1a3b7e48 100644
--- a/app/views/changeset_tags/show.html.erb
+++ b/app/views/changeset_tags/show.html.erb
@@ -3,3 +3,5 @@
 <% end %>
 
 <h2><%= t ".changeset_html", :id => link_to(@changeset.id, @changeset) %></h2>
+
+<p><%= changeset_details(@changeset) %></p>
diff --git a/test/controllers/changeset_tags_controller_test.rb b/test/controllers/changeset_tags_controller_test.rb
index 953755f51..fc3901e9e 100644
--- a/test/controllers/changeset_tags_controller_test.rb
+++ b/test/controllers/changeset_tags_controller_test.rb
@@ -21,6 +21,8 @@ class ChangesetTagsControllerTest < ActionDispatch::IntegrationTest
       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
 

From 71957f32106412f15b3009f5ea55045174d44a26 Mon Sep 17 00:00:00 2001
From: Anton Khorev <tony29@yandex.ru>
Date: Thu, 7 Nov 2024 14:42:50 +0300
Subject: [PATCH 05/12] Add tags list to changeset tags page

---
 app/views/changeset_tags/_tag.html.erb        |  4 ++
 app/views/changeset_tags/show.html.erb        |  8 ++++
 .../changeset_tags_controller_test.rb         | 43 +++++++++++++++++++
 3 files changed, 55 insertions(+)
 create mode 100644 app/views/changeset_tags/_tag.html.erb

diff --git a/app/views/changeset_tags/_tag.html.erb b/app/views/changeset_tags/_tag.html.erb
new file mode 100644
index 000000000..f2e34d879
--- /dev/null
+++ b/app/views/changeset_tags/_tag.html.erb
@@ -0,0 +1,4 @@
+<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>
+</tr>
diff --git a/app/views/changeset_tags/show.html.erb b/app/views/changeset_tags/show.html.erb
index b1a3b7e48..8983d7c2c 100644
--- a/app/views/changeset_tags/show.html.erb
+++ b/app/views/changeset_tags/show.html.erb
@@ -5,3 +5,11 @@
 <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 %>
diff --git a/test/controllers/changeset_tags_controller_test.rb b/test/controllers/changeset_tags_controller_test.rb
index fc3901e9e..44a202297 100644
--- a/test/controllers/changeset_tags_controller_test.rb
+++ b/test/controllers/changeset_tags_controller_test.rb
@@ -26,6 +26,42 @@ class ChangesetTagsControllerTest < ActionDispatch::IntegrationTest
     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_fail_no_changeset
     moderator_user = create(:moderator_user)
 
@@ -51,4 +87,11 @@ class ChangesetTagsControllerTest < ActionDispatch::IntegrationTest
     get changeset_tags_path(changeset)
     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
+  end
 end

From db0b85afb8d6d7ff37a330108c3f036c6b5dd345 Mon Sep 17 00:00:00 2001
From: Anton Khorev <tony29@yandex.ru>
Date: Thu, 7 Nov 2024 15:22:45 +0300
Subject: [PATCH 06/12] Add passive delete buttons to changeset tags page

---
 app/controllers/changeset_tags_controller.rb       |  2 ++
 app/views/changeset_tags/_tag.html.erb             |  8 ++++++++
 config/locales/en.yml                              |  3 +++
 config/routes.rb                                   |  2 +-
 test/controllers/changeset_tags_controller_test.rb | 12 +++++++++++-
 5 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/app/controllers/changeset_tags_controller.rb b/app/controllers/changeset_tags_controller.rb
index a836c1f48..9bc532346 100644
--- a/app/controllers/changeset_tags_controller.rb
+++ b/app/controllers/changeset_tags_controller.rb
@@ -12,4 +12,6 @@ class ChangesetTagsController < ApplicationController
   rescue ActiveRecord::RecordNotFound
     render :action => "not_found", :status => :not_found
   end
+
+  def destroy; end
 end
diff --git a/app/views/changeset_tags/_tag.html.erb b/app/views/changeset_tags/_tag.html.erb
index f2e34d879..83052d96c 100644
--- a/app/views/changeset_tags/_tag.html.erb
+++ b/app/views/changeset_tags/_tag.html.erb
@@ -1,4 +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 => { :key => tag[0] },
+                  :data => { :confirm => t(".confirm_delete", :k => tag[0], :v => tag[1]) },
+                  :class => "btn btn-sm btn-danger" %>
+  </td>
 </tr>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 81ec66896..e0cc92acf 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -477,6 +477,9 @@ en:
     show:
       heading: Manage changeset tags
       changeset_html: "Changeset: %{id}"
+    tag:
+      delete: "Delete"
+      confirm_delete: "Delete the tag %{k}=%{v} ?"
     not_found:
       heading: Changeset does not exist
       body: "Sorry, changeset #%{id} could not be found."
diff --git a/config/routes.rb b/config/routes.rb
index 2187bbd79..754eec35c 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -152,7 +152,7 @@ OpenStreetMap::Application.routes.draw do
       resource :feed, :only => :show, :defaults => { :format => "rss" }
     end
 
-    resource :tags, :controller => "changeset_tags", :only => :show
+    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")
diff --git a/test/controllers/changeset_tags_controller_test.rb b/test/controllers/changeset_tags_controller_test.rb
index 44a202297..f64be67e9 100644
--- a/test/controllers/changeset_tags_controller_test.rb
+++ b/test/controllers/changeset_tags_controller_test.rb
@@ -6,6 +6,10 @@ class ChangesetTagsControllerTest < ActionDispatch::IntegrationTest
       { :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
@@ -90,8 +94,14 @@ class ChangesetTagsControllerTest < ActionDispatch::IntegrationTest
 
   private
 
-  def check_tag_table_row(row, _changeset, key, value)
+  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='key']" do
+        assert_dom "> @value", key
+      end
+      assert_dom "button", :text => "Delete"
+    end
   end
 end

From 52b2cf61b1e3354f16e7b6cb4500d1b974de5558 Mon Sep 17 00:00:00 2001
From: Anton Khorev <tony29@yandex.ru>
Date: Thu, 7 Nov 2024 16:08:23 +0300
Subject: [PATCH 07/12] Implement changeset tag deletion

---
 app/controllers/changeset_tags_controller.rb  | 20 ++++++-
 ....html.erb => changeset_not_found.html.erb} |  0
 .../changeset_tags/tag_not_found.html.erb     |  7 +++
 config/locales/en.yml                         |  5 +-
 .../changeset_tags_controller_test.rb         | 55 +++++++++++++++++++
 5 files changed, 84 insertions(+), 3 deletions(-)
 rename app/views/changeset_tags/{not_found.html.erb => changeset_not_found.html.erb} (100%)
 create mode 100644 app/views/changeset_tags/tag_not_found.html.erb

diff --git a/app/controllers/changeset_tags_controller.rb b/app/controllers/changeset_tags_controller.rb
index 9bc532346..fd20a81fb 100644
--- a/app/controllers/changeset_tags_controller.rb
+++ b/app/controllers/changeset_tags_controller.rb
@@ -10,8 +10,24 @@ class ChangesetTagsController < ApplicationController
   def show
     @changeset = Changeset.find(params[:changeset_id])
   rescue ActiveRecord::RecordNotFound
-    render :action => "not_found", :status => :not_found
+    render :action => "changeset_not_found", :status => :not_found
   end
 
-  def destroy; end
+  def destroy
+    begin
+      @changeset = Changeset.find(params[:changeset_id])
+    rescue ActiveRecord::RecordNotFound
+      render :action => "changeset_not_found", :status => :not_found
+      return
+    end
+    begin
+      @changeset_tag = ChangesetTag.find([params[:changeset_id], params[:key]])
+    rescue ActiveRecord::RecordNotFound
+      render :action => "tag_not_found", :status => :not_found
+      return
+    end
+
+    @changeset_tag.delete
+    redirect_to changeset_tags_path(@changeset)
+  end
 end
diff --git a/app/views/changeset_tags/not_found.html.erb b/app/views/changeset_tags/changeset_not_found.html.erb
similarity index 100%
rename from app/views/changeset_tags/not_found.html.erb
rename to app/views/changeset_tags/changeset_not_found.html.erb
diff --git a/app/views/changeset_tags/tag_not_found.html.erb b/app/views/changeset_tags/tag_not_found.html.erb
new file mode 100644
index 000000000..bc89f35dd
--- /dev/null
+++ b/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 => params[:key] %>
+</div>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index e0cc92acf..09244a430 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -480,9 +480,12 @@ en:
     tag:
       delete: "Delete"
       confirm_delete: "Delete the tag %{k}=%{v} ?"
-    not_found:
+    changeset_not_found:
       heading: Changeset does not exist
       body: "Sorry, changeset #%{id} could not be found."
+    tag_not_found:
+      heading: Changeset tag does not exist
+      body: "Sorry, tag %{k} could not be found in changeset #%{id}."
   changesets:
     changeset:
       no_edits: "(no edits)"
diff --git a/test/controllers/changeset_tags_controller_test.rb b/test/controllers/changeset_tags_controller_test.rb
index f64be67e9..69590ed94 100644
--- a/test/controllers/changeset_tags_controller_test.rb
+++ b/test/controllers/changeset_tags_controller_test.rb
@@ -92,6 +92,61 @@ class ChangesetTagsControllerTest < ActionDispatch::IntegrationTest
     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, :key => "tested-1st-tag-key")
+      assert_redirected_to changeset_tags_path(changeset)
+    end
+    assert_equal({ "tested-2nd-tag-key" => "tested-2nd-tag-value" }, changeset.tags)
+  end
+
+  def test_destroy_fail_no_changeset
+    moderator_user = create(:moderator_user)
+
+    session_for(moderator_user)
+
+    delete changeset_tags_path(999111, :key => "nope")
+    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, :key => "nope")
+    assert_response :not_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, :key => "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, :key => "tested-tag-key")
+    assert_redirected_to :controller => :errors, :action => :forbidden
+  end
+
   private
 
   def check_tag_table_row(row, changeset, key, value)

From cf864c8273ba782dadc5e0a302b33bce84a1b7f8 Mon Sep 17 00:00:00 2001
From: Anton Khorev <tony29@yandex.ru>
Date: Thu, 7 Nov 2024 16:33:43 +0300
Subject: [PATCH 08/12] Add link to manage changeset tags page

---
 app/views/changesets/show.html.erb            |  8 ++++--
 config/locales/en.yml                         |  1 +
 .../controllers/changesets_controller_test.rb | 27 +++++++++++++++++++
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/app/views/changesets/show.html.erb b/app/views/changesets/show.html.erb
index 70b1877b5..1ce4abbdf 100644
--- a/app/views/changesets/show.html.erb
+++ b/app/views/changesets/show.html.erb
@@ -105,9 +105,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 %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 09244a430..6f4a8fbee 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -529,6 +529,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})"
diff --git a/test/controllers/changesets_controller_test.rb b/test/controllers/changesets_controller_test.rb
index 2e701f248..f9d55d2c1 100644
--- a/test/controllers/changesets_controller_test.rb
+++ b/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

From 88fdc809059052ce86f01bb834600481a05b15e4 Mon Sep 17 00:00:00 2001
From: Anton Khorev <tony29@yandex.ru>
Date: Thu, 7 Nov 2024 19:31:54 +0300
Subject: [PATCH 09/12] Use base64 encoding to support arbitrary tag keys

---
 app/controllers/changeset_tags_controller.rb  |  3 +-
 app/views/changeset_tags/_tag.html.erb        |  2 +-
 .../changeset_tags/tag_not_found.html.erb     |  2 +-
 .../changeset_tags_controller_test.rb         | 47 ++++++++++++++++---
 4 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/app/controllers/changeset_tags_controller.rb b/app/controllers/changeset_tags_controller.rb
index fd20a81fb..2cd2a1853 100644
--- a/app/controllers/changeset_tags_controller.rb
+++ b/app/controllers/changeset_tags_controller.rb
@@ -21,7 +21,8 @@ class ChangesetTagsController < ApplicationController
       return
     end
     begin
-      @changeset_tag = ChangesetTag.find([params[:changeset_id], params[:key]])
+      @key = Base64.urlsafe_decode64(params[:base64_key].to_s)
+      @changeset_tag = ChangesetTag.find([params[:changeset_id], @key])
     rescue ActiveRecord::RecordNotFound
       render :action => "tag_not_found", :status => :not_found
       return
diff --git a/app/views/changeset_tags/_tag.html.erb b/app/views/changeset_tags/_tag.html.erb
index 83052d96c..94747c430 100644
--- a/app/views/changeset_tags/_tag.html.erb
+++ b/app/views/changeset_tags/_tag.html.erb
@@ -5,7 +5,7 @@
     <%= button_to t(".delete"),
                   changeset_tags_path(@changeset),
                   :method => :delete,
-                  :params => { :key => tag[0] },
+                  :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>
diff --git a/app/views/changeset_tags/tag_not_found.html.erb b/app/views/changeset_tags/tag_not_found.html.erb
index bc89f35dd..8d50e4c28 100644
--- a/app/views/changeset_tags/tag_not_found.html.erb
+++ b/app/views/changeset_tags/tag_not_found.html.erb
@@ -3,5 +3,5 @@
 <% end %>
 
 <div>
-  <p><%= t ".body", :id => params[:changeset_id], :k => params[:key] %>
+  <p><%= t ".body", :id => params[:changeset_id], :k => @key %>
 </div>
diff --git a/test/controllers/changeset_tags_controller_test.rb b/test/controllers/changeset_tags_controller_test.rb
index 69590ed94..e8bdc0d53 100644
--- a/test/controllers/changeset_tags_controller_test.rb
+++ b/test/controllers/changeset_tags_controller_test.rb
@@ -66,6 +66,23 @@ class ChangesetTagsControllerTest < ActionDispatch::IntegrationTest
     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)
 
@@ -103,18 +120,32 @@ class ChangesetTagsControllerTest < ActionDispatch::IntegrationTest
     session_for(moderator_user)
 
     assert_difference "ChangesetTag.count", -1 do
-      delete changeset_tags_path(changeset, :key => "tested-1st-tag-key")
+      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)
   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, :key => "nope")
+    delete changeset_tags_path(999111, :base64_key => Base64.urlsafe_encode64("nope"))
     assert_response :not_found
   end
 
@@ -124,15 +155,17 @@ class ChangesetTagsControllerTest < ActionDispatch::IntegrationTest
 
     session_for(moderator_user)
 
-    delete changeset_tags_path(changeset, :key => "nope")
+    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, :key => "tested-tag-key")
+    delete changeset_tags_path(changeset, :base64_key => Base64.urlsafe_encode64("tested-tag-key"))
     assert_response :forbidden
   end
 
@@ -143,7 +176,7 @@ class ChangesetTagsControllerTest < ActionDispatch::IntegrationTest
 
     session_for(user)
 
-    delete changeset_tags_path(changeset, :key => "tested-tag-key")
+    delete changeset_tags_path(changeset, :base64_key => Base64.urlsafe_encode64("tested-tag-key"))
     assert_redirected_to :controller => :errors, :action => :forbidden
   end
 
@@ -153,8 +186,8 @@ class ChangesetTagsControllerTest < ActionDispatch::IntegrationTest
     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='key']" do
-        assert_dom "> @value", key
+      assert_dom "input[type='hidden'][name='base64_key']" do
+        assert_dom "> @value", Base64.urlsafe_encode64(key)
       end
       assert_dom "button", :text => "Delete"
     end

From 071563add0016dd916b6a9a681b174ea1b89232d Mon Sep 17 00:00:00 2001
From: Anton Khorev <tony29@yandex.ru>
Date: Thu, 7 Nov 2024 19:44:28 +0300
Subject: [PATCH 10/12] Handle incorrectly encoded tag keys

---
 app/controllers/changeset_tags_controller.rb       |  5 +++++
 app/views/changeset_tags/invalid_tag.html.erb      |  7 +++++++
 config/locales/en.yml                              |  3 +++
 test/controllers/changeset_tags_controller_test.rb | 10 ++++++++++
 4 files changed, 25 insertions(+)
 create mode 100644 app/views/changeset_tags/invalid_tag.html.erb

diff --git a/app/controllers/changeset_tags_controller.rb b/app/controllers/changeset_tags_controller.rb
index 2cd2a1853..662b1ea52 100644
--- a/app/controllers/changeset_tags_controller.rb
+++ b/app/controllers/changeset_tags_controller.rb
@@ -22,6 +22,11 @@ class ChangesetTagsController < ApplicationController
     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
diff --git a/app/views/changeset_tags/invalid_tag.html.erb b/app/views/changeset_tags/invalid_tag.html.erb
new file mode 100644
index 000000000..190b1a3cb
--- /dev/null
+++ b/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>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 6f4a8fbee..7d7dfdd95 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -483,6 +483,9 @@ en:
     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}."
diff --git a/test/controllers/changeset_tags_controller_test.rb b/test/controllers/changeset_tags_controller_test.rb
index e8bdc0d53..a63ce4165 100644
--- a/test/controllers/changeset_tags_controller_test.rb
+++ b/test/controllers/changeset_tags_controller_test.rb
@@ -149,6 +149,16 @@ class ChangesetTagsControllerTest < ActionDispatch::IntegrationTest
     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)

From 8294fd46f016d959b92dbb7297082d9c64b77290 Mon Sep 17 00:00:00 2001
From: Anton Khorev <tony29@yandex.ru>
Date: Sat, 9 Nov 2024 13:37:39 +0300
Subject: [PATCH 11/12] Add 'tag deleted successfully' flash message

---
 app/controllers/changeset_tags_controller.rb       | 1 +
 config/locales/en.yml                              | 2 ++
 test/controllers/changeset_tags_controller_test.rb | 1 +
 3 files changed, 4 insertions(+)

diff --git a/app/controllers/changeset_tags_controller.rb b/app/controllers/changeset_tags_controller.rb
index 662b1ea52..c8fefbb10 100644
--- a/app/controllers/changeset_tags_controller.rb
+++ b/app/controllers/changeset_tags_controller.rb
@@ -34,6 +34,7 @@ class ChangesetTagsController < ApplicationController
     end
 
     @changeset_tag.delete
+    flash[:notice] = t ".success", :k => @changeset_tag.k, :v => @changeset_tag.v
     redirect_to changeset_tags_path(@changeset)
   end
 end
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 7d7dfdd95..7e7c55939 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -489,6 +489,8 @@ en:
     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:
       no_edits: "(no edits)"
diff --git a/test/controllers/changeset_tags_controller_test.rb b/test/controllers/changeset_tags_controller_test.rb
index a63ce4165..f9b3eca4c 100644
--- a/test/controllers/changeset_tags_controller_test.rb
+++ b/test/controllers/changeset_tags_controller_test.rb
@@ -124,6 +124,7 @@ class ChangesetTagsControllerTest < ActionDispatch::IntegrationTest
       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

From 2b5af638951c46c331ef7dff87dd05b1f32f792f Mon Sep 17 00:00:00 2001
From: Anton Khorev <tony29@yandex.ru>
Date: Sat, 9 Nov 2024 14:04:30 +0300
Subject: [PATCH 12/12] Add reminder about commenting after changing tags

---
 app/views/changeset_tags/show.html.erb | 4 ++++
 config/locales/en.yml                  | 1 +
 2 files changed, 5 insertions(+)

diff --git a/app/views/changeset_tags/show.html.erb b/app/views/changeset_tags/show.html.erb
index 8983d7c2c..6d9f35f19 100644
--- a/app/views/changeset_tags/show.html.erb
+++ b/app/views/changeset_tags/show.html.erb
@@ -2,6 +2,10 @@
   <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>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 7e7c55939..e9d4125c6 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -477,6 +477,7 @@ en:
     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} ?"