diff --git a/app/controllers/account_reset/send_notifications_controller.rb b/app/controllers/account_reset/send_notifications_controller.rb index 5626674c33e..313ff0ae168 100644 --- a/app/controllers/account_reset/send_notifications_controller.rb +++ b/app/controllers/account_reset/send_notifications_controller.rb @@ -1,10 +1,13 @@ +# This controller is not user-facing. It is only accessed by an AWS Lambda that +# is triggered by CloudWatch to run on a recurring basis. The Lambda is defined +# in lib/lambdas/account_reset_lambda.rb module AccountReset class SendNotificationsController < ApplicationController skip_before_action :verify_authenticity_token before_action :authorize def update - count = AccountResetService.grant_tokens_and_send_notifications + count = AccountReset::GrantRequestsAndSendEmails.new.call analytics.track_event(Analytics::ACCOUNT_RESET, event: :notifications, count: count) render plain: 'ok' end diff --git a/app/services/account_reset/grant_request.rb b/app/services/account_reset/grant_request.rb new file mode 100644 index 00000000000..8d0b33e9f26 --- /dev/null +++ b/app/services/account_reset/grant_request.rb @@ -0,0 +1,24 @@ +module AccountReset + class GrantRequest + def initialize(user) + @user_id = user.id + end + + def call + token = SecureRandom.uuid + arr = AccountResetRequest.find_by(user_id: @user_id) + arr.with_lock do + return false if arr.granted_token_valid? + account_reset_request.update(granted_at: Time.zone.now, + granted_token: token) + end + true + end + + private + + def account_reset_request + AccountResetRequest.find_or_create_by(user_id: @user_id) + end + end +end diff --git a/app/services/account_reset/grant_requests_and_send_emails.rb b/app/services/account_reset/grant_requests_and_send_emails.rb new file mode 100644 index 00000000000..6d21e2b99bb --- /dev/null +++ b/app/services/account_reset/grant_requests_and_send_emails.rb @@ -0,0 +1,33 @@ +module AccountReset + class GrantRequestsAndSendEmails + def call + notifications_sent = 0 + AccountResetRequest.where( + sql_query_for_users_eligible_to_delete_their_accounts, + tvalue: Time.zone.now - Figaro.env.account_reset_wait_period_days.to_i.days + ).order('requested_at ASC').each do |arr| + notifications_sent += 1 if grant_request_and_send_email(arr) + end + notifications_sent + end + + private + + def sql_query_for_users_eligible_to_delete_their_accounts + <<~SQL + cancelled_at IS NULL AND + granted_at IS NULL AND + requested_at < :tvalue AND + request_token IS NOT NULL AND + granted_token IS NULL + SQL + end + + def grant_request_and_send_email(arr) + user = arr.user + return false unless AccountReset::GrantRequest.new(user).call + UserMailer.account_reset_granted(user, arr.reload).deliver_later + true + end + end +end diff --git a/app/services/account_reset_service.rb b/app/services/account_reset_service.rb deleted file mode 100644 index 3dabb2ecbc3..00000000000 --- a/app/services/account_reset_service.rb +++ /dev/null @@ -1,52 +0,0 @@ -class AccountResetService - def initialize(user) - @user_id = user.id - end - - def grant_request - token = SecureRandom.uuid - arr = AccountResetRequest.find_by(user_id: @user_id) - arr.with_lock do - return false if arr.granted_token_valid? - account_reset_request.update(granted_at: Time.zone.now, - granted_token: token) - end - true - end - - def self.grant_tokens_and_send_notifications - users_sql = <<~SQL - cancelled_at IS NULL AND - granted_at IS NULL AND - requested_at < :tvalue AND - request_token IS NOT NULL AND - granted_token IS NULL - SQL - send_notifications_with_sql(users_sql) - end - - def self.send_notifications_with_sql(users_sql) - notifications_sent = 0 - AccountResetRequest.where( - users_sql, tvalue: Time.zone.now - Figaro.env.account_reset_wait_period_days.to_i.days - ).order('requested_at ASC').each do |arr| - notifications_sent += 1 if reset_and_notify(arr) - end - notifications_sent - end - private_class_method :send_notifications_with_sql - - def self.reset_and_notify(arr) - user = arr.user - return false unless AccountResetService.new(user).grant_request - UserMailer.account_reset_granted(user, arr.reload).deliver_later - true - end - private_class_method :reset_and_notify - - private - - def account_reset_request - AccountResetRequest.find_or_create_by(user_id: @user_id) - end -end diff --git a/lib/tasks/account_reset.rake b/lib/tasks/account_reset.rake index 5b8e5ceb4f2..03e014f54bb 100644 --- a/lib/tasks/account_reset.rake +++ b/lib/tasks/account_reset.rake @@ -1,6 +1,6 @@ namespace :account_reset do desc 'Send Notifications' task send_notifications: :environment do - AccountResetService.grant_tokens_and_send_notifications + AccountReset::GrantRequestsAndSendEmails.new.call end end diff --git a/spec/controllers/account_reset/delete_account_controller_spec.rb b/spec/controllers/account_reset/delete_account_controller_spec.rb index 2675a82b723..1923c3ff6ca 100644 --- a/spec/controllers/account_reset/delete_account_controller_spec.rb +++ b/spec/controllers/account_reset/delete_account_controller_spec.rb @@ -7,7 +7,7 @@ it 'logs a good token to the analytics' do user = create(:user) create_account_reset_request_for(user) - AccountResetService.new(user).grant_request + grant_request(user) session[:granted_token] = AccountResetRequest.all[0].granted_token stub_analytics @@ -63,7 +63,7 @@ it 'displays a flash and redirects to root if the token is expired' do user = create(:user) create_account_reset_request_for(user) - AccountResetService.new(user).grant_request + grant_request(user) stub_analytics properties = { @@ -114,7 +114,7 @@ it 'displays a flash and redirects to root if the token is expired' do user = create(:user) create_account_reset_request_for(user) - AccountResetService.new(user).grant_request + grant_request(user) stub_analytics properties = { diff --git a/spec/controllers/account_reset/send_notifications_controller_spec.rb b/spec/controllers/account_reset/send_notifications_controller_spec.rb index b32b17be624..c4a3dc37844 100644 --- a/spec/controllers/account_reset/send_notifications_controller_spec.rb +++ b/spec/controllers/account_reset/send_notifications_controller_spec.rb @@ -15,7 +15,9 @@ end it 'logs the number of notifications sent in the analytics' do - allow(AccountResetService).to receive(:grant_tokens_and_send_notifications).and_return(7) + service = instance_double(AccountReset::GrantRequestsAndSendEmails) + allow(AccountReset::GrantRequestsAndSendEmails).to receive(:new).and_return(service) + allow(service).to receive(:call).and_return(7) stub_analytics expect(@analytics).to receive(:track_event). diff --git a/spec/features/account_reset/cancel_request_spec.rb b/spec/features/account_reset/cancel_request_spec.rb index 131be607d8a..0596e0d54f7 100644 --- a/spec/features/account_reset/cancel_request_spec.rb +++ b/spec/features/account_reset/cancel_request_spec.rb @@ -35,7 +35,7 @@ reset_email Timecop.travel(Time.zone.now + 2.days) do - AccountResetService.grant_tokens_and_send_notifications + AccountReset::GrantRequestsAndSendEmails.new.call open_last_email click_email_link_matching(/cancel\?token/) click_button t('account_reset.cancel_request.cancel_button') diff --git a/spec/features/account_reset/delete_account_spec.rb b/spec/features/account_reset/delete_account_spec.rb index 5b435de013b..1439fe77fa5 100644 --- a/spec/features/account_reset/delete_account_spec.rb +++ b/spec/features/account_reset/delete_account_spec.rb @@ -24,7 +24,7 @@ reset_email Timecop.travel(Time.zone.now + 2.days) do - AccountResetService.grant_tokens_and_send_notifications + AccountReset::GrantRequestsAndSendEmails.new.call open_last_email click_email_link_matching(/delete_account\?token/) diff --git a/spec/services/account_reset/grant_request_spec.rb b/spec/services/account_reset/grant_request_spec.rb new file mode 100644 index 00000000000..fe85fc9dd56 --- /dev/null +++ b/spec/services/account_reset/grant_request_spec.rb @@ -0,0 +1,18 @@ +require 'rails_helper' + +describe AccountReset::GrantRequest do + include AccountResetHelper + + let(:user) { create(:user) } + let(:user2) { create(:user) } + + describe '#call' do + it 'adds a notified at timestamp and granted token to the user' do + create_account_reset_request_for(user) + AccountReset::GrantRequest.new(user).call + arr = AccountResetRequest.find_by(user_id: user.id) + expect(arr.granted_at).to be_present + expect(arr.granted_token).to be_present + end + end +end diff --git a/spec/services/account_reset_service_spec.rb b/spec/services/account_reset/grant_requests_and_send_emails_spec.rb similarity index 64% rename from spec/services/account_reset_service_spec.rb rename to spec/services/account_reset/grant_requests_and_send_emails_spec.rb index 6170845bc37..e46352b8789 100644 --- a/spec/services/account_reset_service_spec.rb +++ b/spec/services/account_reset/grant_requests_and_send_emails_spec.rb @@ -1,29 +1,19 @@ require 'rails_helper' -describe AccountResetService do +describe AccountReset::GrantRequestsAndSendEmails do include AccountResetHelper let(:user) { create(:user) } let(:user2) { create(:user) } - describe '#grant_request' do - it 'adds a notified at timestamp and granted token to the user' do - create_account_reset_request_for(user) - AccountResetService.new(user).grant_request - arr = AccountResetRequest.find_by(user_id: user.id) - expect(arr.granted_at).to be_present - expect(arr.granted_token).to be_present - end - end - - describe '.grant_tokens_and_send_notifications' do + describe '#call' do context 'after waiting the full wait period' do it 'does not send notifications when the notifications were already sent' do create_account_reset_request_for(user) after_waiting_the_full_wait_period do - AccountResetService.grant_tokens_and_send_notifications - notifications_sent = AccountResetService.grant_tokens_and_send_notifications + AccountReset::GrantRequestsAndSendEmails.new.call + notifications_sent = AccountReset::GrantRequestsAndSendEmails.new.call expect(notifications_sent).to eq(0) end end @@ -33,7 +23,7 @@ cancel_request_for(user) after_waiting_the_full_wait_period do - notifications_sent = AccountResetService.grant_tokens_and_send_notifications + notifications_sent = AccountReset::GrantRequestsAndSendEmails.new.call expect(notifications_sent).to eq(0) end end @@ -42,7 +32,7 @@ create_account_reset_request_for(user) after_waiting_the_full_wait_period do - notifications_sent = AccountResetService.grant_tokens_and_send_notifications + notifications_sent = AccountReset::GrantRequestsAndSendEmails.new.call expect(notifications_sent).to eq(1) end @@ -53,7 +43,7 @@ create_account_reset_request_for(user2) after_waiting_the_full_wait_period do - notifications_sent = AccountResetService.grant_tokens_and_send_notifications + notifications_sent = AccountReset::GrantRequestsAndSendEmails.new.call expect(notifications_sent).to eq(2) end @@ -64,7 +54,7 @@ it 'does not send notifications after a request' do create_account_reset_request_for(user) - notifications_sent = AccountResetService.grant_tokens_and_send_notifications + notifications_sent = AccountReset::GrantRequestsAndSendEmails.new.call expect(notifications_sent).to eq(0) end @@ -72,7 +62,7 @@ create_account_reset_request_for(user) cancel_request_for(user) - notifications_sent = AccountResetService.grant_tokens_and_send_notifications + notifications_sent = AccountReset::GrantRequestsAndSendEmails.new.call expect(notifications_sent).to eq(0) end end diff --git a/spec/support/account_reset_helper.rb b/spec/support/account_reset_helper.rb index 9176d3078c1..1e4e1f86319 100644 --- a/spec/support/account_reset_helper.rb +++ b/spec/support/account_reset_helper.rb @@ -9,4 +9,8 @@ def cancel_request_for(user) account_reset_request = AccountResetRequest.find_by(user_id: user.id) account_reset_request.update(cancelled_at: Time.zone.now) end + + def grant_request(user) + AccountReset::GrantRequest.new(user).call + end end