diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index bd3ea28bb..545fd837f 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..c8fefbb10 --- /dev/null +++ b/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 diff --git a/app/views/changeset_tags/_tag.html.erb b/app/views/changeset_tags/_tag.html.erb new file mode 100644 index 000000000..94747c430 --- /dev/null +++ b/app/views/changeset_tags/_tag.html.erb @@ -0,0 +1,12 @@ + + <%= format_key(tag[0]) %> + <%= format_value(tag[0], tag[1]) %> + + <%= 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" %> + + diff --git a/app/views/changeset_tags/changeset_not_found.html.erb b/app/views/changeset_tags/changeset_not_found.html.erb new file mode 100644 index 000000000..26e801b8b --- /dev/null +++ b/app/views/changeset_tags/changeset_not_found.html.erb @@ -0,0 +1,7 @@ +<% content_for :heading do %> +

<%= t(".heading") %>

+<% end %> + +
+

<%= t ".body", :id => params[:changeset_id] %> +

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 %> +

<%= t(".heading") %>

+<% end %> + +
+

<%= t ".body" %> +

diff --git a/app/views/changeset_tags/show.html.erb b/app/views/changeset_tags/show.html.erb new file mode 100644 index 000000000..6d9f35f19 --- /dev/null +++ b/app/views/changeset_tags/show.html.erb @@ -0,0 +1,19 @@ +<% content_for :heading do %> +

<%= t ".heading" %>

+<% end %> + +
+ <%= t ".comment_warning" %> +
+ +

<%= t ".changeset_html", :id => link_to(@changeset.id, @changeset) %>

+ +

<%= changeset_details(@changeset) %>

+ +<% unless @changeset.tags.empty? %> +
+ + <%= render :partial => "tag", :collection => @changeset.tags.sort %> +
+
+<% end %> 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..8d50e4c28 --- /dev/null +++ b/app/views/changeset_tags/tag_not_found.html.erb @@ -0,0 +1,7 @@ +<% content_for :heading do %> +

<%= t(".heading") %>

+<% end %> + +
+

<%= t ".body", :id => params[:changeset_id], :k => @key %> +

diff --git a/app/views/changesets/show.html.erb b/app/views/changesets/show.html.erb index 3f1aeb201..481bbbe5f 100644 --- a/app/views/changesets/show.html.erb +++ b/app/views/changesets/show.html.erb @@ -111,9 +111,13 @@
- <%= link_to t(".changesetxml"), api_changeset_path(@changeset) %> + <%= link_to t(".changesetxml"), api_changeset_path(@changeset), :class => "text-nowrap" %> · - <%= 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 %> + · + <%= link_to t(".manage_tags"), changeset_tags_path(@changeset), :class => "text-nowrap" %> + <% end %>
<% if @next_by_user || @prev_by_user %> diff --git a/config/locales/en.yml b/config/locales/en.yml index bc2f3b1b3..4ac6b9988 100644 --- a/config/locales/en.yml +++ b/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})" diff --git a/config/routes.rb b/config/routes.rb index ec78f6ffd..ace5231de 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, :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 new file mode 100644 index 000000000..f9b3eca4c --- /dev/null +++ b/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 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