From f1b222a74f955b37ba2ab35e6f8b86f4e1a38d88 Mon Sep 17 00:00:00 2001
From: Bilka <Bilka2@users.noreply.github.com>
Date: Thu, 9 Jan 2025 20:02:19 +0100
Subject: [PATCH 1/8] AO3-6204 Allow certain admins to remove non-default
orphan pseuds from works
---
.../admin/user_creations_controller.rb | 35 ++++-
app/controllers/orphans_controller.rb | 2 +-
app/models/concerns/creatable.rb | 5 +
app/policies/user_creation_policy.rb | 6 +
app/views/admin/_admin_options.html.erb | 14 ++
.../confirm_remove_pseud.html.erb | 30 ++++
config/locales/controllers/en.yml | 8 ++
config/locales/views/en.yml | 9 ++
config/routes.rb | 2 +
features/admins/admin_works.feature | 114 ++++++++++++++-
features/step_definitions/admin_steps.rb | 4 +
public/javascripts/application.js | 6 +
.../admin/user_creations_controller_spec.rb | 135 ++++++++++++++++++
13 files changed, 367 insertions(+), 3 deletions(-)
create mode 100644 app/views/admin/user_creations/confirm_remove_pseud.html.erb
diff --git a/app/controllers/admin/user_creations_controller.rb b/app/controllers/admin/user_creations_controller.rb
index 6e5d0300a33..09f44cfbfb1 100644
--- a/app/controllers/admin/user_creations_controller.rb
+++ b/app/controllers/admin/user_creations_controller.rb
@@ -1,5 +1,5 @@
class Admin::UserCreationsController < Admin::BaseController
- before_action :get_creation
+ before_action :get_creation, except: [:remove_pseud, :confirm_remove_pseud]
before_action :can_be_marked_as_spam, only: [:set_spam]
def get_creation
@@ -59,4 +59,37 @@ def destroy
redirect_to works_path
end
end
+
+ def confirm_remove_pseud
+ authorize @work = Work.find(params[:id])
+
+ @orphan_pseuds = @work.orphan_pseuds
+ return unless @orphan_pseuds.empty?
+
+ flash[:error] = t(".must_have_orphan_pseuds")
+ redirect_to work_path(@work) and return
+ end
+
+ def remove_pseud
+ authorize @work = Work.find(params[:id])
+
+ pseuds = params[:pseuds]
+ orphan_account = User.orphan_account
+ if pseuds.blank?
+ pseuds = @work.orphan_pseuds
+ if pseuds.length > 1
+ flash[:error] = t(".must_select_pseud")
+ redirect_to work_path(@work) and return
+ end
+ else
+ pseuds = Pseud.find(pseuds).select { |p| p.user_id == orphan_account.id }
+ end
+
+ pseuds.each do |pseud|
+ pseud.change_ownership(@work, User.orphan_account.default_pseud)
+ end
+ AdminActivity.log_action(current_admin, @work, action: "remove orphan_account pseuds")
+ flash[:notice] = t(".success", pseuds: pseuds.map(&:byline).to_sentence, count: pseuds.length)
+ redirect_to work_path(@work)
+ end
end
diff --git a/app/controllers/orphans_controller.rb b/app/controllers/orphans_controller.rb
index 3e3fd82374a..b5edab1efd9 100644
--- a/app/controllers/orphans_controller.rb
+++ b/app/controllers/orphans_controller.rb
@@ -33,7 +33,7 @@ def create
use_default = params[:use_default] == "true"
Creatorship.orphan(@pseuds, @orphans, use_default)
flash[:notice] = ts("Orphaning was successful.")
- redirect_to(current_user)
+ redirect_to user_path(current_user)
end
protected
diff --git a/app/models/concerns/creatable.rb b/app/models/concerns/creatable.rb
index b407bfa90ef..dab9dbb8cb5 100644
--- a/app/models/concerns/creatable.rb
+++ b/app/models/concerns/creatable.rb
@@ -220,4 +220,9 @@ def user_is_owner_or_invited?(user)
return false unless user.is_a?(User)
creatorships.for_user(user).exists?
end
+
+ # Get all orphan_account pseuds that (co-)created this creatable, excluding the orphan_account's default_pseud
+ def orphan_pseuds
+ self.pseuds.where(user_id: User.orphan_account.id, is_default: false)
+ end
end
diff --git a/app/policies/user_creation_policy.rb b/app/policies/user_creation_policy.rb
index 3eb5f8b247c..8159dc885b5 100644
--- a/app/policies/user_creation_policy.rb
+++ b/app/policies/user_creation_policy.rb
@@ -13,6 +13,10 @@ def hide?
user_has_roles?(FULL_ACCESS_ROLES)
end
+ def remove_pseud?
+ user_has_roles?(%w[superadmin support policy_and_abuse])
+ end
+
def show_ip_address?
user_has_roles?(FULL_ACCESS_ROLES)
end
@@ -20,4 +24,6 @@ def show_ip_address?
def show_original_creators?
user_has_roles?(FULL_ACCESS_ROLES)
end
+
+ alias confirm_remove_pseud? remove_pseud?
end
diff --git a/app/views/admin/_admin_options.html.erb b/app/views/admin/_admin_options.html.erb
index 5fcc5f22e0b..7d3bf46d3b8 100644
--- a/app/views/admin/_admin_options.html.erb
+++ b/app/views/admin/_admin_options.html.erb
@@ -37,6 +37,20 @@
<% end %>
</li>
<% end %>
+ <% if policy(@work).remove_pseud? %>
+ <% orphan_pseuds = @work.orphan_pseuds %>
+ <% if orphan_pseuds.length == 1 %>
+ <li>
+ <%= link_to t(".remove_pseud"),
+ confirm_remove_pseud_admin_user_creation_path(@work),
+ data: { confirm: t(".remove_pseud_confirmation") } %>
+ </li>
+ <% elsif orphan_pseuds.length > 1 %>
+ <li>
+ <%= link_to t(".remove_pseud"), confirm_remove_pseud_admin_user_creation_path(@work) %>
+ </li>
+ <% end %>
+ <% end %>
<% end %>
<% if item.class == ExternalWork %>
<% if policy(item).edit? %>
diff --git a/app/views/admin/user_creations/confirm_remove_pseud.html.erb b/app/views/admin/user_creations/confirm_remove_pseud.html.erb
new file mode 100644
index 00000000000..77719bcb231
--- /dev/null
+++ b/app/views/admin/user_creations/confirm_remove_pseud.html.erb
@@ -0,0 +1,30 @@
+<!--Descriptive page name, messages and instructions-->
+<h2 class="heading"><%= t(".page_heading") %></h2>
+
+<!--main content-->
+<%= form_with url: remove_pseud_admin_user_creation_path(@work), method: :put, class: "simple destroy" do |f| %>
+ <% if @orphan_pseuds.length > 1 %>
+ <p class="caution notice">
+ <%= t(".choose") %>
+ </p>
+ <ul>
+ <%= f.collection_check_boxes :pseuds, @orphan_pseuds, :id, :byline, {include_hidden: false} do |builder| %>
+ <li>
+ <%= builder.check_box %>
+ <%= builder.label %>
+ </li>
+ <% end %>
+ </ul>
+ <p class="actions">
+ <%= f.submit t(".submit_multiple") %>
+ </p>
+ <% else %>
+ <p class="caution notice">
+ <%= t(".caution") %>
+ </p>
+ <p class="actions">
+ <%= f.submit t(".submit_one") %>
+ </p>
+ <% end %>
+<% end %>
+<!--/content-->
diff --git a/config/locales/controllers/en.yml b/config/locales/controllers/en.yml
index b8b56ec9838..b6287c7fe81 100644
--- a/config/locales/controllers/en.yml
+++ b/config/locales/controllers/en.yml
@@ -14,6 +14,14 @@ en:
admin_users:
destroy_user_creations:
success: All creations by user %{login} have been deleted.
+ user_creations:
+ confirm_remove_pseud:
+ must_have_orphan_pseuds: Sorry, this action is only available for works by orphan_account pseuds.
+ remove_pseud:
+ must_select_pseud: You must select which orphan_account pseud to remove.
+ success:
+ one: Successfully removed pseud %{pseuds} from this work.
+ other: Successfully removed pseuds %{pseuds} from this work.
archive_faqs:
create:
success: Archive FAQ was successfully created.
diff --git a/config/locales/views/en.yml b/config/locales/views/en.yml
index 00d07232965..938e91df3f1 100644
--- a/config/locales/views/en.yml
+++ b/config/locales/views/en.yml
@@ -119,6 +119,8 @@ en:
work: Hide Work
landmark: Admin Actions
not_spam: Mark Not Spam
+ remove_pseud: Remove Pseud
+ remove_pseud_confirmation: Are you sure you want to remove the creator's pseud from this work?
spam: Mark As Spam
unhide:
bookmark: Make Bookmark Visible
@@ -352,6 +354,13 @@ en:
update: Update
update:
success: Archive settings were successfully updated.
+ user_creations:
+ confirm_remove_pseud:
+ caution: Are you sure you want to remove the creator's pseud from this work?
+ choose: Please choose which creators' pseuds you would like to remove from this work.
+ page_heading: Remove Pseud
+ submit_multiple: Remove Pseuds
+ submit_one: Yes, Remove Pseud
admin_posts:
admin_post_form:
comment_permissions:
diff --git a/config/routes.rb b/config/routes.rb
index a4fedf346dc..8807ad64cff 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -196,6 +196,8 @@
member do
put :hide
put :set_spam
+ get :confirm_remove_pseud
+ put :remove_pseud
end
end
resources :users, controller: "admin_users", only: [:index, :show] do
diff --git a/features/admins/admin_works.feature b/features/admins/admin_works.feature
index 39807cbd650..5c824d4ed75 100644
--- a/features/admins/admin_works.feature
+++ b/features/admins/admin_works.feature
@@ -36,7 +36,7 @@ Feature: Admin Actions for Works, Comments, Series, Bookmarks
Scenario Outline: Can unhide works
Given I am logged in as "regular_user"
And I post the work "ToS Violation"
- When I am logged in as a "policy_and_abuse" admin
+ When I am logged in as a "<role>" admin
And I view the work "ToS Violation"
And I follow "Hide Work"
And all indexing jobs have been run
@@ -491,3 +491,115 @@ Feature: Admin Actions for Works, Comments, Series, Bookmarks
| superadmin |
| legal |
| policy_and_abuse |
+
+ Scenario Outline: Certain admins can remove orphan_account pseuds from works
+ Given I have an orphan account
+ And I am logged in as "Leaver"
+ And I post the work "Bye"
+ And I orphan and keep my pseud on the work "Bye"
+ When I am logged in as a "<role>" admin
+ And I view the work "Bye"
+ Then I should see "Remove Pseud"
+ When I follow "Remove Pseud"
+ Then I should see "Are you sure you want to remove the creator's pseud from this work?"
+ # Expire byline cache
+ When it is currently 1 second from now
+ And I press "Yes, Remove Pseud"
+ Then I should see "Successfully removed pseud Leaver (orphan_account) from this work."
+ And I should see "orphan_account" within ".byline"
+ But I should not see "Leaver" within ".byline"
+
+ Examples:
+ | role |
+ | superadmin |
+ | policy_and_abuse |
+ | support |
+
+ @javascript
+ Scenario Outline: Removing orphan_account pseuds from works with JavaScript shows a confirmation pop-up instead of a page
+ Given I have an orphan account
+ And I am logged in as "Leaver"
+ And I post the work "Bye"
+ And I orphan and keep my pseud on the work "Bye"
+ When I am logged in as a "<role>" admin
+ And I view the work "Bye"
+ Then I should see "Remove Pseud"
+ # Expire byline cache
+ When it is currently 1 second from now
+ And I follow "Remove Pseud"
+ And I confirm I want to remove the pseud
+ Then I should see "Successfully removed pseud Leaver (orphan_account) from this work."
+ And I should see "orphan_account" within ".byline"
+ But I should not see "Leaver" within ".byline"
+
+ Examples:
+ | role |
+ | superadmin |
+ | policy_and_abuse |
+ | support |
+
+ Scenario: When removing orphan_account pseuds from a work with multiple pseuds admins choose which pseud to remove
+ Given I have an orphan account
+ And I am logged in as "Leaver"
+ And I post the work "Bye"
+ And I add the co-author "Another" to the work "Bye"
+ And it is currently 1 second from now
+ And I add the co-author "Third" to the work "Bye"
+ And I orphan and keep my pseud on the work "Bye"
+ And I am logged in as "Another"
+ And I orphan and keep my pseud on the work "Bye"
+ And I am logged in as "Third"
+ And I orphan and keep my pseud on the work "Bye"
+ When I am logged in as a "policy_and_abuse" admin
+ And I view the work "Bye"
+ Then I should see "Remove Pseud"
+ When I follow "Remove Pseud"
+ Then I should see "Please choose which creators' pseuds you would like to remove from this work."
+ And I should see "Third (orphan_account)"
+ When I check "Leaver (orphan_account)"
+ And I check "Another (orphan_account)"
+ # Expire byline cache
+ And it is currently 1 second from now
+ And I press "Remove Pseud"
+ Then I should see "Successfully removed pseuds Leaver (orphan_account) and Another (orphan_account) from this work."
+ And I should see "orphan_account, " within ".byline"
+ And I should see "Third (orphan_account)" within ".byline"
+ But I should not see "Leaver" within ".byline"
+ And I should not see "Another" within ".byline"
+ When I go to the admin-activities page
+ Then I should see 1 admin activity log entry
+ And I should see "remove orphan_account pseuds"
+
+ Scenario: The Remove pseud option is only shown on orphaned works with non-default pseuds
+ Given I have an orphan account
+ And I am logged in as "Leaver"
+ And I post the work "Hey"
+ And I post the work "Bye"
+ And I orphan and take my pseud off the work "Bye"
+ When I am logged in as a "superadmin" admin
+ And I view the work "Hey"
+ Then I should not see "Remove Pseud"
+ When I view the work "Bye"
+ Then I should not see "Remove Pseud"
+
+ Scenario Outline: The Remove pseud option is not shown to admins who don't have permissions to remove pseuds
+ Given I have an orphan account
+ And I am logged in as "Leaver"
+ And I post the work "Bye"
+ And I orphan and keep my pseud on the work "Bye"
+ When I am logged in as a "<role>" admin
+ And I view the work "Bye"
+ Then I should not see "Remove Pseud"
+
+ Examples:
+ | role |
+ | board |
+ | board_assistants_team |
+ | communications |
+ | development_and_membership |
+ | docs |
+ | elections |
+ | legal |
+ | translation |
+ | tag_wrangling |
+ | open_doors |
diff --git a/features/step_definitions/admin_steps.rb b/features/step_definitions/admin_steps.rb
index fa8e38d8926..6573c1cb4b2 100644
--- a/features/step_definitions/admin_steps.rb
+++ b/features/step_definitions/admin_steps.rb
@@ -349,6 +349,10 @@
step "it is currently #{days} days from now"
end
+When "I confirm I want to remove the pseud" do
+ expect(page.accept_alert).to eq("Are you sure you want to remove the creator's pseud from this work?") if @javascript
+end
+
### THEN
Then (/^the translation information should still be filled in$/) do
diff --git a/public/javascripts/application.js b/public/javascripts/application.js
index 84aa957879a..eee05d689ed 100644
--- a/public/javascripts/application.js
+++ b/public/javascripts/application.js
@@ -407,6 +407,12 @@ function prepareDeleteLinks() {
$j(this).attr("data-method", "delete");
});
+ // Removing non-default orphan_account pseuds from works
+ $j('a[href$="/confirm_remove_pseud"][data-confirm]').each(function() {
+ this.href = this.href.replace(/\/confirm_remove_pseud$/, "/remove_pseud");
+ $j(this).attr("data-method", "put");
+ });
+
// For purging assignments in gift exchanges. This is only on one page and easy to
// check, so don't worry about adding a fallback data-confirm message.
$j('a[href$="/confirm_purge"][data-confirm]').each(function() {
diff --git a/spec/controllers/admin/user_creations_controller_spec.rb b/spec/controllers/admin/user_creations_controller_spec.rb
index b663cca34d5..ce85cad2cd3 100644
--- a/spec/controllers/admin/user_creations_controller_spec.rb
+++ b/spec/controllers/admin/user_creations_controller_spec.rb
@@ -164,4 +164,139 @@
it_behaves_like "unauthorized admin cannot delete bookmarks"
end
end
+
+ shared_examples "an action only authorized admins can access" do |authorized_roles:|
+ before { fake_login_admin(admin) }
+
+ context "with no role" do
+ let(:admin) { create(:admin, roles: []) }
+
+ it "redirects with an error" do
+ subject
+ 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 |role|
+ context "with role #{role}" do
+ let(:admin) { create(:admin, roles: [role]) }
+
+ it "redirects with an error" do
+ subject
+ 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 |role|
+ context "with role #{role}" do
+ let(:admin) { create(:admin, roles: [role]) }
+
+ it "succeeds" do
+ subject
+ success
+ end
+ end
+ end
+ end
+
+ authorized_roles = %w[superadmin policy_and_abuse support].freeze
+
+ describe "GET #confirm_remove_pseud" do
+ subject { get :confirm_remove_pseud, params: { id: work.id } }
+ let(:work) do
+ work = create(:work)
+ create(:user, login: "orphan_account")
+ Creatorship.orphan(work.pseuds, [work], false)
+ work
+ end
+ let(:success) do
+ expect(response).to render_template(:confirm_remove_pseud)
+ end
+
+ it_behaves_like "an action only authorized admins can access", authorized_roles: authorized_roles
+
+ context "when logged in as user" do
+ it "redirects with notice" do
+ fake_login
+ subject
+ it_redirects_to_with_notice(root_path, "I'm sorry, only an admin can look at that area")
+ end
+ end
+
+ context "for a non-orphaned work" do
+ let(:work) { create(:work) }
+
+ before do
+ fake_login_admin(create(:superadmin))
+ end
+
+ it "redirects with an error" do
+ subject
+ it_redirects_to_with_error(work_path(work), "Sorry, this action is only available for works by orphan_account pseuds.")
+ end
+ end
+ end
+
+ describe "PUT #remove_pseud" do
+ subject { put :remove_pseud, params: { id: work.id } }
+ let(:user) { create(:user, login: "Leaver") }
+ let!(:orphan_account) { create(:user, login: "orphan_account") }
+ let!(:orphan_pseud) { create(:pseud, name: "Leaver", user: orphan_account) }
+ let(:work) do
+ work = create(:work, authors: [user.default_pseud])
+ Creatorship.orphan([user.default_pseud], [work], false)
+ work
+ end
+ let(:success) do
+ it_redirects_to_with_notice(work_path(work), "Successfully removed pseud Leaver (orphan_account) from this work.")
+ expect(work.reload.pseuds).to include(orphan_account.default_pseud)
+ expect(work.pseuds).not_to include(orphan_pseud)
+ end
+
+ it_behaves_like "an action only authorized admins can access", authorized_roles: authorized_roles
+
+ context "when logged in as user" do
+ it "redirects with notice" do
+ fake_login
+ subject
+ it_redirects_to_with_notice(root_path, "I'm sorry, only an admin can look at that area")
+ end
+ end
+
+ context "for a work with multiple orphan pseuds" do
+ let!(:orphaneer_orphan_pseud) { create(:pseud, name: "orphaneer", user: orphan_account) }
+
+ let(:work) do
+ orphaneer = create(:user, login: "orphaneer")
+ work = create(:work, authors: [user.default_pseud, orphaneer.default_pseud])
+ Creatorship.orphan([user.default_pseud, orphaneer.default_pseud], [work], false)
+ work
+ end
+
+ before do
+ fake_login_admin(create(:superadmin))
+ end
+
+ context "without a pseuds parameter" do
+ it "redirects with an error" do
+ subject
+ it_redirects_to_with_error(work_path(work), "You must select which orphan_account pseud to remove.")
+ expect(work.reload.pseuds).not_to include(orphan_account.default_pseud)
+ end
+ end
+
+ context "with a pseuds parameter" do
+ subject { put :remove_pseud, params: { id: work.id, pseuds: [orphan_pseud.id] } }
+
+ it "redirects removes only that pseud" do
+ subject
+ it_redirects_to_with_notice(work_path(work), "Successfully removed pseud Leaver (orphan_account) from this work.")
+ expect(work.reload.pseuds).to include(orphan_account.default_pseud)
+ expect(work.pseuds).not_to include(orphan_pseud)
+ expect(work.reload.pseuds).to include(orphaneer_orphan_pseud)
+ end
+ end
+ end
+ end
end
From 6a1704c801a56c822fc41f4833b3424b70fdd630 Mon Sep 17 00:00:00 2001
From: Bilka <Bilka2@users.noreply.github.com>
Date: Thu, 9 Jan 2025 22:52:36 +0100
Subject: [PATCH 2/8] AO3-6204 ERB lint
---
app/views/admin/_admin_options.html.erb | 6 +++---
.../admin/user_creations/confirm_remove_pseud.html.erb | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/app/views/admin/_admin_options.html.erb b/app/views/admin/_admin_options.html.erb
index 7d3bf46d3b8..d358cb65901 100644
--- a/app/views/admin/_admin_options.html.erb
+++ b/app/views/admin/_admin_options.html.erb
@@ -40,10 +40,10 @@
<% if policy(@work).remove_pseud? %>
<% orphan_pseuds = @work.orphan_pseuds %>
<% if orphan_pseuds.length == 1 %>
- <li>
+ <li>
<%= link_to t(".remove_pseud"),
- confirm_remove_pseud_admin_user_creation_path(@work),
- data: { confirm: t(".remove_pseud_confirmation") } %>
+ confirm_remove_pseud_admin_user_creation_path(@work),
+ data: { confirm: t(".remove_pseud_confirmation") } %>
</li>
<% elsif orphan_pseuds.length > 1 %>
<li>
diff --git a/app/views/admin/user_creations/confirm_remove_pseud.html.erb b/app/views/admin/user_creations/confirm_remove_pseud.html.erb
index 77719bcb231..b2fcf6e40a6 100644
--- a/app/views/admin/user_creations/confirm_remove_pseud.html.erb
+++ b/app/views/admin/user_creations/confirm_remove_pseud.html.erb
@@ -8,7 +8,7 @@
<%= t(".choose") %>
</p>
<ul>
- <%= f.collection_check_boxes :pseuds, @orphan_pseuds, :id, :byline, {include_hidden: false} do |builder| %>
+ <%= f.collection_check_boxes :pseuds, @orphan_pseuds, :id, :byline, { include_hidden: false } do |builder| %>
<li>
<%= builder.check_box %>
<%= builder.label %>
From 431ab1c9ffbff1847b9e8d7c10d5fe0b6b2a64eb Mon Sep 17 00:00:00 2001
From: Bilka <Bilka2@users.noreply.github.com>
Date: Thu, 9 Jan 2025 22:55:40 +0100
Subject: [PATCH 3/8] AO3-6204 Pressed TAB but not alt
---
app/views/admin/_admin_options.html.erb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/app/views/admin/_admin_options.html.erb b/app/views/admin/_admin_options.html.erb
index d358cb65901..4b6800c617e 100644
--- a/app/views/admin/_admin_options.html.erb
+++ b/app/views/admin/_admin_options.html.erb
@@ -40,7 +40,7 @@
<% if policy(@work).remove_pseud? %>
<% orphan_pseuds = @work.orphan_pseuds %>
<% if orphan_pseuds.length == 1 %>
- <li>
+ <li>
<%= link_to t(".remove_pseud"),
confirm_remove_pseud_admin_user_creation_path(@work),
data: { confirm: t(".remove_pseud_confirmation") } %>
From a4b8f83cc649df503c1b764741671b97fa3c256d Mon Sep 17 00:00:00 2001
From: Bilka <Bilka2@users.noreply.github.com>
Date: Fri, 10 Jan 2025 20:42:33 +0100
Subject: [PATCH 4/8] AO3-6204 Add test for attempting to remove a normal users
pseud
---
app/controllers/admin/user_creations_controller.rb | 3 ++-
.../admin/user_creations_controller_spec.rb | 13 ++++++++++++-
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/app/controllers/admin/user_creations_controller.rb b/app/controllers/admin/user_creations_controller.rb
index 09f44cfbfb1..8444450d6fe 100644
--- a/app/controllers/admin/user_creations_controller.rb
+++ b/app/controllers/admin/user_creations_controller.rb
@@ -85,8 +85,9 @@ def remove_pseud
pseuds = Pseud.find(pseuds).select { |p| p.user_id == orphan_account.id }
end
+ orphan_pseud = orphan_account.default_pseud
pseuds.each do |pseud|
- pseud.change_ownership(@work, User.orphan_account.default_pseud)
+ pseud.change_ownership(@work, orphan_pseud)
end
AdminActivity.log_action(current_admin, @work, action: "remove orphan_account pseuds")
flash[:notice] = t(".success", pseuds: pseuds.map(&:byline).to_sentence, count: pseuds.length)
diff --git a/spec/controllers/admin/user_creations_controller_spec.rb b/spec/controllers/admin/user_creations_controller_spec.rb
index ce85cad2cd3..c6517c62c3d 100644
--- a/spec/controllers/admin/user_creations_controller_spec.rb
+++ b/spec/controllers/admin/user_creations_controller_spec.rb
@@ -286,7 +286,7 @@
end
end
- context "with a pseuds parameter" do
+ context "with a orphan_account pseuds parameter" do
subject { put :remove_pseud, params: { id: work.id, pseuds: [orphan_pseud.id] } }
it "redirects removes only that pseud" do
@@ -297,6 +297,17 @@
expect(work.reload.pseuds).to include(orphaneer_orphan_pseud)
end
end
+
+ context "with a pseud parameter by a normal user" do
+ subject { put :remove_pseud, params: { id: work.id, pseuds: [user.default_pseud.id] } }
+
+ it "does not modify the work" do
+ expect do
+ subject
+ end.not_to change { work.pseuds }
+ it_redirects_to_with_notice(work_path(work), "Successfully removed pseuds from this work.")
+ end
+ end
end
end
end
From fa683b5b1b574631f6ccac3cd0eaf071dc526da3 Mon Sep 17 00:00:00 2001
From: Bilka <Bilka2@users.noreply.github.com>
Date: Sat, 11 Jan 2025 21:56:53 +0100
Subject: [PATCH 5/8] AO3-6204 Add test for attempting to remove a normal users
pseud
---
spec/controllers/admin/user_creations_controller_spec.rb | 1 +
1 file changed, 1 insertion(+)
diff --git a/spec/controllers/admin/user_creations_controller_spec.rb b/spec/controllers/admin/user_creations_controller_spec.rb
index c6517c62c3d..0819d70c939 100644
--- a/spec/controllers/admin/user_creations_controller_spec.rb
+++ b/spec/controllers/admin/user_creations_controller_spec.rb
@@ -300,6 +300,7 @@
context "with a pseud parameter by a normal user" do
subject { put :remove_pseud, params: { id: work.id, pseuds: [user.default_pseud.id] } }
+ let(:work) { create(:work, authors: [user.default_pseud]) }
it "does not modify the work" do
expect do
From acf3df2c595391e0d484567567a6841c9e64b545 Mon Sep 17 00:00:00 2001
From: Bilka <Bilka2@users.noreply.github.com>
Date: Fri, 21 Feb 2025 19:10:31 +0100
Subject: [PATCH 6/8] AO3-6204 Review fixes
---
app/controllers/admin/user_creations_controller.rb | 12 +++++++-----
app/policies/user_creation_policy.rb | 6 ------
app/policies/work_policy.rb | 6 ++++++
.../admin/user_creations_controller_spec.rb | 2 +-
4 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/app/controllers/admin/user_creations_controller.rb b/app/controllers/admin/user_creations_controller.rb
index 8444450d6fe..3c588449d6f 100644
--- a/app/controllers/admin/user_creations_controller.rb
+++ b/app/controllers/admin/user_creations_controller.rb
@@ -1,5 +1,5 @@
class Admin::UserCreationsController < Admin::BaseController
- before_action :get_creation, except: [:remove_pseud, :confirm_remove_pseud]
+ before_action :get_creation, only: [:hide, :set_spam, :destroy]
before_action :can_be_marked_as_spam, only: [:set_spam]
def get_creation
@@ -61,7 +61,7 @@ def destroy
end
def confirm_remove_pseud
- authorize @work = Work.find(params[:id])
+ @work = authorize Work.find(params[:id])
@orphan_pseuds = @work.orphan_pseuds
return unless @orphan_pseuds.empty?
@@ -71,7 +71,7 @@ def confirm_remove_pseud
end
def remove_pseud
- authorize @work = Work.find(params[:id])
+ @work = authorize Work.find(params[:id])
pseuds = params[:pseuds]
orphan_account = User.orphan_account
@@ -89,8 +89,10 @@ def remove_pseud
pseuds.each do |pseud|
pseud.change_ownership(@work, orphan_pseud)
end
- AdminActivity.log_action(current_admin, @work, action: "remove orphan_account pseuds")
- flash[:notice] = t(".success", pseuds: pseuds.map(&:byline).to_sentence, count: pseuds.length)
+ unless pseuds.empty?
+ AdminActivity.log_action(current_admin, @work, action: "remove orphan_account pseuds")
+ flash[:notice] = t(".success", pseuds: pseuds.map(&:byline).to_sentence, count: pseuds.length)
+ end
redirect_to work_path(@work)
end
end
diff --git a/app/policies/user_creation_policy.rb b/app/policies/user_creation_policy.rb
index 8159dc885b5..3eb5f8b247c 100644
--- a/app/policies/user_creation_policy.rb
+++ b/app/policies/user_creation_policy.rb
@@ -13,10 +13,6 @@ def hide?
user_has_roles?(FULL_ACCESS_ROLES)
end
- def remove_pseud?
- user_has_roles?(%w[superadmin support policy_and_abuse])
- end
-
def show_ip_address?
user_has_roles?(FULL_ACCESS_ROLES)
end
@@ -24,6 +20,4 @@ def show_ip_address?
def show_original_creators?
user_has_roles?(FULL_ACCESS_ROLES)
end
-
- alias confirm_remove_pseud? remove_pseud?
end
diff --git a/app/policies/work_policy.rb b/app/policies/work_policy.rb
index a7cde4e609d..afa1de15ead 100644
--- a/app/policies/work_policy.rb
+++ b/app/policies/work_policy.rb
@@ -19,4 +19,10 @@ def destroy?
def set_spam?
user_has_roles?(%w[superadmin policy_and_abuse])
end
+
+ def remove_pseud?
+ user_has_roles?(%w[superadmin support policy_and_abuse])
+ end
+
+ alias confirm_remove_pseud? remove_pseud?
end
diff --git a/spec/controllers/admin/user_creations_controller_spec.rb b/spec/controllers/admin/user_creations_controller_spec.rb
index 0819d70c939..58dbbd0ef7c 100644
--- a/spec/controllers/admin/user_creations_controller_spec.rb
+++ b/spec/controllers/admin/user_creations_controller_spec.rb
@@ -294,7 +294,7 @@
it_redirects_to_with_notice(work_path(work), "Successfully removed pseud Leaver (orphan_account) from this work.")
expect(work.reload.pseuds).to include(orphan_account.default_pseud)
expect(work.pseuds).not_to include(orphan_pseud)
- expect(work.reload.pseuds).to include(orphaneer_orphan_pseud)
+ expect(work.pseuds).to include(orphaneer_orphan_pseud)
end
end
From 493add75dc26772e9f91fbfa97dd20547fbb2a4b Mon Sep 17 00:00:00 2001
From: Bilka <Bilka2@users.noreply.github.com>
Date: Fri, 21 Feb 2025 19:30:22 +0100
Subject: [PATCH 7/8] AO3-6204 Move authorized admin shared example to shared
file
---
.../admin/user_creations_controller_spec.rb | 35 ------------------
.../tag_wranglings_controller_spec.rb | 37 -------------------
spec/controllers/tags_controller_spec.rb | 35 ------------------
.../shared_examples/admin_shared_examples.rb | 34 +++++++++++++++++
4 files changed, 34 insertions(+), 107 deletions(-)
create mode 100644 spec/support/shared_examples/admin_shared_examples.rb
diff --git a/spec/controllers/admin/user_creations_controller_spec.rb b/spec/controllers/admin/user_creations_controller_spec.rb
index 58dbbd0ef7c..c5d9e753b0f 100644
--- a/spec/controllers/admin/user_creations_controller_spec.rb
+++ b/spec/controllers/admin/user_creations_controller_spec.rb
@@ -165,41 +165,6 @@
end
end
- shared_examples "an action only authorized admins can access" do |authorized_roles:|
- before { fake_login_admin(admin) }
-
- context "with no role" do
- let(:admin) { create(:admin, roles: []) }
-
- it "redirects with an error" do
- subject
- 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 |role|
- context "with role #{role}" do
- let(:admin) { create(:admin, roles: [role]) }
-
- it "redirects with an error" do
- subject
- 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 |role|
- context "with role #{role}" do
- let(:admin) { create(:admin, roles: [role]) }
-
- it "succeeds" do
- subject
- success
- end
- end
- end
- end
-
authorized_roles = %w[superadmin policy_and_abuse support].freeze
describe "GET #confirm_remove_pseud" do
diff --git a/spec/controllers/tag_wranglings_controller_spec.rb b/spec/controllers/tag_wranglings_controller_spec.rb
index 581a1772925..719b2d9410c 100644
--- a/spec/controllers/tag_wranglings_controller_spec.rb
+++ b/spec/controllers/tag_wranglings_controller_spec.rb
@@ -7,43 +7,6 @@
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)
- end
-
- context "when logged in as an admin with no role" do
- let(:admin) { create(:admin) }
-
- it "redirects with an error" do
- subject
- 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
- subject
- 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
- subject
- success
- end
- end
- end
- end
-
describe "GET #index" do
let(:success) { expect(response).to have_http_status(:success) }
diff --git a/spec/controllers/tags_controller_spec.rb b/spec/controllers/tags_controller_spec.rb
index 359e34bbf22..a11a72f6964 100644
--- a/spec/controllers/tags_controller_spec.rb
+++ b/spec/controllers/tags_controller_spec.rb
@@ -17,41 +17,6 @@
end
end
- shared_examples "an action only authorized admins can access" do |authorized_roles:|
- before { fake_login_admin(admin) }
-
- context "with no role" do
- let(:admin) { create(:admin, roles: []) }
-
- it "redirects with an error" do
- subject
- 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 |role|
- context "with role #{role}" do
- let(:admin) { create(:admin, roles: [role]) }
-
- it "redirects with an error" do
- subject
- 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 |role|
- context "with role #{role}" do
- let(:admin) { create(:admin, roles: [role]) }
-
- it "succeeds" do
- subject
- success
- end
- end
- end
- end
-
describe "#create" do
let(:tag_params) do
{ name: Faker::FunnyName.name, canonical: "0", type: "Character" }
diff --git a/spec/support/shared_examples/admin_shared_examples.rb b/spec/support/shared_examples/admin_shared_examples.rb
new file mode 100644
index 00000000000..e5686d1a918
--- /dev/null
+++ b/spec/support/shared_examples/admin_shared_examples.rb
@@ -0,0 +1,34 @@
+shared_examples "an action only authorized admins can access" do |authorized_roles:|
+ before { fake_login_admin(admin) }
+
+ context "with no role" do
+ let(:admin) { create(:admin, roles: []) }
+
+ it "redirects with an error" do
+ subject
+ 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 |role|
+ context "with role #{role}" do
+ let(:admin) { create(:admin, roles: [role]) }
+
+ it "redirects with an error" do
+ subject
+ 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 |role|
+ context "with role #{role}" do
+ let(:admin) { create(:admin, roles: [role]) }
+
+ it "succeeds" do
+ subject
+ success
+ end
+ end
+ end
+end
From e7c67fdd96cc14bd3b5560701edbed57c9c48b22 Mon Sep 17 00:00:00 2001
From: Bilka <Bilka2@users.noreply.github.com>
Date: Fri, 21 Feb 2025 19:35:55 +0100
Subject: [PATCH 8/8] AO3-6204 Fix test
---
spec/controllers/admin/user_creations_controller_spec.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/spec/controllers/admin/user_creations_controller_spec.rb b/spec/controllers/admin/user_creations_controller_spec.rb
index c5d9e753b0f..2c9c8765263 100644
--- a/spec/controllers/admin/user_creations_controller_spec.rb
+++ b/spec/controllers/admin/user_creations_controller_spec.rb
@@ -271,7 +271,7 @@
expect do
subject
end.not_to change { work.pseuds }
- it_redirects_to_with_notice(work_path(work), "Successfully removed pseuds from this work.")
+ it_redirects_to(work_path(work))
end
end
end