From 4a36b99614a90030eb690ef277f29f9f7d3240ce Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 17 Mar 2020 17:42:29 -0500 Subject: [PATCH] pundit: convert tag aliases / implications to pundit. --- app/controllers/tag_aliases_controller.rb | 13 +++-------- .../tag_implications_controller.rb | 13 +++-------- app/models/tag_relationship.rb | 4 ---- app/policies/tag_alias_policy.rb | 5 +++++ app/policies/tag_implication_policy.rb | 5 +++++ app/views/tag_aliases/_listing.html.erb | 2 +- app/views/tag_implications/_listing.html.erb | 2 +- .../functional/tag_aliases_controller_test.rb | 22 ++++++++++++------- .../tag_implications_controller_test.rb | 18 ++++++++++----- 9 files changed, 44 insertions(+), 40 deletions(-) create mode 100644 app/policies/tag_alias_policy.rb create mode 100644 app/policies/tag_implication_policy.rb diff --git a/app/controllers/tag_aliases_controller.rb b/app/controllers/tag_aliases_controller.rb index 276502f1b..6baf30a5e 100644 --- a/app/controllers/tag_aliases_controller.rb +++ b/app/controllers/tag_aliases_controller.rb @@ -1,29 +1,22 @@ class TagAliasesController < ApplicationController - before_action :admin_only, only: [:destroy] respond_to :html, :xml, :json, :js def show - @tag_alias = TagAlias.find(params[:id]) + @tag_alias = authorize TagAlias.find(params[:id]) respond_with(@tag_alias) end def index - @tag_aliases = TagAlias.paginated_search(params, count_pages: true) + @tag_aliases = authorize TagAlias.paginated_search(params, count_pages: true) @tag_aliases = @tag_aliases.includes(:antecedent_tag, :consequent_tag, :approver) if request.format.html? respond_with(@tag_aliases) end def destroy - @tag_alias = TagAlias.find(params[:id]) + @tag_alias = authorize TagAlias.find(params[:id]) @tag_alias.reject! respond_with(@tag_alias, location: tag_aliases_path, notice: "Tag alias was deleted") end - - private - - def tag_alias_params - params.require(:tag_alias).permit(%i[antecedent_name consequent_name forum_topic_id skip_secondary_validations]) - end end diff --git a/app/controllers/tag_implications_controller.rb b/app/controllers/tag_implications_controller.rb index 76c2f1fe2..02264b92a 100644 --- a/app/controllers/tag_implications_controller.rb +++ b/app/controllers/tag_implications_controller.rb @@ -1,29 +1,22 @@ class TagImplicationsController < ApplicationController - before_action :admin_only, only: [:destroy] respond_to :html, :xml, :json, :js def show - @tag_implication = TagImplication.find(params[:id]) + @tag_implication = authorize TagImplication.find(params[:id]) respond_with(@tag_implication) end def index - @tag_implications = TagImplication.paginated_search(params, count_pages: true) + @tag_implications = authorize TagImplication.paginated_search(params, count_pages: true) @tag_implications = @tag_implications.includes(:antecedent_tag, :consequent_tag, :approver) if request.format.html? respond_with(@tag_implications) end def destroy - @tag_implication = TagImplication.find(params[:id]) + @tag_implication = authorize TagImplication.find(params[:id]) @tag_implication.reject! respond_with(@tag_implication, location: tag_implications_path, notice: "Tag implication was deleted") end - - private - - def tag_implication_params - params.require(:tag_implication).permit(%i[antecedent_name consequent_name forum_topic_id skip_secondary_validations]) - end end diff --git a/app/models/tag_relationship.rb b/app/models/tag_relationship.rb index ee14ecce6..a463deee5 100644 --- a/app/models/tag_relationship.rb +++ b/app/models/tag_relationship.rb @@ -64,10 +64,6 @@ class TagRelationship < ApplicationRecord status =~ /\Aerror:/ end - def deletable_by?(user) - user.is_admin? - end - def reject! update!(status: "deleted") end diff --git a/app/policies/tag_alias_policy.rb b/app/policies/tag_alias_policy.rb new file mode 100644 index 000000000..15ed6acb3 --- /dev/null +++ b/app/policies/tag_alias_policy.rb @@ -0,0 +1,5 @@ +class TagAliasPolicy < ApplicationPolicy + def destroy? + user.is_admin? + end +end diff --git a/app/policies/tag_implication_policy.rb b/app/policies/tag_implication_policy.rb new file mode 100644 index 000000000..e46a89fce --- /dev/null +++ b/app/policies/tag_implication_policy.rb @@ -0,0 +1,5 @@ +class TagImplicationPolicy < ApplicationPolicy + def destroy? + user.is_admin? + end +end diff --git a/app/views/tag_aliases/_listing.html.erb b/app/views/tag_aliases/_listing.html.erb index 21388ab73..277f1fbad 100644 --- a/app/views/tag_aliases/_listing.html.erb +++ b/app/views/tag_aliases/_listing.html.erb @@ -23,7 +23,7 @@ <% t.column column: "control", width: "15%" do |tag_alias| %> <%= link_to "Show", tag_alias_path(tag_alias) %> - <% if tag_alias.deletable_by?(CurrentUser.user) %> + <% if policy(tag_alias).destroy? %> | <%= link_to "Delete", tag_alias_path(tag_alias), :remote => true, :method => :delete, :data => {:confirm => "Are you sure you want to delete this alias?"} %> <% end %> <% end %> diff --git a/app/views/tag_implications/_listing.html.erb b/app/views/tag_implications/_listing.html.erb index 457818e8a..48952f928 100644 --- a/app/views/tag_implications/_listing.html.erb +++ b/app/views/tag_implications/_listing.html.erb @@ -23,7 +23,7 @@ <% t.column column: "control", width: "15%" do |tag_implication| %> <%= link_to "Show", tag_implication_path(tag_implication) %> - <% if tag_implication.deletable_by?(CurrentUser.user) %> + <% if policy(tag_implication).destroy? %> | <%= link_to "Delete", tag_implication_path(tag_implication), :remote => true, :method => :delete, :data => {:confirm => "Are you sure you want to delete this implication?"} %> <% end %> <% end %> diff --git a/test/functional/tag_aliases_controller_test.rb b/test/functional/tag_aliases_controller_test.rb index 540e253eb..1d7e52d08 100644 --- a/test/functional/tag_aliases_controller_test.rb +++ b/test/functional/tag_aliases_controller_test.rb @@ -3,28 +3,34 @@ require 'test_helper' class TagAliasesControllerTest < ActionDispatch::IntegrationTest context "The tag aliases controller" do setup do - @user = create(:admin_user) @tag_alias = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb") end context "index action" do should "list all tag alias" do - get_auth tag_aliases_path, @user + get tag_aliases_path assert_response :success end should "list all tag_alias (with search)" do - get_auth tag_aliases_path, @user, params: {:search => {:antecedent_name => "aaa"}} + get tag_aliases_path, params: {:search => {:antecedent_name => "aaa"}} assert_response :success end end context "destroy action" do - should "mark the alias as deleted" do - assert_difference("TagAlias.count", 0) do - delete_auth tag_alias_path(@tag_alias), @user - assert_equal("deleted", @tag_alias.reload.status) - end + should "allow admins to delete aliases" do + delete_auth tag_alias_path(@tag_alias), create(:admin_user) + + assert_response :redirect + assert_equal("deleted", @tag_alias.reload.status) + end + + should "not allow members to delete aliases" do + delete_auth tag_alias_path(@tag_alias), create(:user) + + assert_response 403 + assert_equal("active", @tag_alias.reload.status) end end end diff --git a/test/functional/tag_implications_controller_test.rb b/test/functional/tag_implications_controller_test.rb index 29dd4dd05..e3d66e8f8 100644 --- a/test/functional/tag_implications_controller_test.rb +++ b/test/functional/tag_implications_controller_test.rb @@ -3,7 +3,6 @@ require 'test_helper' class TagImplicationsControllerTest < ActionDispatch::IntegrationTest context "The tag implications controller" do setup do - @user = create(:admin_user) @tag_implication = create(:tag_implication, antecedent_name: "aaa", consequent_name: "bbb") end @@ -20,11 +19,18 @@ class TagImplicationsControllerTest < ActionDispatch::IntegrationTest end context "destroy action" do - should "mark the implication as deleted" do - assert_difference("TagImplication.count", 0) do - delete_auth tag_implication_path(@tag_implication), @user - assert_equal("deleted", @tag_implication.reload.status) - end + should "allow admins to delete implications" do + delete_auth tag_implication_path(@tag_implication), create(:admin_user) + + assert_response :redirect + assert_equal("deleted", @tag_implication.reload.status) + end + + should "not allow members to delete aliases" do + delete_auth tag_implication_path(@tag_implication), create(:user) + + assert_response 403 + assert_equal("active", @tag_implication.reload.status) end end end