From f62cf60e810a56278d524fc8b26e8110055b44de Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Sat, 21 Dec 2024 16:28:56 -0500 Subject: [PATCH 1/7] AO3-931 Show placeholder for deleted invitee --- app/helpers/invitations_helper.rb | 15 ++++---- app/views/invitations/_invitation.html.erb | 40 ++++++++++++---------- config/locales/views/en.yml | 11 ++++++ spec/helpers/invitations_helper_spec.rb | 36 +++++++++++++++++++ 4 files changed, 77 insertions(+), 25 deletions(-) create mode 100644 spec/helpers/invitations_helper_spec.rb diff --git a/app/helpers/invitations_helper.rb b/app/helpers/invitations_helper.rb index 847252207a1..5ce797057c0 100644 --- a/app/helpers/invitations_helper.rb +++ b/app/helpers/invitations_helper.rb @@ -1,18 +1,19 @@ module InvitationsHelper - def creator_link(invitation) - if invitation.creator.is_a?(User) + case invitation.creator + when User link_to(invitation.creator.login, invitation.creator) - elsif invitation.creator.is_a?(Admin) + when Admin invitation.creator.login else - "queue" + t("invitations.invitation.queue") end end def invitee_link(invitation) - if invitation.invitee && invitation.invitee.is_a?(User) - link_to(invitation.invitee.login, invitation.invitee) - end + return unless invitation.invitee_type == "User" + return t("invitations.invitation.deleted_user", user_id: invitation.invitee_id) if invitation.invitee.blank? + + link_to(invitation.invitee.login, invitation.invitee) end end diff --git a/app/views/invitations/_invitation.html.erb b/app/views/invitations/_invitation.html.erb index f2cefdfb5c9..eef8801dd96 100644 --- a/app/views/invitations/_invitation.html.erb +++ b/app/views/invitations/_invitation.html.erb @@ -1,29 +1,33 @@
-
Sender
-
<%= creator_link(invitation) %>
-
Invitation token
-
<%= invitation.token %>
-
Copy link
-
<% unless invitation.redeemed_at %><%= link_to "copy and use", signup_path(:invitation_token => invitation.token) %><% end %>
-
Sent to
+
<%= t(".sender") %>
+
<%= creator_link(invitation) %>
+
<%= t(".invitation_token") %>
+
<%= invitation.token %>
+
<%= t(".copy_link") %>
+
+ <% unless invitation.redeemed_at %> + <%= link_to t(".copy_and_use"), signup_path(invitation_token: invitation.token) %> + <% end %> +
+
<%= t(".sent_to") %>
+
<% if invitation.redeemed_at %> -
<%= invitation.invitee_email %>
+ <%= invitation.invitee_email %> <% else %> -
<%= form_for(invitation) do |f| %> <%= error_messages_for invitation %>

<%= f.label :invitee_email, t(".email_address_label") %> <%= f.text_field :invitee_email %>

<%= hidden_field_tag :user_id, @user.try(:login) %>

<%= f.submit %>

<% end %> -
<% end %> -
Redeemed by
-
<%= invitee_link(invitation) %>
-
Created at
-
<%= invitation.created_at %>
-
Sent at
-
<%= invitation.sent_at %>
-
Redeemed at
-
<%= invitation.redeemed_at %>
+ +
<%= t(".redeemed_by") %>
+
<%= invitee_link(invitation) %>
+
<%= t(".created_at") %>
+
<%= invitation.created_at %>
+
<%= t(".sent_at") %>
+
<%= invitation.sent_at %>
+
<%= t(".redeemed_at") %>
+
<%= invitation.redeemed_at %>
diff --git a/config/locales/views/en.yml b/config/locales/views/en.yml index 3d2c6a20a1f..662cbcd158e 100644 --- a/config/locales/views/en.yml +++ b/config/locales/views/en.yml @@ -1358,7 +1358,18 @@ en: what_you_cant_do: What you can't do invitations: invitation: + copy_and_use: copy and use + copy_link: Copy link + created_at: Created at + deleted_user: deleted user (%{user_id}) email_address_label: Enter an email address + invitation_token: Invitation token + queue: queue + redeemed_at: Redeemed at + redeemed_by: Redeemed by + sender: Sender + sent_at: Sent at + sent_to: Sent to invite_requests: index_open: add_to_list: Add me to the list diff --git a/spec/helpers/invitations_helper_spec.rb b/spec/helpers/invitations_helper_spec.rb new file mode 100644 index 00000000000..f0d9580b4d7 --- /dev/null +++ b/spec/helpers/invitations_helper_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require "spec_helper" + +describe InvitationsHelper do + describe "#invitee_link" do + let(:invitation) { create(:invitation) } + + context "when the invitee is an existing user" do + let(:user) { create(:user) } + + before do + invitation.update!(invitee: user) + end + + it "returns a link to the user" do + expect(helper.invitee_link(invitation)).to eq(link_to(user.login, user_path(user))) + end + end + + context "when the invitee is a deleted user" do + let(:user) { create(:user) } + let!(:user_id) { user.id } + + before do + invitation.update!(invitee: user) + user.destroy! + invitation.reload + end + + it "returns a placeholder with the user ID" do + expect(helper.invitee_link(invitation)).to eq("deleted user (#{user_id})") + end + end + end +end From 6c11025c0caf9a81b3258f13adccb67aa1113d4c Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Mon, 23 Dec 2024 10:44:48 -0500 Subject: [PATCH 2/7] Hide ID for non admins --- app/helpers/invitations_helper.rb | 5 +++-- config/locales/views/en.yml | 3 ++- spec/helpers/invitations_helper_spec.rb | 16 ++++++++++++++-- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/app/helpers/invitations_helper.rb b/app/helpers/invitations_helper.rb index 5ce797057c0..1d8d3e7c9d9 100644 --- a/app/helpers/invitations_helper.rb +++ b/app/helpers/invitations_helper.rb @@ -12,8 +12,9 @@ def creator_link(invitation) def invitee_link(invitation) return unless invitation.invitee_type == "User" - return t("invitations.invitation.deleted_user", user_id: invitation.invitee_id) if invitation.invitee.blank? + return link_to(invitation.invitee.login, invitation.invitee) if invitation.invitee.present? + return t("invitations.invitation.deleted_user_with_id", user_id: invitation.invitee_id) if User.current_user.is_a?(Admin) - link_to(invitation.invitee.login, invitation.invitee) + t("invitations.invitation.deleted_user") end end diff --git a/config/locales/views/en.yml b/config/locales/views/en.yml index 662cbcd158e..2b3b2a6ade1 100644 --- a/config/locales/views/en.yml +++ b/config/locales/views/en.yml @@ -1361,7 +1361,8 @@ en: copy_and_use: copy and use copy_link: Copy link created_at: Created at - deleted_user: deleted user (%{user_id}) + deleted_user: deleted user + deleted_user_with_id: deleted user (%{user_id}) email_address_label: Enter an email address invitation_token: Invitation token queue: queue diff --git a/spec/helpers/invitations_helper_spec.rb b/spec/helpers/invitations_helper_spec.rb index f0d9580b4d7..e675c7cc4f7 100644 --- a/spec/helpers/invitations_helper_spec.rb +++ b/spec/helpers/invitations_helper_spec.rb @@ -20,7 +20,6 @@ context "when the invitee is a deleted user" do let(:user) { create(:user) } - let!(:user_id) { user.id } before do invitation.update!(invitee: user) @@ -29,7 +28,20 @@ end it "returns a placeholder with the user ID" do - expect(helper.invitee_link(invitation)).to eq("deleted user (#{user_id})") + expect(helper.invitee_link(invitation)).to eq("deleted user") + end + + context "when logged in as an admin" do + let(:admin) { build(:admin) } + let!(:user_id) { user.id } + + before do + User.current_user = admin + end + + it "returns a placeholder with the user ID" do + expect(helper.invitee_link(invitation)).to eq("deleted user (#{user_id})") + end end end end From da2b311da6e6b455ad1bbe65fe54a40c0b8cd18e Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Sun, 12 Jan 2025 22:24:49 -0500 Subject: [PATCH 3/7] Add AO3-5498 --- app/helpers/invitations_helper.rb | 12 ++++-- app/policies/invitation_policy.rb | 7 ++++ config/locales/views/en.yml | 4 +- features/admins/admin_invitations.feature | 25 ++++++++++++ features/other_a/invite_use.feature | 14 +++++++ features/step_definitions/invite_steps.rb | 12 ++++++ spec/helpers/invitations_helper_spec.rb | 48 ----------------------- 7 files changed, 69 insertions(+), 53 deletions(-) create mode 100644 app/policies/invitation_policy.rb delete mode 100644 spec/helpers/invitations_helper_spec.rb diff --git a/app/helpers/invitations_helper.rb b/app/helpers/invitations_helper.rb index 1d8d3e7c9d9..3d30d9b6fd6 100644 --- a/app/helpers/invitations_helper.rb +++ b/app/helpers/invitations_helper.rb @@ -12,9 +12,15 @@ def creator_link(invitation) def invitee_link(invitation) return unless invitation.invitee_type == "User" - return link_to(invitation.invitee.login, invitation.invitee) if invitation.invitee.present? - return t("invitations.invitation.deleted_user_with_id", user_id: invitation.invitee_id) if User.current_user.is_a?(Admin) - t("invitations.invitation.deleted_user") + if User.current_user.is_a?(Admin) && policy(invitation).access_invitee_details? + return t("invitations.invitation.user_id_deleted", user_id: invitation.invitee_id) if invitation.invitee.blank? + + return link_to(invitation.invitee.login, admin_user_path(invitation.invitee)) + end + + return t("invitations.invitation.deleted_user") if invitation.invitee.blank? + + link_to(invitation.invitee.login, invitation.invitee) if invitation.invitee.present? end end diff --git a/app/policies/invitation_policy.rb b/app/policies/invitation_policy.rb new file mode 100644 index 00000000000..8c9c3cb6a5b --- /dev/null +++ b/app/policies/invitation_policy.rb @@ -0,0 +1,7 @@ +class InvitationPolicy < ApplicationPolicy + EXTRA_INFO_ROLES = %w[superadmin open_doors policy_and_abuse support tag_wrangling].freeze + + def access_invitee_details? + user_has_roles?(EXTRA_INFO_ROLES) + end +end diff --git a/config/locales/views/en.yml b/config/locales/views/en.yml index 2b3b2a6ade1..135e5bff1ad 100644 --- a/config/locales/views/en.yml +++ b/config/locales/views/en.yml @@ -1361,8 +1361,7 @@ en: copy_and_use: copy and use copy_link: Copy link created_at: Created at - deleted_user: deleted user - deleted_user_with_id: deleted user (%{user_id}) + deleted_user: Deleted User email_address_label: Enter an email address invitation_token: Invitation token queue: queue @@ -1371,6 +1370,7 @@ en: sender: Sender sent_at: Sent at sent_to: Sent to + user_id_deleted: User %{user_id} (Deleted) invite_requests: index_open: add_to_list: Add me to the list diff --git a/features/admins/admin_invitations.feature b/features/admins/admin_invitations.feature index 30e5d1de468..5d5a09dea00 100644 --- a/features/admins/admin_invitations.feature +++ b/features/admins/admin_invitations.feature @@ -468,3 +468,28 @@ Feature: Admin Actions to Manage Invitations | position | email | | 1 | andy-jones@example.com | | 5 | eliot-jones@example.com | + + Scenario Outline: Viewing a user's invitation details + Given the user "creator" exists and is activated + And the user "invitee" exists and is activated + And an invitation created by "creator" and used by "invitee" + And I am logged in as a "" admin + When I view the most recent invitation for "creator" + Then I should see "invitee" + When I follow "invitee" + Then I should see "User: invitee" + When I am logged in as "invitee" + And "invitee" deletes their account + And I am logged in as a "" admin + And I view the most recent invitation for "creator" + Then I should see "User" + And I should see "(Deleted)" + But I should not see "invitee" + + Examples: + | role | + | superadmin | + | tag_wrangling | + | support | + | policy_and_abuse | + | open_doors | diff --git a/features/other_a/invite_use.feature b/features/other_a/invite_use.feature index a95c8038012..f54d6b15b3a 100644 --- a/features/other_a/invite_use.feature +++ b/features/other_a/invite_use.feature @@ -29,3 +29,17 @@ I want to use an invitation to create an account And I go to SOME_USER2's invitations page Then I should be on Scott's user page And I should see "Sorry, you don't have permission to access the page you were trying to reach." + + Scenario: Viewing invitation details when the invitee has deleted their account + Given the user "creator" exists and is activated + And the user "invitee" exists and is activated + And an invitation created by "creator" and used by "invitee" + And I am logged in as "creator" + When I view the most recent invitation for "creator" + Then I should see "invitee" + When I am logged in as "invitee" + And "invitee" deletes their account + And I am logged in as "creator" + And I view the most recent invitation for "creator" + Then I should see "Deleted User" + But I should not see "invitee" diff --git a/features/step_definitions/invite_steps.rb b/features/step_definitions/invite_steps.rb index 6adc74971ef..dc27c6936f6 100644 --- a/features/step_definitions/invite_steps.rb +++ b/features/step_definitions/invite_steps.rb @@ -95,6 +95,12 @@ def invite(attributes = {}) allow(InviteRequest).to receive(:per_page).and_return(amount) end +Given "an invitation created by {string} and used by {string}" do |creator, invitee| + creator = User.find_by(login: creator) + invitee = User.find_by(login: invitee) + FactoryBot.create(:invitation, creator:, invitee:) +end + ### WHEN When /^I use an invitation to sign up$/ do @@ -156,6 +162,12 @@ def invite(attributes = {}) click_button("Look me up") end +When "I view the most recent invitation for {string}" do |creator| + user = User.find_by(login: creator) + invitation = user.invitations.last + visit user_invitation_path(creator, invitation) +end + ### Then Then /^I should see how long I have to activate my account$/ do diff --git a/spec/helpers/invitations_helper_spec.rb b/spec/helpers/invitations_helper_spec.rb deleted file mode 100644 index e675c7cc4f7..00000000000 --- a/spec/helpers/invitations_helper_spec.rb +++ /dev/null @@ -1,48 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -describe InvitationsHelper do - describe "#invitee_link" do - let(:invitation) { create(:invitation) } - - context "when the invitee is an existing user" do - let(:user) { create(:user) } - - before do - invitation.update!(invitee: user) - end - - it "returns a link to the user" do - expect(helper.invitee_link(invitation)).to eq(link_to(user.login, user_path(user))) - end - end - - context "when the invitee is a deleted user" do - let(:user) { create(:user) } - - before do - invitation.update!(invitee: user) - user.destroy! - invitation.reload - end - - it "returns a placeholder with the user ID" do - expect(helper.invitee_link(invitation)).to eq("deleted user") - end - - context "when logged in as an admin" do - let(:admin) { build(:admin) } - let!(:user_id) { user.id } - - before do - User.current_user = admin - end - - it "returns a placeholder with the user ID" do - expect(helper.invitee_link(invitation)).to eq("deleted user (#{user_id})") - end - end - end - end -end From b0a439c57cc225b2f70b40db69b8e2b14eb921eb Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Sun, 12 Jan 2025 22:37:45 -0500 Subject: [PATCH 4/7] Ok rubocop --- features/step_definitions/invite_steps.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/step_definitions/invite_steps.rb b/features/step_definitions/invite_steps.rb index dc27c6936f6..2d9bfc867e8 100644 --- a/features/step_definitions/invite_steps.rb +++ b/features/step_definitions/invite_steps.rb @@ -98,7 +98,7 @@ def invite(attributes = {}) Given "an invitation created by {string} and used by {string}" do |creator, invitee| creator = User.find_by(login: creator) invitee = User.find_by(login: invitee) - FactoryBot.create(:invitation, creator:, invitee:) + FactoryBot.create(:invitation, creator: creator, invitee: invitee) end ### WHEN From 95382989daf2d04d0971f68ad4135aee3585ea30 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Mon, 13 Jan 2025 20:44:46 -0500 Subject: [PATCH 5/7] Add test for manage page --- features/admins/admin_invitations.feature | 7 ++++++- features/other_a/invite_use.feature | 7 ++++++- features/step_definitions/invite_steps.rb | 5 +++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/features/admins/admin_invitations.feature b/features/admins/admin_invitations.feature index 5d5a09dea00..35258386b8e 100644 --- a/features/admins/admin_invitations.feature +++ b/features/admins/admin_invitations.feature @@ -474,6 +474,8 @@ Feature: Admin Actions to Manage Invitations And the user "invitee" exists and is activated And an invitation created by "creator" and used by "invitee" And I am logged in as a "" admin + When I manage invitations belonging to "creator" + Then I should see "invitee" When I view the most recent invitation for "creator" Then I should see "invitee" When I follow "invitee" @@ -481,7 +483,10 @@ Feature: Admin Actions to Manage Invitations When I am logged in as "invitee" And "invitee" deletes their account And I am logged in as a "" admin - And I view the most recent invitation for "creator" + And I manage invitations belonging to "creator" + Then I should see "(Deleted)" + But I should not see "invitee" + When I view the most recent invitation for "creator" Then I should see "User" And I should see "(Deleted)" But I should not see "invitee" diff --git a/features/other_a/invite_use.feature b/features/other_a/invite_use.feature index f54d6b15b3a..48ac211e93e 100644 --- a/features/other_a/invite_use.feature +++ b/features/other_a/invite_use.feature @@ -35,11 +35,16 @@ I want to use an invitation to create an account And the user "invitee" exists and is activated And an invitation created by "creator" and used by "invitee" And I am logged in as "creator" + When I manage invitations belonging to "creator" + Then I should see "invitee" When I view the most recent invitation for "creator" Then I should see "invitee" When I am logged in as "invitee" And "invitee" deletes their account And I am logged in as "creator" - And I view the most recent invitation for "creator" + And I manage invitations belonging to "creator" + Then I should see "Deleted User" + But I should not see "invitee" + When I view the most recent invitation for "creator" Then I should see "Deleted User" But I should not see "invitee" diff --git a/features/step_definitions/invite_steps.rb b/features/step_definitions/invite_steps.rb index 2d9bfc867e8..1bec39ff856 100644 --- a/features/step_definitions/invite_steps.rb +++ b/features/step_definitions/invite_steps.rb @@ -162,6 +162,11 @@ def invite(attributes = {}) click_button("Look me up") end +When "I manage invitations belonging to {string}" do |creator| + user = User.find_by(login: creator) + visit manage_user_invitations_path(creator) +end + When "I view the most recent invitation for {string}" do |creator| user = User.find_by(login: creator) invitation = user.invitations.last From 33be6d0a5b5cd22e530146f3fe30810317315927 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Mon, 13 Jan 2025 20:47:05 -0500 Subject: [PATCH 6/7] Remove unused thing --- features/step_definitions/invite_steps.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/features/step_definitions/invite_steps.rb b/features/step_definitions/invite_steps.rb index 1bec39ff856..dc885433d1c 100644 --- a/features/step_definitions/invite_steps.rb +++ b/features/step_definitions/invite_steps.rb @@ -163,7 +163,6 @@ def invite(attributes = {}) end When "I manage invitations belonging to {string}" do |creator| - user = User.find_by(login: creator) visit manage_user_invitations_path(creator) end From 7e9461b62832cea355ae2659a0ffb9a340962601 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Fri, 24 Jan 2025 09:42:41 -0500 Subject: [PATCH 7/7] Use existing step --- features/admins/admin_invitations.feature | 4 ++-- features/other_a/invite_use.feature | 4 ++-- features/step_definitions/invite_steps.rb | 4 ---- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/features/admins/admin_invitations.feature b/features/admins/admin_invitations.feature index 35258386b8e..91694b0d324 100644 --- a/features/admins/admin_invitations.feature +++ b/features/admins/admin_invitations.feature @@ -474,7 +474,7 @@ Feature: Admin Actions to Manage Invitations And the user "invitee" exists and is activated And an invitation created by "creator" and used by "invitee" And I am logged in as a "" admin - When I manage invitations belonging to "creator" + When I go to creator's manage invitations page Then I should see "invitee" When I view the most recent invitation for "creator" Then I should see "invitee" @@ -483,7 +483,7 @@ Feature: Admin Actions to Manage Invitations When I am logged in as "invitee" And "invitee" deletes their account And I am logged in as a "" admin - And I manage invitations belonging to "creator" + And I go to creator's manage invitations page Then I should see "(Deleted)" But I should not see "invitee" When I view the most recent invitation for "creator" diff --git a/features/other_a/invite_use.feature b/features/other_a/invite_use.feature index 48ac211e93e..54054d89e6c 100644 --- a/features/other_a/invite_use.feature +++ b/features/other_a/invite_use.feature @@ -35,14 +35,14 @@ I want to use an invitation to create an account And the user "invitee" exists and is activated And an invitation created by "creator" and used by "invitee" And I am logged in as "creator" - When I manage invitations belonging to "creator" + When I go to creator's manage invitations page Then I should see "invitee" When I view the most recent invitation for "creator" Then I should see "invitee" When I am logged in as "invitee" And "invitee" deletes their account And I am logged in as "creator" - And I manage invitations belonging to "creator" + And I go to creator's manage invitations page Then I should see "Deleted User" But I should not see "invitee" When I view the most recent invitation for "creator" diff --git a/features/step_definitions/invite_steps.rb b/features/step_definitions/invite_steps.rb index dc885433d1c..2d9bfc867e8 100644 --- a/features/step_definitions/invite_steps.rb +++ b/features/step_definitions/invite_steps.rb @@ -162,10 +162,6 @@ def invite(attributes = {}) click_button("Look me up") end -When "I manage invitations belonging to {string}" do |creator| - visit manage_user_invitations_path(creator) -end - When "I view the most recent invitation for {string}" do |creator| user = User.find_by(login: creator) invitation = user.invitations.last