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-5520 Admin roles restrict ability to add or modify wrangling guidelines #4398

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions app/controllers/wrangling_guidelines_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,25 @@ def show
# GET /wrangling_guidelines/new
def new
@wrangling_guideline = WranglingGuideline.new
authorize @wrangling_guideline
Copy link
Collaborator

Choose a reason for hiding this comment

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

Anywhere we're doing @wrangling_guideline = WranglingGuideline.new or @wrangling_guideline = WranglingGuideline.find, we should be able to just stick authorize in front of WranglingGuideline to make things a bit neater.

end

# GET /wrangling_guidelines/1/edit
def edit
@wrangling_guideline = WranglingGuideline.find(params[:id])
authorize @wrangling_guideline
end

# GET /wrangling_guidelines/manage
def manage
authorize WranglingGuideline
@wrangling_guidelines = WranglingGuideline.order('position ASC')
end

# POST /wrangling_guidelines
def create
@wrangling_guideline = WranglingGuideline.new(wrangling_guideline_params)
authorize @wrangling_guideline

if @wrangling_guideline.save
flash[:notice] = ts('Wrangling Guideline was successfully created.')
Expand All @@ -41,6 +45,7 @@ def create
# PUT /wrangling_guidelines/1
def update
@wrangling_guideline = WranglingGuideline.find(params[:id])
authorize @wrangling_guideline

if @wrangling_guideline.update(wrangling_guideline_params)
flash[:notice] = ts('Wrangling Guideline was successfully updated.')
Expand All @@ -52,6 +57,7 @@ def update

# reorder FAQs
def update_positions
authorize WranglingGuideline
if params[:wrangling_guidelines]
@wrangling_guidelines = WranglingGuideline.reorder_list(params[:wrangling_guidelines])
flash[:notice] = ts('Wrangling Guidelines order was successfully updated.')
Expand All @@ -62,6 +68,7 @@ def update_positions
# DELETE /wrangling_guidelines/1
def destroy
@wrangling_guideline = WranglingGuideline.find(params[:id])
authorize @wrangling_guideline
@wrangling_guideline.destroy
flash[:notice] = ts('Wrangling Guideline was successfully deleted.')
redirect_to(wrangling_guidelines_path)
Expand Down
14 changes: 14 additions & 0 deletions app/policies/wrangling_guideline_policy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
class WranglingGuidelinePolicy < ApplicationPolicy
MANAGE_WRANGLING_GUIDELINE = %w[superadmin tag_wrangling].freeze

def new?
user_has_roles?(MANAGE_WRANGLING_GUIDELINE)
end

alias edit? new?
alias manage? new?
alias create? new?
alias update? new?
alias update_positions? new?
alias destroy? new?
end
8 changes: 6 additions & 2 deletions app/views/admin/_header.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
<% end %>
<li><%= link_to ts("Archive FAQ", key: "header"), archive_faqs_path %></li>
<li><%= link_to ts("Known Issues", key: "header"), known_issues_path %></li>
<li><%= link_to ts("Wrangling Guidelines", key: "header"), wrangling_guidelines_path %></li>
<% if policy(WranglingGuideline).new? %>
<li><%= link_to ts("Wrangling Guidelines", key: "header"), wrangling_guidelines_path %></li>
<% end %>
</ul>
</li>
<% if policy(AdminBlacklistedEmail).index? %>
Expand Down Expand Up @@ -58,7 +60,9 @@
</ul>
</li>
<% end %>
<li><%= link_to ts("Tag Wrangling", key: "header"), tag_wranglings_path %></li>
<% if policy(WranglingGuideline).new? %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

We're not actually restricting Tag Wrangling in this PR, so let's leave the Tag Wrangling link as-is.

<li><%= link_to ts("Tag Wrangling", key: "header"), tag_wranglings_path %></li>
<% end %>
<li><%= link_to ts("Locales", key: "header"), locales_path %></li>

<% if policy(AdminActivity).index? %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/wrangling_guidelines/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<!--Descriptive page name, messages and instructions-->
<% if logged_in_as_admin? %>
<% if policy(WranglingGuideline).new? %>
<%= render 'admin_index' %>
<% else %>
<h2 class="heading"><%= ts('Wrangling Guidelines') %></h2>
Expand Down
2 changes: 1 addition & 1 deletion app/views/wrangling_guidelines/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

<!--main content-->
<div class="admin" role="article">
<% if logged_in_as_admin? %>
<% if policy(WranglingGuideline).new? %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we're deciding whether to display the edit link, let's use policy(WranglingGuideline).edit? instead.

<div class="header">
<h3 class="heading">
Updated: <%=h @wrangling_guideline.updated_at %> | <%= link_to ts('Edit'), edit_wrangling_guideline_path(@wrangling_guideline) %>
Expand Down
2 changes: 1 addition & 1 deletion features/step_definitions/tag_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@
end

Given /^I have posted a Wrangling Guideline?(?: titled "([^\"]*)")?$/ do |title|
step %{I am logged in as an admin}
step %{I am logged in as a "tag_wrangling" admin}
visit new_wrangling_guideline_path
if title
fill_in("Guideline text", with: "This is a page about how we wrangle things.")
Expand Down
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
6 changes: 3 additions & 3 deletions features/tags_and_wrangling/wrangling_guidelines.feature
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Feature: Wrangling Guidelines

Scenario: Post a Wrangling Guideline

Given I am logged in as an admin
Given I am logged in as a "tag_wrangling" admin
And I am on the wrangling guidelines page
And I follow "New Wrangling Guideline"
And I fill in "Guideline text" with "This series of documents (Wrangling Guidelines) are intended to help tag wranglers remain consistent as they go about the business of wrangling tags by providing a set of formatting guidelines."
Expand All @@ -29,7 +29,7 @@ Feature: Wrangling Guidelines

Scenario: Reorder Wrangling Guidelines

Given I am logged in as an admin
Given I am logged in as a "tag_wrangling" admin
And 3 Wrangling Guidelines exist
When I go to the Wrangling Guidelines reorder page
And I fill in "wrangling_guidelines_1" with "3"
Expand All @@ -44,7 +44,7 @@ Feature: Wrangling Guidelines

Scenario: Delete Wrangling Guideline

Given I am logged in as an admin
Given I am logged in as a "tag_wrangling" admin
And I have posted a Wrangling Guideline titled "Relationship Tags"
When I go to the Wrangling Guidelines page
And I follow "Delete"
Expand Down
Loading