Skip to content

Commit

Permalink
AO3-6698 Limit roles that can modify archive FAQs (#4894)
Browse files Browse the repository at this point in the history
* AO3-6698 Add authorization to archive faqs controller

* AO3-6698 Forbid creation, reordering and deletion of non-English FAQ categories

* AO3-6698 Add authorization to archive faq questions controller

* AO3-6698 Hide links to unavailable FAQ actions

* AO3-6698 I18n and fix tests

* AO3-6698 Shut up rubocop

* AO3-6698 Fix flaky test

* AO3-6698 Review fixes

* AO3-6698 Fix tests
  • Loading branch information
Bilka2 authored Nov 28, 2024
1 parent 6ed2c5f commit 08eae80
Show file tree
Hide file tree
Showing 16 changed files with 596 additions and 66 deletions.
50 changes: 33 additions & 17 deletions app/controllers/archive_faqs_controller.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
class ArchiveFaqsController < ApplicationController

before_action :admin_only, except: [:index, :show]
before_action :set_locale
before_action :validate_locale, if: :logged_in_as_admin?
before_action :require_language_id
before_action :default_locale_only, only: [:new, :create, :manage, :update_positions, :confirm_delete, :destroy]
around_action :with_locale

# GET /archive_faqs
def index
@archive_faqs = ArchiveFaq.order('position ASC')
@archive_faqs = ArchiveFaq.order("position ASC")
unless logged_in_as_admin?
@archive_faqs = @archive_faqs.with_translations(I18n.locale)
end
Expand Down Expand Up @@ -40,6 +40,7 @@ def show
end

protected

def build_questions
notice = ""
num_to_build = params["num_questions"] ? params["num_questions"].to_i : @archive_faq.questions.count
Expand All @@ -59,9 +60,10 @@ def build_questions
end

public

# GET /archive_faqs/new
def new
@archive_faq = ArchiveFaq.new
@archive_faq = authorize ArchiveFaq.new
1.times { @archive_faq.questions.build(attributes: { question: "This is a temporary question", content: "This is temporary content", anchor: "ThisIsATemporaryAnchor"})}
respond_to do |format|
format.html # new.html.erb
Expand All @@ -70,32 +72,34 @@ def new

# GET /archive_faqs/1/edit
def edit
@archive_faq = ArchiveFaq.find_by(slug: params[:id])
@archive_faq = authorize ArchiveFaq.find_by(slug: params[:id])
authorize :archive_faq, :full_access? if default_locale?
build_questions
end

# GET /archive_faqs/manage
def manage
@archive_faqs = ArchiveFaq.order('position ASC')
@archive_faqs = authorize ArchiveFaq.order("position ASC")
end

# POST /archive_faqs
def create
@archive_faq = ArchiveFaq.new(archive_faq_params)
if @archive_faq.save
flash[:notice] = 'ArchiveFaq was successfully created.'
redirect_to(@archive_faq)
else
render action: "new"
end
@archive_faq = authorize ArchiveFaq.new(archive_faq_params)
if @archive_faq.save
flash[:notice] = t(".success")
redirect_to(@archive_faq)
else
render action: "new"
end
end

# PUT /archive_faqs/1
def update
@archive_faq = ArchiveFaq.find_by(slug: params[:id])
@archive_faq = authorize ArchiveFaq.find_by(slug: params[:id])
authorize :archive_faq, :full_access? if default_locale?

if @archive_faq.update(archive_faq_params)
flash[:notice] = 'ArchiveFaq was successfully updated.'
flash[:notice] = t(".success")
redirect_to(@archive_faq)
else
render action: "edit"
Expand All @@ -104,9 +108,10 @@ def update

# reorder FAQs
def update_positions
authorize :archive_faq
if params[:archive_faqs]
@archive_faqs = ArchiveFaq.reorder_list(params[:archive_faqs])
flash[:notice] = ts("Archive FAQs order was successfully updated.")
flash[:notice] = t(".success")
elsif params[:archive_faq]
params[:archive_faq].each_with_index do |id, position|
ArchiveFaq.update(id, position: position + 1)
Expand Down Expand Up @@ -149,25 +154,36 @@ def require_language_id
redirect_to url_for(request.query_parameters.merge(language_id: @i18n_locale.to_s))
end

def default_locale_only
return if default_locale?

flash[:error] = t("archive_faqs.default_locale_only")
redirect_to archive_faqs_path
end

# Setting I18n.locale directly is not thread safe
def with_locale
I18n.with_locale(@i18n_locale) { yield }
end

# GET /archive_faqs/1/confirm_delete
def confirm_delete
@archive_faq = ArchiveFaq.find_by(slug: params[:id])
@archive_faq = authorize ArchiveFaq.find_by(slug: params[:id])
end

# DELETE /archive_faqs/1
def destroy
@archive_faq = ArchiveFaq.find_by(slug: params[:id])
@archive_faq = authorize ArchiveFaq.find_by(slug: params[:id])
@archive_faq.destroy
redirect_to(archive_faqs_path)
end

private

def default_locale?
@i18n_locale.to_s == I18n.default_locale.to_s
end

def archive_faq_params
params.require(:archive_faq).permit(
:title,
Expand Down
17 changes: 9 additions & 8 deletions app/controllers/questions_controller.rb
Original file line number Diff line number Diff line change
@@ -1,33 +1,34 @@
class QuestionsController < ApplicationController
before_action :load_archive_faq, except: [:index, :update_positions]
before_action :load_archive_faq, except: :update_positions

# GET /archive_faq/:archive_faq_id/questions/manage
def manage
@questions = @archive_faq.questions.order('position')
authorize :archive_faq, :full_access?
@questions = @archive_faq.questions.order("position")
end

# fetch archive_faq these questions belong to from db
def load_archive_faq
@archive_faq = ArchiveFaq.find_by_slug(params[:archive_faq_id])
@archive_faq = ArchiveFaq.find_by(slug: params[:archive_faq_id])
unless @archive_faq.present?
flash[:error] = ts("Sorry, we couldn't find the FAQ you were looking for."
)
flash[:error] = t("questions.not_found")
redirect_to root_path and return
end
end

# Update the position number of questions within a archive_faq
def update_positions
authorize :archive_faq, :full_access?
if params[:questions]
@archive_faq = ArchiveFaq.find_by_slug(params[:archive_faq_id])
@archive_faq = ArchiveFaq.find_by(slug: params[:archive_faq_id])
@archive_faq.reorder_list(params[:questions])
flash[:notice] = ts("Question order has been successfully updated.")
flash[:notice] = t(".success")
elsif params[:question]
params[:question].each_with_index do |id, position|
Question.update(id, position: position + 1)
(@questions ||= []) << Question.find(id)
end
flash[:notice] = ts("Question order has been successfully updated.")
flash[:notice] = t(".success")
end
respond_to do |format|
format.html { redirect_to(@archive_faq) and return }
Expand Down
24 changes: 24 additions & 0 deletions app/policies/archive_faq_policy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# frozen_string_literal: true

class ArchiveFaqPolicy < ApplicationPolicy
TRANSLATION_ACCESS_ROLES = %w[superadmin docs support translation].freeze
# a subset of TRANSLATION_ACCESS_ROLES
FULL_ACCESS_ROLES = %w[superadmin docs support].freeze

def translation_access?
user_has_roles?(TRANSLATION_ACCESS_ROLES)
end

def full_access?
user_has_roles?(FULL_ACCESS_ROLES)
end

alias edit? translation_access?
alias update? translation_access?
alias new? full_access?
alias create? full_access?
alias manage? full_access?
alias update_positions? full_access?
alias confirm_delete? full_access?
alias destroy? full_access?
end
8 changes: 5 additions & 3 deletions app/views/admin/_admin_nav.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@
</li>
<% end %>
<li>
<%= span_if_current ts("Archive FAQ", key: "header"), archive_faqs_path %>
</li>
<% if policy(ArchiveFaq).translation_access? %>
<li>
<%= span_if_current t(".archive_faq"), archive_faqs_path %>
</li>
<% end %>
<li>
<%= span_if_current ts("Known Issues", key: "header"), known_issues_path %>
</li>
Expand Down
4 changes: 3 additions & 1 deletion app/views/admin/_header.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@
<% if policy(AdminPost).can_post? %>
<li><%= link_to t(".nav.posts.post_news"), new_admin_post_path %></li>
<% end %>
<li><%= link_to t(".nav.posts.faqs"), archive_faqs_path %></li>
<% if policy(ArchiveFaq).translation_access? %>
<li><%= link_to t(".nav.posts.faqs"), archive_faqs_path %></li>
<% end %>
<li><%= link_to t(".nav.posts.known_issues"), known_issues_path %></li>
<li><%= link_to t(".nav.posts.wrangling_guidelines"), wrangling_guidelines_path %></li>
</ul>
Expand Down
26 changes: 14 additions & 12 deletions app/views/archive_faqs/_admin_index.html.erb
Original file line number Diff line number Diff line change
@@ -1,31 +1,33 @@
<div class="admin">
<!--Descriptive page name, messages and instructions-->
<h2 class="heading"><%= ts("Archive FAQ") %></h2>
<h2 class="heading"><%= t(".page_heading") %></h2>
<!--/descriptions-->

<!--subnav-->
<%= render 'archive_faqs/filters' %>
<%= render 'admin/admin_nav' %>
<%= render "archive_faqs/filters" %>
<%= render "admin/admin_nav" %>
<ul class="navigation actions" role="navigation">
<% if Globalize.locale.to_s == "en" %>
<li><%= link_to ts("New FAQ Category"), new_archive_faq_path %></li>
<li><%= link_to ts("Reorder FAQs"), manage_archive_faqs_path(:language_id => params[:language_id]) %></li>
<% if Globalize.locale.to_s == "en" && policy(ArchiveFaq).full_access? %>
<li><%= link_to t(".new_faq_category"), new_archive_faq_path %></li>
<li><%= link_to t(".reorder_faqs"), manage_archive_faqs_path(language_id: params[:language_id]) %></li>
<% end %>
</ul>
<!--/subnav-->

<!--main content-->
<h3 class="landmark heading"><%= ts("Manage Archive FAQs") %></h3>
<h3 class="landmark heading"><%= t(".manage_faqs") %></h3>
<dl class="faq index group">
<% @archive_faqs.each do |archive_faq| %>
<dt><%= link_to archive_faq.title, archive_faq %></dt>
<dd>
<p class="datetime"><%= ts("Created at %{date_created} and updated at %{date_updated}", :date_created => archive_faq.created_at, :date_updated => archive_faq.updated_at) %></p>
<p class="datetime"><%= t(".created_updated_date", date_created: l(archive_faq.created_at), date_updated: l(archive_faq.updated_at)) %></p>
<ul class="navigation actions">
<li><%= link_to ts("Show"), archive_faq %></li>
<li><%= link_to ts("Edit"), edit_archive_faq_path(archive_faq) %></li>
<% if Globalize.locale.to_s == "en" %>
<li><%= link_to ts("Delete"), confirm_delete_archive_faq_path(archive_faq), data: {confirm: ts('Are you sure you want to delete this FAQ category?')} %></li>
<li><%= link_to t(".show"), archive_faq %></li>
<% if Globalize.locale.to_s != "en" || policy(ArchiveFaq).full_access? %>
<li><%= link_to t(".edit"), edit_archive_faq_path(archive_faq) %></li>
<% end %>
<% if Globalize.locale.to_s == "en" && policy(ArchiveFaq).full_access? %>
<li><%= link_to t(".delete"), confirm_delete_archive_faq_path(archive_faq), data: { confirm: t(".confirm_delete") } %></li>
<% end %>
</ul>
</dd>
Expand Down
2 changes: 1 addition & 1 deletion app/views/archive_faqs/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<% if logged_in_as_admin? %>
<% if policy(ArchiveFaq).translation_access? %>
<%= render "admin_index" %>
<% else %>
<%= render "faq_index" %>
Expand Down
19 changes: 11 additions & 8 deletions app/views/archive_faqs/show.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<!--Descriptive page name, messages and instructions-->
<h2 class="heading"><%= link_to ts("Archive FAQ"), archive_faqs_path %> > <%= @archive_faq.title %></h2>
<h2 class="heading"><%= link_to t(".page_heading"), archive_faqs_path %> > <%= @archive_faq.title %></h2>
<!--/descriptions-->

<!--subnav-->
Expand All @@ -8,21 +8,24 @@
<!--main content-->
<% if @archive_faq.slug == "search-and-browse" %>
<p class="notice">
<%= ts("Our search engine has recently been updated, and this FAQ is based on our old version. We're working on bringing you more up-to-date information, but in the meantime, you can find out more in our %{elasticsearch_post}!", elasticsearch_post: link_to(ts("news post announcing the search and filter updates"), admin_post_path(10575))).html_safe %>
<%= t(".elasticsearch_update_notice_html", elasticsearch_news_link: link_to(t(".elasticsearch_news"), admin_post_path(10_575))) %>
</p>
<% end %>

<div class="admin" role="article">
<% if logged_in_as_admin? %>
<% if policy(ArchiveFaq).translation_access? %>
<div class="header">
<h3 class="heading">
Updated: <%=h @archive_faq.updated_at %> | <%= link_to 'Edit', edit_archive_faq_path(@archive_faq) %>
Updated: <%= h @archive_faq.updated_at %>
<% if Globalize.locale.to_s != "en" || policy(ArchiveFaq).full_access? -%>
| <%= link_to t(".edit"), edit_archive_faq_path(@archive_faq) %>
<% end %>
</h3>
</div>
<% end %>
<% if @archive_faq.questions.blank? %>
<p class="notice"><%= ts("We're sorry, there are currently no entries in this category.") %></p>
<% if @archive_faq.questions.blank? %>
<p class="notice"><%= t(".no_category_entries") %></p>
<% else %>
<div <% if rtl? %>dir="rtl"<% end %> class="userstuff">
<nav id="toc" aria-labelledby="toc-nav-heading" class="toc">
Expand All @@ -32,7 +35,7 @@
</h3>
<ul class="toc">
<% for q in @questions %>
<li><%= link_to q.question, archive_faq_path(@archive_faq, :anchor => q.anchor) %></li>
<li><%= link_to q.question, archive_faq_path(@archive_faq, anchor: q.anchor) %></li>
<% end %>
</ul>
</nav>
Expand All @@ -44,7 +47,7 @@
</h3>
<% unless q.screencast.to_s == "" %>
<p class="screencast">
<span class="label"><%= ts("Screencast") %>:</span> <%= link_to q.question, q.screencast.to_s %>
<span class="label"><%= t(".screencast") %></span> <%= link_to q.question, q.screencast.to_s %>
</p>
<% end %>
<%= raw sanitize_field(q, :content) %>
Expand Down
12 changes: 12 additions & 0 deletions config/locales/controllers/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ en:
admin_users:
destroy_user_creations:
success: All creations by user %{login} have been deleted.
archive_faqs:
create:
success: Archive FAQ was successfully created.
default_locale_only: Sorry, this action is only available for English FAQs.
update:
success: Archive FAQ was successfully updated.
update_positions:
success: Archive FAQs order was successfully updated.
blocked:
users:
create:
Expand Down Expand Up @@ -99,6 +107,10 @@ en:
muted: You have muted the user %{name}.
destroy:
unmuted: You have unmuted the user %{name}.
questions:
not_found: Sorry, we couldn't find the FAQ you were looking for.
update_positions:
success: Question order has been successfully updated.
users:
passwords:
create:
Expand Down
20 changes: 20 additions & 0 deletions config/locales/views/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ en:
queue: Manage Queue
requests: Manage Requests
page_heading: Invite New Users
admin_nav:
archive_faq: Archive FAQ
admin_options:
delete:
bookmark: Delete Bookmark
Expand Down Expand Up @@ -376,6 +378,24 @@ en:
roles:
heading: 'Your admin roles:'
none: You currently have no admin roles assigned to you.
archive_faqs:
admin_index:
confirm_delete: Are you sure you want to delete this FAQ category?
created_updated_date: Created at %{date_created} and updated at %{date_updated}
delete: Delete
edit: Edit
manage_faqs: Manage Archive FAQs
new_faq_category: New FAQ Category
page_heading: Archive FAQ
reorder_faqs: Reorder FAQs
show: Show
show:
edit: Edit
elasticsearch_news: news post announcing the search and filter updates
elasticsearch_update_notice_html: Our search engine has recently been updated, and this FAQ is based on our old version. We're working on bringing you more up-to-date information, but in the meantime, you can find out more in our %{elasticsearch_news_link}!
no_category_entries: We're sorry, there are currently no entries in this category.
page_heading: Archive FAQ
screencast: 'Screencast:'
blocked:
block: Block
unblock: Unblock
Expand Down
Loading

0 comments on commit 08eae80

Please sign in to comment.