LG-666 Refactor AccountResetService

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

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

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

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

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 = {

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).

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')

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/)
 

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

spec/services/account_reset_service_spec.rb → 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,15 +54,15 @@
       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
 
       it 'does not send notifications when the request was cancelled' do
         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

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