Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete changeset tags #5309

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions app/abilities/ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def initialize(user)
can [:index, :create, :destroy], UserMute

if user.moderator?
can :manage, ChangesetTag
can [:hide, :unhide], [DiaryEntry, DiaryComment]
can [:index, :show, :resolve, :ignore, :reopen], Issue
can :create, IssueComment
Expand Down
40 changes: 40 additions & 0 deletions app/controllers/changeset_tags_controller.rb
Original file line number Diff line number Diff line change
@@ -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 index
@changeset = Changeset.find(params[:changeset_id])
rescue ActiveRecord::RecordNotFound
render :action => "changeset_not_found", :status => :not_found
end

def delete
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
11 changes: 11 additions & 0 deletions app/views/changeset_tags/_tag.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<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'>
<%= form_tag delete_changeset_tags_path(@changeset) do %>
<%= hidden_field_tag "base64_key", Base64.urlsafe_encode64(tag[0]) %>
<%= button_tag t(".delete"),
:data => { :confirm => t(".confirm_delete", :k => tag[0], :v => tag[1]) },
:class => "btn btn-sm btn-danger" %>
<% end %>
</tr>
7 changes: 7 additions & 0 deletions app/views/changeset_tags/changeset_not_found.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<% content_for :heading do %>
<h1><%= t(".heading") %></h1>
<% end %>

<div>
<p><%= t ".body", :id => params[:changeset_id] %>
</div>
19 changes: 19 additions & 0 deletions app/views/changeset_tags/index.html.erb
Original file line number Diff line number Diff line change
@@ -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 %>
7 changes: 7 additions & 0 deletions app/views/changeset_tags/invalid_tag.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<% content_for :heading do %>
<h1><%= t(".heading") %></h1>
<% end %>

<div>
<p><%= t ".body" %>
</div>
7 changes: 7 additions & 0 deletions app/views/changeset_tags/tag_not_found.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<% content_for :heading do %>
<h1><%= t(".heading") %></h1>
<% end %>

<div>
<p><%= t ".body", :id => params[:changeset_id], :k => @key %>
</div>
8 changes: 6 additions & 2 deletions app/views/changesets/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,13 @@
</div>

<div class='secondary-actions'>
<%= link_to(t(".changesetxml"), :controller => "api/changesets", :action => "show") %>
<%= link_to t(".changesetxml"), { :controller => "api/changesets", :action => "show" }, :class => "text-nowrap" %>
&middot;
<%= link_to(t(".osmchangexml"), :controller => "api/changesets", :action => "download") %>
<%= link_to t(".osmchangexml"), { :controller => "api/changesets", :action => "download" }, :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 %>
Expand Down
20 changes: 20 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,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:
index:
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}."
delete:
success: Tag %{k}=%{v} deleted successfully.
changesets:
changeset:
no_edits: "(no edits)"
Expand Down Expand Up @@ -492,6 +511,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})"
Expand Down
6 changes: 6 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,12 @@
namespace :changeset_comments, :as => :comments, :path => :comments do
resource :feed, :only => :show, :defaults => { :format => "rss" }
end

resources :tags, :controller => "changeset_tags", :only => :index do
collection do
post "delete"
end
end
end
resources :notes, :path => "note", :id => /\d+/, :only => [:show, :new]

Expand Down
206 changes: 206 additions & 0 deletions test/controllers/changeset_tags_controller_test.rb
Original file line number Diff line number Diff line change
@@ -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 => "index", :changeset_id => "1" }
)
assert_routing(
{ :path => "/changeset/1/tags/delete", :method => :post },
{ :controller => "changeset_tags", :action => "delete", :changeset_id => "1" }
)
end

def test_index_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_index_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_index_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_index_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_index_fail_no_changeset
moderator_user = create(:moderator_user)

session_for(moderator_user)

get changeset_tags_path(999111)
assert_response :not_found
end

def test_index_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_index_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_delete_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
post delete_changeset_tags_path(changeset, :params => { :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_delete_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
post delete_changeset_tags_path(changeset, :params => { :base64_key => Base64.urlsafe_encode64("") })
assert_redirected_to changeset_tags_path(changeset)
end
assert_empty changeset.tags
end

def test_delete_fail_no_changeset
moderator_user = create(:moderator_user)

session_for(moderator_user)

post delete_changeset_tags_path(999111, :params => { :base64_key => Base64.urlsafe_encode64("nope") })
assert_response :not_found
end

def test_delete_fail_invalid_key_encoding
changeset = create(:changeset)
moderator_user = create(:moderator_user)

session_for(moderator_user)

post delete_changeset_tags_path(changeset, :params => { :base64_key => "ZnJvbV9jb" })
assert_response :not_found
end

def test_delete_fail_no_key
changeset = create(:changeset)
moderator_user = create(:moderator_user)

session_for(moderator_user)

post delete_changeset_tags_path(changeset, :params => { :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_delete_fail_not_logged_in
changeset = create(:changeset)
create(:changeset_tag, :changeset => changeset, :k => "tested-tag-key", :v => "tested-tag-value")

post delete_changeset_tags_path(changeset, :params => { :base64_key => Base64.urlsafe_encode64("tested-tag-key") })
assert_response :forbidden
end

def test_delete_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)

post delete_changeset_tags_path(changeset, :params => { :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='#{delete_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
Loading