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

AO3-6761 Check admin roles in TagWranglingsController #4937

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
7 changes: 6 additions & 1 deletion app/controllers/tag_wranglings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ class TagWranglingsController < ApplicationController

def index
@counts = tag_counts_per_category
unless params[:show].blank?
if params[:show].blank?
authorize :wrangling, :full_access? if logged_in_as_admin?
else
authorize :wrangling, :read_access? if logged_in_as_admin?
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved
raise "Redshirt: Attempted to constantize invalid class initialize tag_wranglings_controller_index #{params[:show].classify}" unless Tag::USER_DEFINED.include?(params[:show].classify)

params[:sort_column] = 'created_at' if !valid_sort_column(params[:sort_column], 'tag')
Expand All @@ -34,6 +37,8 @@ def index
end

def wrangle
authorize :wrangling, :full_access? if logged_in_as_admin?

params[:page] = '1' if params[:page].blank?
params[:sort_column] = 'name' if !valid_sort_column(params[:sort_column], 'tag')
params[:sort_direction] = 'ASC' if !valid_sort_direction(params[:sort_direction])
Expand Down
5 changes: 5 additions & 0 deletions app/policies/wrangling_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,16 @@

class WranglingPolicy < ApplicationPolicy
FULL_ACCESS_ROLES = %w[superadmin tag_wrangling].freeze
READ_ACCESS_ROLES = (FULL_ACCESS_ROLES + %w[policy_and_abuse]).freeze

def full_access?
user_has_roles?(FULL_ACCESS_ROLES)
end

def read_access?
user_has_roles?(READ_ACCESS_ROLES)
end

alias create? full_access?
alias destroy? full_access?
alias show? full_access?
Expand Down
5 changes: 3 additions & 2 deletions app/views/admin/_header.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,9 @@
</ul>
</li>
<% end %>
<li><%= link_to t(".nav.wrangling"), tag_wranglings_path %></li>

<% if policy(:wrangling).read_access? %>
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved
<li><%= link_to t(".nav.wrangling"), tag_wranglings_path %></li>
<% end %>
<% if policy(Locale).index? %>
<li><%= link_to t(".nav.locales"), locales_path %></li>
<% end %>
Expand Down
74 changes: 39 additions & 35 deletions app/views/tag_wranglings/_wrangler_dashboard.html.erb
Original file line number Diff line number Diff line change
@@ -1,39 +1,43 @@
<div id="dashboard" role="navigation region" class="tag wrangling region">
<ul class="navigation actions">
<% if current_user.is_a?(User) %>
<li><%= span_if_current(ts('Wrangling Home'), tag_wrangler_path(current_user)) %></li>
<% if (logged_in_as_admin? && policy(:wrangling).full_access?) || current_user.try(:is_tag_wrangler?) || @counts %>
Copy link
Contributor

@Bilka2 Bilka2 Nov 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 I thought this needed to be read_access? as well but the tests pass without it? Maybe still good to change though so it's not confusing?

Copy link
Contributor

@Bilka2 Bilka2 Nov 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, it doesn't show on the tags index page, because that one doesn't have @counts set. So this should be changed and the tests should all start out in the tags index, not the tag wrangling tools

<div id="dashboard" role="navigation region" class="tag wrangling region">
<% if (logged_in_as_admin? && policy(:wrangling).full_access?) || current_user.try(:is_tag_wrangler?) %>
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved
<ul class="navigation actions">
<% if current_user.is_a?(User) %>
<li><%= span_if_current(ts('Wrangling Home'), tag_wrangler_path(current_user)) %></li>

Check failure on line 6 in app/views/tag_wranglings/_wrangler_dashboard.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner

[] reported by reviewdog 🐶 Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. Raw Output: app/views/tag_wranglings/_wrangler_dashboard.html.erb:6:37: Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
<% end %>
<li><%= span_if_current(ts('Wrangling Tools'), tag_wranglings_path, current_page?(tag_wranglings_path) && params[:show].blank?) %></li>

Check failure on line 8 in app/views/tag_wranglings/_wrangler_dashboard.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner

[] reported by reviewdog 🐶 Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. Raw Output: app/views/tag_wranglings/_wrangler_dashboard.html.erb:8:35: Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
<li><%= span_if_current(ts('Wranglers'), tag_wranglers_path) %></li>

Check failure on line 9 in app/views/tag_wranglings/_wrangler_dashboard.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner

[] reported by reviewdog 🐶 Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. Raw Output: app/views/tag_wranglings/_wrangler_dashboard.html.erb:9:35: Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
<li><%= span_if_current(ts('Search Tags'), search_tags_path) %></li>

Check failure on line 10 in app/views/tag_wranglings/_wrangler_dashboard.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner

[] reported by reviewdog 🐶 Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. Raw Output: app/views/tag_wranglings/_wrangler_dashboard.html.erb:10:35: Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
<li><%= span_if_current(ts('New Tag'), new_tag_path) %></li>

Check failure on line 11 in app/views/tag_wranglings/_wrangler_dashboard.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner

[] reported by reviewdog 🐶 Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. Raw Output: app/views/tag_wranglings/_wrangler_dashboard.html.erb:11:35: Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
</ul>
<% end %>
<li><%= span_if_current(ts('Wrangling Tools'), tag_wranglings_path) %></li>
<li><%= span_if_current(ts('Wranglers'), tag_wranglers_path) %></li>
<li><%= span_if_current(ts('Search Tags'), search_tags_path) %></li>
<li><%= span_if_current(ts('New Tag'), new_tag_path) %></li>
</ul>

<% if @counts %>
<ul class="navigation actions">
<% if @tag && @uses %>
<% @uses.each do |key| %>
<% if key == 'Works' || key == 'Bookmarks' %>
<li><%= span_if_current "#{key} (#{@counts[key]})", { controller: key.downcase.to_sym, action: :index, tag_id: @tag } %></li>
<% elsif key == 'External Works' %>
<li><%= span_if_current "#{key} (#{@counts[key]})", { controller: :bookmarks, action: :index, tag_id: @tag } %></li>
<% else %>
<li><span><%= "#{key} (#{@counts[key]})" %></span></li>
<% if @counts %>
<ul class="navigation actions">
<% if @tag && @uses %>
<% @uses.each do |key| %>
<% if key == 'Works' || key == 'Bookmarks' %>
<li><%= span_if_current "#{key} (#{@counts[key]})", { controller: key.downcase.to_sym, action: :index, tag_id: @tag } %></li>
<% elsif key == 'External Works' %>
<li><%= span_if_current "#{key} (#{@counts[key]})", { controller: :bookmarks, action: :index, tag_id: @tag } %></li>
<% else %>
<li><span><%= "#{key} (#{@counts[key]})" %></span></li>
<% end %>
<% end %>
<% elsif @tag && @tag.child_types %>
<% @tag.child_types.each do |tag_type| %>
<li>
<%= span_if_current tag_type.pluralize + " (#{@counts[tag_type.underscore.pluralize.to_sym]})", url_for(show: tag_type.underscore.pluralize, id: @tag) %>
</li>
<% end %>
<% else %>
<li><%= span_if_current ts("Fandoms by media (%{count})", count: @counts[:fandoms]), tag_wranglings_path(show: "fandoms") %></li>

Check failure on line 34 in app/views/tag_wranglings/_wrangler_dashboard.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner

[] reported by reviewdog 🐶 I18n/DeprecatedHelper: Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards Raw Output: app/views/tag_wranglings/_wrangler_dashboard.html.erb:34:34: I18n/DeprecatedHelper: Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards
<li><%= span_if_current ts("Characters by fandom (%{count})", count: @counts[:characters]), tag_wranglings_path(show: "characters") %></li>

Check failure on line 35 in app/views/tag_wranglings/_wrangler_dashboard.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner

[] reported by reviewdog 🐶 I18n/DeprecatedHelper: Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards Raw Output: app/views/tag_wranglings/_wrangler_dashboard.html.erb:35:34: I18n/DeprecatedHelper: Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards
<li><%= span_if_current ts("Relationships by fandom (%{count})", count: @counts[:relationships]), tag_wranglings_path(show: "relationships") %></li>

Check failure on line 36 in app/views/tag_wranglings/_wrangler_dashboard.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner

[] reported by reviewdog 🐶 I18n/DeprecatedHelper: Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards Raw Output: app/views/tag_wranglings/_wrangler_dashboard.html.erb:36:34: I18n/DeprecatedHelper: Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards
<li><%= span_if_current ts("Freeforms by fandom (%{count})", count: @counts[:freeforms]), tag_wranglings_path(show: "freeforms") %></li>

Check failure on line 37 in app/views/tag_wranglings/_wrangler_dashboard.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner

[] reported by reviewdog 🐶 I18n/DeprecatedHelper: Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards Raw Output: app/views/tag_wranglings/_wrangler_dashboard.html.erb:37:34: I18n/DeprecatedHelper: Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards
<li><%= span_if_current(ts("Unsorted Tags (%{count})", count: @counts[:UnsortedTag]), unsorted_tags_path) %></li>
<% end %>
<% elsif @tag && @tag.child_types %>
<% @tag.child_types.each do |tag_type| %>
<li>
<%= span_if_current tag_type.pluralize + " (#{@counts[tag_type.underscore.pluralize.to_sym]})", url_for(show: tag_type.underscore.pluralize, id: @tag) %>
</li>
<% end %>
<% else %>
<li><%= span_if_current ts("Fandoms by media (%{count})", count: @counts[:fandoms]), tag_wranglings_path(show: "fandoms") %></li>
<li><%= span_if_current ts("Characters by fandom (%{count})", count: @counts[:characters]), tag_wranglings_path(show: "characters") %></li>
<li><%= span_if_current ts("Relationships by fandom (%{count})", count: @counts[:relationships]), tag_wranglings_path(show: "relationships") %></li>
<li><%= span_if_current ts("Freeforms by fandom (%{count})", count: @counts[:freeforms]), tag_wranglings_path(show: "freeforms") %></li>
<li><%= span_if_current(ts("Unsorted Tags (%{count})", count: @counts[:UnsortedTag]), unsorted_tags_path) %></li>
<% end %>
</ul>
<% end %>
</div>
</ul>
<% end %>
</div>
<% end %>
39 changes: 39 additions & 0 deletions features/tags_and_wrangling/tag_wrangling_admin.feature
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,42 @@ Feature: Tag wrangling
Then I should see "Tags Wrangled (CSV)"
When I follow "Tags Wrangled (CSV)"
Then I should download a csv file with the header row "Name Last Updated Type Merger Fandoms Unwrangleable"

Scenario Outline: Authorized admins get the wrangling dashboard sidebar

Given I am logged in as a "<role>" admin
When I go to the wrangling tools page
Then I should see "Wrangling Tools" within "div#dashboard"
And I should see "Wranglers" within "div#dashboard"
And I should see "Search Tags" within "div#dashboard"
And I should see "New Tag" within "div#dashboard"
But I should not see "Wrangling Home" within "div#dashboard"

Examples:
| role |
| superadmin |
| tag_wrangling |

Scenario Outline: Unauthorized admins do not get the wrangling dashboard sidebar

Given I am logged in as a "<role>" admin
When I go to the wrangling tools page
Then I should not see "Wrangling Tools"
And I should not see "Wranglers"
And I should not see "Search Tags"
And I should not see "New Tag"
And I should not see "Wrangling Home"

Examples:
| role |
| board |
| board_assistants_team |
| communications |
| development_and_membership |
| docs |
| elections |
| legal |
| translation |
| support |
| policy_and_abuse |
| open_doors |
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Scenario: relationship wrangling - syns, mergers, characters, autocompletes
And a canonical character "Zoe Washburne"
And a canonical character "Jack Harkness"
And a canonical character "Ianto Jones"
And I am logged in as an admin
And I am logged in as a "tag_wrangling" admin
And I follow "Tag Wrangling"

# create a new canonical relationship from tag wrangling interface
Expand Down
122 changes: 104 additions & 18 deletions spec/controllers/tag_wranglings_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,60 +4,146 @@
include LoginMacros
include RedirectExpectationHelper

before do
fake_login
controller.current_user.roles << Role.new(name: "tag_wrangler")
full_access_roles = %w[superadmin tag_wrangling].freeze
read_access_roles = %w[superadmin policy_and_abuse tag_wrangling].freeze

shared_examples "an action only authorized admins can access" do |authorized_roles:|
before do
fake_login_admin(admin)
subject
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved
end

context "when logged in as an admin with no role" do
let(:admin) { create(:admin) }

it "redirects with an error" do
it_redirects_to_with_error(root_url, "Sorry, only an authorized admin can access the page you were trying to reach.")
end
end

(Admin::VALID_ROLES - authorized_roles).each do |admin_role|
context "when logged in as an admin with role #{admin_role}" do
let(:admin) { create(:admin, roles: [admin_role]) }

it "redirects with an error" do
it_redirects_to_with_error(root_url, "Sorry, only an authorized admin can access the page you were trying to reach.")
end
end
end

authorized_roles.each do |admin_role|
context "when logged in as an admin with role #{admin_role}" do
let(:admin) { create(:admin, roles: [admin_role]) }

it "succeeds" do
success
end
end
end
end

shared_examples "set last wrangling activity" do
it "sets the last wrangling activity time to now", :frozen do
user = controller.current_user
expect(user.last_wrangling_activity.updated_at).to eq(Time.now.utc)
describe "GET #index" do
let(:success) { expect(response).to have_http_status(:success) }

context "when the show parameter is absent" do
subject { get :index }

it_behaves_like "an action only authorized admins can access", authorized_roles: full_access_roles

context "when logged in as a tag wrangler" do
before do
fake_login_known_user(create(:tag_wrangler))
end

it "shows the wrangling tools page" do
subject
success
end
end
end

context "when the show parameter is present" do
subject { get :index, params: { show: "fandoms" } }

it_behaves_like "an action only authorized admins can access", authorized_roles: read_access_roles

context "when logged in as a tag wrangler" do
before do
fake_login_known_user(create(:tag_wrangler))
end

it "shows the wrangling tools page" do
subject
success
end
end
end
end

describe "#wrangle" do
let(:page_options) { { page: 1, sort_column: "name", sort_direction: "ASC" } }
describe "POST #wrangle" do
shared_examples "set last wrangling activity" do
before do
fake_login_known_user(create(:tag_wrangler))
subject
end

it "sets the last wrangling activity time to now", :frozen do
user = controller.current_user
expect(user.last_wrangling_activity.updated_at).to eq(Time.now.utc)
end
end

it "displays error when there are no fandoms to wrangle to" do
fake_login_known_user(create(:tag_wrangler))
character = create(:character)
page_options = { page: 1, sort_column: "name", sort_direction: "ASC" }
post :wrangle, params: { fandom_string: "", selected_tags: [character.id] }
it_redirects_to_with_error(tag_wranglings_path(page_options), "There were no Fandom tags!")
end

context "when making tags canonical" do
subject { post :wrangle, params: { canonicals: [tag1.id, tag2.id] } }
let(:tag1) { create(:character) }
let(:tag2) { create(:character) }
let(:success) do
expect(tag1.reload.canonical?).to be(true)
expect(tag2.reload.canonical?).to be(true)
end

before do
post :wrangle, params: { canonicals: [tag1.id, tag2.id] }
end
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved

include_examples "set last wrangling activity"
it_behaves_like "set last wrangling activity"
it_behaves_like "an action only authorized admins can access", authorized_roles: full_access_roles
end

context "when assigning tags to a medium" do
subject { post :wrangle, params: { media: medium.name, selected_tags: [fandom1.id, fandom2.id] } }
let(:fandom1) { create(:fandom, canonical: true) }
let(:fandom2) { create(:fandom, canonical: true) }
let(:medium) { create(:media) }

before do
post :wrangle, params: { media: medium.name, selected_tags: [fandom1.id, fandom2.id] }
let(:success) do
expect(fandom1.medias).to include(medium)
expect(fandom2.medias).to include(medium)
end

include_examples "set last wrangling activity"
it_behaves_like "set last wrangling activity"
it_behaves_like "an action only authorized admins can access", authorized_roles: full_access_roles
end

context "when adding tags to a fandom" do
subject { post :wrangle, params: { fandom_string: fandom.name, selected_tags: [tag1.id, tag2.id] } }
let(:tag1) { create(:character) }
let(:tag2) { create(:character) }
let(:fandom) { create(:fandom, canonical: true) }

before do
post :wrangle, params: { fandom_string: fandom.name, selected_tags: [tag1.id, tag2.id] }
let(:success) do
expect(tag1.fandoms).to include(fandom)
expect(tag2.fandoms).to include(fandom)
end

include_examples "set last wrangling activity"
it_behaves_like "set last wrangling activity"
it_behaves_like "an action only authorized admins can access", authorized_roles: full_access_roles
end
end
end
Loading