From 6c3041970f9bb9825847afa79f61e2e56f0fc506 Mon Sep 17 00:00:00 2001 From: weeklies <80141759+weeklies@users.noreply.github.com> Date: Sun, 3 Dec 2023 12:26:24 +0800 Subject: [PATCH] AO3-6609 Allow users to resend invitations (#4648) * Check for invitations in invite request status. * AO3-6609 Fill spec requirements * AO3-6609 Hound * AO3-6609 Houndilocks * AO3-6609 Reviewdog * Feedback * AO3-6609 Houndini * Apply suggestions Co-authored-by: Brian Austin <13002992+brianjaustin@users.noreply.github.com> * Fix spec Co-authored-by: Brian Austin <13002992+brianjaustin@users.noreply.github.com> * AO3-6609 Feedback * AO3-6609 Oops --------- Co-authored-by: tickinginstant Co-authored-by: Brian Austin <13002992+brianjaustin@users.noreply.github.com> --- app/controllers/invite_requests_controller.rb | 31 +++++++++-- app/models/invitation.rb | 47 +++++++++-------- app/views/invitations/_invitation.html.erb | 2 + .../invite_requests/_invitation.html.erb | 35 +++++++++++++ .../invite_requests/_no_invitation.html.erb | 3 ++ app/views/invite_requests/show.html.erb | 10 +++- app/views/invite_requests/show.js.erb | 4 +- config/config.yml | 2 + config/locales/controllers/en.yml | 5 ++ config/locales/models/en.yml | 5 ++ config/locales/views/en.yml | 21 ++++++++ config/routes.rb | 1 + ...1027172035_add_resent_at_to_invitations.rb | 7 +++ features/other_a/invite_queue.feature | 30 ++++++++++- .../invite_requests_controller_spec.rb | 51 ++++++++++++++----- 15 files changed, 211 insertions(+), 43 deletions(-) create mode 100644 app/views/invite_requests/_invitation.html.erb create mode 100644 app/views/invite_requests/_no_invitation.html.erb create mode 100644 db/migrate/20231027172035_add_resent_at_to_invitations.rb diff --git a/app/controllers/invite_requests_controller.rb b/app/controllers/invite_requests_controller.rb index ba1c69166f3..9d2f72430f2 100644 --- a/app/controllers/invite_requests_controller.rb +++ b/app/controllers/invite_requests_controller.rb @@ -9,17 +9,40 @@ def index # GET /invite_requests/1 def show @invite_request = InviteRequest.find_by(email: params[:email]) - @position_in_queue = @invite_request.position if @invite_request.present? - unless (request.xml_http_request?) || @invite_request - flash[:error] = "You can search for the email address you signed up with below. If you can't find it, your invitation may have already been emailed to that address; please check your email spam folder as your spam filters may have placed it there." - redirect_to status_invite_requests_path and return + + if @invite_request.present? + @position_in_queue = @invite_request.position + else + @invitation = Invitation.unredeemed.from_queue.find_by(invitee_email: params[:email]) end + respond_to do |format| format.html format.js end end + def resend + @invitation = Invitation.unredeemed.from_queue.find_by(invitee_email: params[:email]) + + if @invitation.nil? + flash[:error] = t("invite_requests.resend.not_found") + elsif !@invitation.can_resend? + flash[:error] = t("invite_requests.resend.not_yet", + count: ArchiveConfig.HOURS_BEFORE_RESEND_INVITATION) + else + @invitation.send_and_set_date(resend: true) + + if @invitation.errors.any? + flash[:error] = @invitation.errors.full_messages.first + else + flash[:notice] = t("invite_requests.resend.success", email: @invitation.invitee_email) + end + end + + redirect_to status_invite_requests_path + end + # POST /invite_requests def create unless AdminSetting.current.invite_from_queue_enabled? diff --git a/app/models/invitation.rb b/app/models/invitation.rb index cc7bb464a88..7e6801e9066 100644 --- a/app/models/invitation.rb +++ b/app/models/invitation.rb @@ -19,9 +19,10 @@ def recipient_is_not_registered scope :unsent, -> { where(invitee_email: nil, redeemed_at: nil) } scope :unredeemed, -> { where('invitee_email IS NOT NULL and redeemed_at IS NULL') } scope :redeemed, -> { where('redeemed_at IS NOT NULL') } + scope :from_queue, -> { where(external_author: nil).where(creator_type: [nil, "Admin"]) } before_validation :generate_token, on: :create - after_save :send_and_set_date + after_save :send_and_set_date, if: :saved_change_to_invitee_email? after_save :adjust_user_invite_status #Create a certain number of invitations for all valid users @@ -54,30 +55,34 @@ def mark_as_redeemed(user=nil) save end - private + def send_and_set_date(resend: false) + return if invitee_email.blank? - def generate_token - self.token = Digest::SHA1.hexdigest([Time.now, rand].join) + if self.external_author + archivist = self.external_author.external_creatorships.collect(&:archivist).collect(&:login).uniq.join(", ") + # send invite synchronously for now -- this should now work delayed but just to be safe + UserMailer.invitation_to_claim(self.id, archivist).deliver_now + else + # send invitations actively sent by a user synchronously to avoid delays + UserMailer.invitation(self.id).deliver_now + end + + date_column = resend ? :resent_at : :sent_at + # Skip callbacks within after_save by using update_column to avoid a callback loop + self.update_column(date_column, Time.current) + rescue StandardError => e + errors.add(:base, :notification_could_not_be_sent, error: e.message) + end + + def can_resend? + checked_date = self.resent_at || self.sent_at + checked_date < ArchiveConfig.HOURS_BEFORE_RESEND_INVITATION.hours.ago end - def send_and_set_date - if self.saved_change_to_invitee_email? && !self.invitee_email.blank? - begin - if self.external_author - archivist = self.external_author.external_creatorships.collect(&:archivist).collect(&:login).uniq.join(", ") - # send invite synchronously for now -- this should now work delayed but just to be safe - UserMailer.invitation_to_claim(self.id, archivist).deliver_now - else - # send invitations actively sent by a user synchronously to avoid delays - UserMailer.invitation(self.id).deliver_now - end + private - # Skip callbacks within after_save by using update_column to avoid a callback loop - self.update_column(:sent_at, Time.now) - rescue Exception => exception - errors.add(:base, "Notification email could not be sent: #{exception.message}") - end - end + def generate_token + self.token = Digest::SHA1.hexdigest([Time.current, rand].join) end #Update the user's out_of_invites status diff --git a/app/views/invitations/_invitation.html.erb b/app/views/invitations/_invitation.html.erb index f2cefdfb5c9..24fdc3b778b 100644 --- a/app/views/invitations/_invitation.html.erb +++ b/app/views/invitations/_invitation.html.erb @@ -24,6 +24,8 @@
<%= invitation.created_at %>
Sent at
<%= invitation.sent_at %>
+
Last resent at
+
<%= invitation.resent_at %>
Redeemed at
<%= invitation.redeemed_at %>
diff --git a/app/views/invite_requests/_invitation.html.erb b/app/views/invite_requests/_invitation.html.erb new file mode 100644 index 00000000000..88bfd5fcd09 --- /dev/null +++ b/app/views/invite_requests/_invitation.html.erb @@ -0,0 +1,35 @@ + +

+ <%= t(".title", email: invitation.invitee_email) %> +

+ + + +<% status = invitation.resent_at ? "resent" : "not_resent" %> +

+ <%= t(".info.#{status}", + sent_at: l(invitation.sent_at.to_date), + resent_at: invitation.resent_at ? l(invitation.resent_at.to_date) : nil) %> + <% if invitation.can_resend? %> + <%# i18n-tasks-use t("invite_requests.invitation.after_cooldown_period.not_resent") + i18n-tasks-use t("invite_requests.invitation.after_cooldown_period.resent")-%> + <%= t(".after_cooldown_period.#{status}", + count: ArchiveConfig.HOURS_BEFORE_RESEND_INVITATION, + contact_support_link: link_to(t(".contact_support"), new_feedback_report_path)) %> + <%= button_to t(".resend_button"), resend_invite_requests_path(email: invitation.invitee_email) %> + <% else %> + <%= t(".before_cooldown_period", count: ArchiveConfig.HOURS_BEFORE_RESEND_INVITATION) %> + <% end %> +

+ +<%# Correct heading size for JavaScript display %> +<%= content_for :footer_js do %> + +<% end %> + diff --git a/app/views/invite_requests/_no_invitation.html.erb b/app/views/invite_requests/_no_invitation.html.erb new file mode 100644 index 00000000000..dc55ef502f2 --- /dev/null +++ b/app/views/invite_requests/_no_invitation.html.erb @@ -0,0 +1,3 @@ +

+ <%= t(".email_not_found") %> +

diff --git a/app/views/invite_requests/show.html.erb b/app/views/invite_requests/show.html.erb index 3cc3537b524..aa36bc95fac 100644 --- a/app/views/invite_requests/show.html.erb +++ b/app/views/invite_requests/show.html.erb @@ -1,5 +1,11 @@ -<%= render "invite_request", invite_request: @invite_request %> +<% if @invite_request %> + <%= render "invite_request", invite_request: @invite_request %> +<% elsif @invitation %> + <%= render "invitation", invitation: @invitation %> +<% else %> + <%= render "no_invitation" %> +<% end %>

- <%= ts("To check on the status of your invitation, go to the %{status_page} and enter your email in the space provided!", status_page: link_to("Invitation Request Status page", status_invite_requests_path)).html_safe %> + <%= t(".instructions_html", status_link: link_to("Invitation Request Status page", status_invite_requests_path)) %>

diff --git a/app/views/invite_requests/show.js.erb b/app/views/invite_requests/show.js.erb index c6ead4bd5b0..ac4b7fe396f 100644 --- a/app/views/invite_requests/show.js.erb +++ b/app/views/invite_requests/show.js.erb @@ -1,5 +1,7 @@ <% if @invite_request %> $j("#invite-status").html("<%= escape_javascript(render "invite_requests/invite_request", invite_request: @invite_request) %>"); +<% elsif @invitation %> + $j("#invite-status").html("<%= escape_javascript(render "invitation", invitation: @invitation) %>"); <% else %> - $j("#invite-status").html("

Sorry, we can't find the email address you entered. If you had used it to join our invitation queue, it's possible that your invitation may have already been emailed to you; please check your spam folder, as your spam filters may have placed it there.

"); + $j("#invite-status").html("<%= escape_javascript(render "no_invitation") %>"); <% end %> diff --git a/config/config.yml b/config/config.yml index b0b022493ce..846ebcb45ed 100644 --- a/config/config.yml +++ b/config/config.yml @@ -78,6 +78,8 @@ DELIMITER_FOR_OUTPUT: ', ' INVITE_FROM_QUEUE_ENABLED: true INVITE_FROM_QUEUE_NUMBER: 10 INVITE_FROM_QUEUE_FREQUENCY: 7 + +HOURS_BEFORE_RESEND_INVITATION: 24 # this is whether or not people without invitations can create accounts ACCOUNT_CREATION_ENABLED: false DAYS_TO_PURGE_UNACTIVATED: 7 diff --git a/config/locales/controllers/en.yml b/config/locales/controllers/en.yml index 9b92f55418f..701779766bf 100644 --- a/config/locales/controllers/en.yml +++ b/config/locales/controllers/en.yml @@ -61,6 +61,11 @@ en: error: Sorry, that comment could not be unhidden. permission_denied: Sorry, you don't have permission to unhide that comment. success: Comment successfully unhidden! + invite_requests: + resend: + not_found: Could not find an invitation associated with that email. + not_yet: You cannot resend an invitation that was sent in the last %{count} hours. + success: Invitation resent to %{email}. kudos: create: success: Thank you for leaving kudos! diff --git a/config/locales/models/en.yml b/config/locales/models/en.yml index 094daa3290e..2bc3ecf93b4 100644 --- a/config/locales/models/en.yml +++ b/config/locales/models/en.yml @@ -115,6 +115,11 @@ en: attributes: user_defined_tags_count: at_most: must not add up to more than %{count}. You have entered %{value} of these tags, so you must remove %{diff} of them. + invitation: + attributes: + base: + format: "%{message}" + notification_could_not_be_sent: 'Notification email could not be sent: %{error}' kudo: attributes: commentable: diff --git a/config/locales/views/en.yml b/config/locales/views/en.yml index f21a12d21ee..040a8b695bb 100644 --- a/config/locales/views/en.yml +++ b/config/locales/views/en.yml @@ -505,10 +505,31 @@ en: invitation: email_address_label: Enter an email address invite_requests: + invitation: + after_cooldown_period: + not_resent: + one: Because your invitation was sent more than an hour ago, you can have your invitation resent. + other: Because your invitation was sent more than %{count} hours ago, you can have your invitation resent. + resent: + one: Because your invitation was resent more than an hour ago, you can have your invitation resent again, or you may want to %{contact_support_link}. + other: Because your invitation was resent more than %{count} hours ago, you can have your invitation resent again, or you may want to %{contact_support_link}. + before_cooldown_period: + one: If it has been more than an hour since you should have received your invitation, but you have not received it after checking your spam folder, you can visit this page to resend the invitation. + other: If it has been more than %{count} hours since you should have received your invitation, but you have not received it after checking your spam folder, you can visit this page to resend the invitation. + contact_support: contact Support + info: + not_resent: Your invitation was emailed to this address on %{sent_at}. If you can't find it, please check your email spam folder as your spam filters may have placed it there. + resent: Your invitation was emailed to this address on %{sent_at} and resent on %{resent_at}. If you can't find it, please check your email spam folder as your spam filters may have placed it there. + resend_button: Resend Invitation + title: Invitation Status for %{email} invite_request: date: 'At our current rate, you should receive an invitation on or around: %{date}.' position_html: You are currently number %{position} on our waiting list! title: Invitation Status for %{email} + no_invitation: + email_not_found: Sorry, we can't find the email address you entered. + show: + instructions_html: To check on the status of your invitation, go to the %{status_link} and enter your email in the space provided. kudos: guest_header: one: "%{count} guest has also left kudos" diff --git a/config/routes.rb b/config/routes.rb index e0c027fe8f0..1c2bc1a2352 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -81,6 +81,7 @@ collection do get :manage get :status + post :resend end end diff --git a/db/migrate/20231027172035_add_resent_at_to_invitations.rb b/db/migrate/20231027172035_add_resent_at_to_invitations.rb new file mode 100644 index 00000000000..c95aa1ff884 --- /dev/null +++ b/db/migrate/20231027172035_add_resent_at_to_invitations.rb @@ -0,0 +1,7 @@ +class AddResentAtToInvitations < ActiveRecord::Migration[6.1] + uses_departure! if Rails.env.staging? || Rails.env.production? + + def change + add_column :invitations, :resent_at, :datetime + end +end diff --git a/features/other_a/invite_queue.feature b/features/other_a/invite_queue.feature index 902b43cfdc6..12c6bfccd44 100644 --- a/features/other_a/invite_queue.feature +++ b/features/other_a/invite_queue.feature @@ -57,7 +57,7 @@ Feature: Invite queue management # check your place in the queue - invalid address When I check how long "testttt@archiveofourown.org" will have to wait in the invite request queue Then I should see "Invitation Request Status" - And I should see "If you can't find it, your invitation may have already been emailed to that address; please check your email spam folder as your spam filters may have placed it there." + And I should see "Sorry, we can't find the email address you entered." And I should not see "You are currently number" # check your place in the queue - correct address @@ -98,7 +98,7 @@ Feature: Invite queue management Then 1 email should be delivered to test@archiveofourown.org When I check how long "test@archiveofourown.org" will have to wait in the invite request queue Then I should see "Invitation Request Status" - And I should see "If you can't find it, your invitation may have already been emailed to that address;" + And I should see "If you can't find it, please check your email spam folder as your spam filters may have placed it there." # invite can be used When I am logged in as an admin @@ -155,3 +155,29 @@ Feature: Invite queue management And I fill in "invite_request_email" with "fred@bedrock.com" And I press "Add me to the list" Then I should see "Email is already being used by an account holder." + + Scenario: Users can resend their invitation after enough time has passed + Given account creation is enabled + And the invitation queue is enabled + And account creation requires an invitation + And the invite_from_queue_at is yesterday + And an invitation request for "invitee@example.org" + When the scheduled check_invite_queue job is run + Then 1 email should be delivered to invitee@example.org + + When I check how long "invitee@example.org" will have to wait in the invite request queue + Then I should see "Invitation Request Status" + And I should see "If you can't find it, please check your email spam folder as your spam filters may have placed it there." + And I should not see "Because your invitation was sent more than 24 hours ago, you can have your invitation resent." + And I should not see a "Resend Invitation" button + + When all emails have been delivered + And it is currently 25 hours from now + And I check how long "invitee@example.org" will have to wait in the invite request queue + Then I should see "Invitation Request Status" + And I should see "If you can't find it, please check your email spam folder as your spam filters may have placed it there." + And I should see "Because your invitation was sent more than 24 hours ago, you can have your invitation resent." + And I should see a "Resend Invitation" button + + When I press "Resend Invitation" + Then 1 email should be delivered to invitee@example.org diff --git a/spec/controllers/invite_requests_controller_spec.rb b/spec/controllers/invite_requests_controller_spec.rb index c7d42457936..e223ba681ea 100644 --- a/spec/controllers/invite_requests_controller_spec.rb +++ b/spec/controllers/invite_requests_controller_spec.rb @@ -17,21 +17,14 @@ describe "GET #show" do context "when given invalid emails" do - it "redirects to index with error" do - message = "You can search for the email address you signed up with below. If you can't find it, your invitation may have already been emailed to that address; please check your email spam folder as your spam filters may have placed it there." - get :show, params: { id: 0 } - it_redirects_to_with_error(status_invite_requests_path, message) - expect(assigns(:invite_request)).to be_nil - get :show, params: { id: 0, email: "mistressofallevil@example.org" } - it_redirects_to_with_error(status_invite_requests_path, message) + it "renders" do + get :show, params: { email: "mistressofallevil@example.org" } + expect(response).to render_template("show") expect(assigns(:invite_request)).to be_nil end it "renders for an ajax call" do - get :show, params: { id: 0 }, xhr: true - expect(response).to render_template("show") - expect(assigns(:invite_request)).to be_nil - get :show, params: { id: 0, email: "mistressofallevil@example.org" }, xhr: true + get :show, params: { email: "mistressofallevil@example.org" }, xhr: true expect(response).to render_template("show") expect(assigns(:invite_request)).to be_nil end @@ -41,19 +34,51 @@ let(:invite_request) { create(:invite_request) } it "renders" do - get :show, params: { id: 0, email: invite_request.email } + get :show, params: { email: invite_request.email } expect(response).to render_template("show") expect(assigns(:invite_request)).to eq(invite_request) end it "renders for an ajax call" do - get :show, params: { id: 0, email: invite_request.email }, xhr: true + get :show, params: { email: invite_request.email }, xhr: true expect(response).to render_template("show") expect(assigns(:invite_request)).to eq(invite_request) end end end + describe "POST #resend" do + context "when the email doesn't match any invitations" do + it "redirects with an error" do + post :resend, params: { email: "test@example.org" } + it_redirects_to_with_error(status_invite_requests_path, + "Could not find an invitation associated with that email.") + end + end + + context "when the invitation is too recent" do + let(:invitation) { create(:invitation) } + + it "redirects with an error" do + post :resend, params: { email: invitation.invitee_email } + it_redirects_to_with_error(status_invite_requests_path, + "You cannot resend an invitation that was sent in the last 24 hours.") + end + end + + context "when the email and time are valid" do + let!(:invitation) { create(:invitation) } + + it "redirects with a success message" do + travel_to((1 + ArchiveConfig.HOURS_BEFORE_RESEND_INVITATION).hours.from_now) + post :resend, params: { email: invitation.invitee_email } + + it_redirects_to_with_notice(status_invite_requests_path, + "Invitation resent to #{invitation.invitee_email}.") + end + end + end + describe "POST #create" do it "redirects to index with error given invalid emails" do post :create, params: { invite_request: { email: "wat" } }