refactor sheerid webhook code, do not trigger on confirmed

app/handlers/newflow/educator_signup/sheerid_webhook.rb

@@ -3,144 +3,128 @@ module EducatorSignup
     class SheeridWebhook
       lev_handler
 
-      protected ###############
+      protected
 
       def authorized?
         true
       end
 
       def handle(verification_id=nil)
-        unless verification_id
-          verification_id = params.fetch('verificationId')
-        end
-        verification_details_from_sheerid = SheeridAPI.get_verification_details(verification_id)
+        verification_id ||= params.fetch('verificationId')
+        verification_details = fetch_verification_details(verification_id)
+        return unless verification_details
+
+        verification = find_or_initialize_verification(verification_id, verification_details)
+        user = find_user_by_email(verification.email)
+        return unless user
+
+        log_webhook_received(user)
+        handle_user_verification(user, verification_id, verification_details)
+        update_user_with_verification_data(user, verification, verification_details)
+        process_verification_step(user, verification_id, verification_details)
+
+        CreateOrUpdateSalesforceLead.perform_later(user: user)
+        log_webhook_processed(user, verification_id, verification_details)
+        outputs.verification_id = verification_id
+      end
 
-        # there are no details included with this step that are helpful a future
-        # TODO: might be to use this to update the user faculty state to PENDING_SHEERID or AWAITING_DOC_UPLOAD?
-        return if verification_details_from_sheerid.current_step == 'error'
-        return if verification_details_from_sheerid.current_step == 'collectTeacherPersonalInfo'
+      private
 
-        if !verification_details_from_sheerid.success?
+      def fetch_verification_details(verification_id)
+        details = SheeridAPI.get_verification_details(verification_id)
+        unless details.success?
           Sentry.capture_message("[SheerID Webhook] fetching verification details FAILED",
-                                 extra: { verification_id: verification_id, verification_details: verification_details_from_sheerid }
-          )
+                                 extra: { verification_id: verification_id, verification_details: details })
           fatal_error(code: :sheerid_api_call_failed)
+          return nil
         end
+        details
+      end
 
-        # grab the details from what SheerID sends back and add them to the verification object
-        verification = SheeridVerification.find_or_initialize_by(verification_id: verification_id)
-        verification.email = verification_details_from_sheerid.email
-        verification.current_step = verification_details_from_sheerid.current_step
-        verification.first_name = verification_details_from_sheerid.first_name
-        verification.last_name = verification_details_from_sheerid.last_name
-        verification.organization_name = verification_details_from_sheerid.organization_name
-        verification.save
-
-        user = EmailAddress.verified.find_by(value: verification.email)&.user
-
-        if !user.present?
-          Sentry.capture_message("[SheerID Webhook] No user found with verification id (#{verification_id}) and email (#{verification.email})",
-                                 extra: { verification_id: verification_id, verification_details_from_sheer_id: verification_details_from_sheerid }
+      def find_or_initialize_verification(verification_id, details)
+        SheeridVerification.find_or_initialize_by(verification_id: verification_id).tap do |verification|
+          verification.assign_attributes(
+            email: details.email,
+            current_step: details.current_step,
+            first_name: details.first_name,
+            last_name: details.last_name,
+            organization_name: details.organization_name
           )
-          return
+          verification.save
+        end
+      end
+
+      def find_user_by_email(email)
+        EmailAddress.find_by(value: email)&.user.tap do |user|
+          unless user
+            Sentry.capture_message("[SheerID Webhook] No user found with email (#{email})")
+          end
         end
+      end
 
-        # update the security log and the user to say we got the webhook - we use this in lead processing
+      def log_webhook_received(user)
         SecurityLog.create!(event_type: :sheerid_webhook_received, user: user)
+      end
 
-        # Set the user's sheerid_verification_id only if they didn't already have one  we don't want to overwrite the approved one
-        if verification_id.present? && user.sheerid_verification_id.blank? && user.sheerid_verification_id != verification_id
+      def handle_user_verification(user, verification_id, details)
+        if user.faculty_status == User::CONFIRMED_FACULTY
+          SecurityLog.create!(event_type: :sheerid_webhook_ignored, user: user, event_data: { reason: "User already confirmed" })
+        elsif verification_id.present? && user.sheerid_verification_id.blank? && user.sheerid_verification_id != verification_id
           user.update!(sheerid_verification_id: verification_id)
-
-          SecurityLog.create!(
-            event_type: :sheerid_verification_id_added_to_user_from_webhook,
-            user: user,
-            event_data: { verification_id: verification_id }
-          )
+          SecurityLog.create!(event_type: :sheerid_verification_id_added_to_user_from_webhook, user: user, event_data: { verification_id: verification_id })
         else
-          SecurityLog.create!(
-            event_type: :sheerid_conflicting_verification_id,
-            user: user,
-            event_data: { verification_id: verification_id }
-          )
+          SecurityLog.create!(event_type: :sheerid_conflicting_verification_id, user: user, event_data: { verification_id: verification_id })
         end
+      end
 
+      def update_user_with_verification_data(user, verification, details)
+        return unless details.relevant?
+
+        user.update!(
+          first_name: verification.first_name,
+          last_name: verification.last_name,
+          sheerid_reported_school: verification.organization_name,
+          faculty_status: verification.current_step_to_faculty_status,
+          sheer_id_webhook_received: true,
+          school: find_or_fuzzy_match_school(verification.organization_name)
+        )
+        SecurityLog.create!(event_type: :school_added_to_user_from_sheerid_webhook, user: user, event_data: { school: user.school })
+      end
 
-        # Update the user account with the data returned from SheerID
-        if verification_details_from_sheerid.relevant?
-          user.first_name = verification.first_name
-          user.last_name = verification.last_name
-          user.sheerid_reported_school = verification.organization_name
-          user.faculty_status = verification.current_step_to_faculty_status
-          user.sheer_id_webhook_received = true
-
-          # Attempt to exactly match a school based on the sheerid_reported_school field
-          school = School.find_by sheerid_school_name: user.sheerid_reported_school
-
-          if school.nil?
-            # No exact match found, so attempt to fuzzy match the school name
-            match = SheeridAPI::SHEERID_REGEX.match user.sheerid_reported_school
-            name = match[1]
-            city = match[2]
-            state = match[3]
-
-            # Sometimes the city and/or state are duplicated, so remove them
-            name = name.chomp(" (#{city})") unless city.nil?
-            name = name.chomp(" (#{state})") unless state.nil?
-            name = name.chomp(" (#{city}, #{state})") unless city.nil? || state.nil?
-
-            # For Homeschool, the city is "Any" and the state is missing
-            city = nil if city == 'Any'
-
-            school = School.fuzzy_search name, city, state
-          end
-
-          user.school = school
+      def find_or_fuzzy_match_school(school_name)
+        School.find_by(sheerid_school_name: school_name) || fuzzy_match_school(school_name)
+      end
 
-          SecurityLog.create!(
-            event_type: :school_added_to_user_from_sheerid_webhook,
-            user: user,
-            event_data: { school: school }
-          )
-        end
+      def fuzzy_match_school(school_name)
+        match = SheeridAPI::SHEERID_REGEX.match(school_name)
+        name, city, state = match[1], match[2], match[3]
+        name = name.chomp(" (#{city})").chomp(" (#{state})").chomp(" (#{city}, #{state})")
+        city = nil if city == 'Any'
+        School.fuzzy_search(name, city, state)
+      end
 
-        if verification.current_step == 'rejected'
-          user.update!(faculty_status: User::REJECTED_BY_SHEERID, sheerid_verification_id: verification_id)
-          SecurityLog.create!(
-            event_type: :fv_reject_by_sheerid,
-            user: user,
-            event_data: { verification_id: verification_id })
-        elsif verification.current_step == 'success'
-          user.update!(faculty_status: User::CONFIRMED_FACULTY, sheerid_verification_id: verification_id)
-          SecurityLog.create!(
-            event_type: :fv_success_by_sheerid,
-            user: user,
-            event_data: { verification_id: verification_id })
-        elsif verification.current_step == 'collectTeacherPersonalInfo'
-          user.update!(faculty_status: User::PENDING_SHEERID, sheerid_verification_id: verification_id)
-          SecurityLog.create!(
-            event_type: :sheerid_webhook_request_more_info,
-            user: user,
-            event_data: { verification: verification_details_from_sheerid.inspect })
-        elsif verification.current_step == 'error'
-          user.update!(sheerid_verification_id: verification_id)
-          SecurityLog.create!(
-            event_type: :sheerid_error,
-            user: user,
-            event_data: { verification: verification_details_from_sheerid.inspect })
+      def process_verification_step(user, verification_id, details)
+        case details.current_step
+        when 'rejected'
+          update_user_status(user, User::REJECTED_BY_SHEERID, verification_id, :fv_reject_by_sheerid)
+        when 'success'
+          update_user_status(user, User::CONFIRMED_FACULTY, verification_id, :fv_success_by_sheerid)
+        when 'collectTeacherPersonalInfo'
+          update_user_status(user, User::PENDING_SHEERID, verification_id, :sheerid_webhook_request_more_info, details.inspect)
+        when 'error'
+          update_user_status(user, nil, verification_id, :sheerid_error, details.inspect)
         else
-          user.update!(sheerid_verification_id: verification_id)
-          SecurityLog.create!(
-            event_type: :unknown_sheerid_response,
-            user: user,
-            event_data: { verification: verification_details_from_sheerid.inspect })
+          update_user_status(user, nil, verification_id, :unknown_sheerid_response, details.inspect)
         end
+      end
 
-        CreateOrUpdateSalesforceLead.perform_later(user: user)
-
+      def update_user_status(user, status, verification_id, event_type, event_data = nil)
+        user.update!(faculty_status: status, sheerid_verification_id: verification_id)
+        SecurityLog.create!(event_type: event_type, user: user, event_data: { verification_id: verification_id, verification: event_data })
+      end
 
-        SecurityLog.create!(user: user, event_type: :sheerid_webhook_processed)
-        outputs.verification_id = verification_id
+      def log_webhook_processed(user, verification_id, details)
+        SecurityLog.create!(event_type: :sheerid_webhook_processed, user: user, event_data: { verification_id: verification_id, verification_details: details.inspect, faculty_status: user.faculty_status })
       end
     end
   end

app/models/security_log.rb

@@ -107,6 +107,8 @@ class SecurityLog < ApplicationRecord
     attempted_to_add_school_not_cached_yet
     school_added_to_user_from_sheerid_webhook
     user_lead_id_updated_from_salesforce
+    sheerid_webhook_ignored
+    sheerid_api_call_failed
   ]
 
   json_serialize :event_data, Hash

lib/sheerid_api/constants.rb

@@ -0,0 +1,10 @@
+module SheeridAPI
+  module Constants
+    AUTHORIZATION_HEADER = "Bearer #{Rails.application.secrets.sheerid_api_secret}"
+    HEADERS = {
+      'Authorization': AUTHORIZATION_HEADER,
+      'Accept': 'application/json',
+      'Content-Type': 'application/json'
+    }.freeze
+  end
+end

lib/sheerid_api/request.rb

@@ -1,14 +1,8 @@
+require_relative 'constants'
+
 module SheeridAPI
   class Request
-
-    AUTHORIZATION_HEADER = "Bearer #{Rails.application.secrets.sheerid_api_secret}"
-    HEADERS = {
-      'Authorization': AUTHORIZATION_HEADER,
-      'Accept': 'application/json',
-      'Content-Type': 'application/json'
-    }.freeze
-
-    private_constant(:AUTHORIZATION_HEADER, :HEADERS)
+    include Constants
 
     def initialize(http_method, url, request_body = nil)
       @http_method = http_method
@@ -20,28 +14,43 @@ def response
       @response ||= call_api
     end
 
-    private #################
+    private
 
     def call_api
-      http_response = Faraday.send(@http_method, @url, @request_body, HEADERS)
-      return Response.new(parse_body(http_response.body))
-    rescue Net::ReadTimeout => ee
-      message = 'SheeridAPI: timeout'
-      Sentry.capture_message(message)
-      Rails.logger.warn(message)
-      return NullResponse.instance
+      http_response = send_request
+      Response.new(parse_body(http_response.body))
+    rescue Net::ReadTimeout
+      handle_timeout
     rescue => ee
-      # We don't want explosions here to trickle out and impact callers
-      Sentry.capture_exception(ee)
-      Rails.logger.warn(ee)
-      return NullResponse.instance
+      handle_exception(ee)
     end
 
-    private
+    def send_request
+      case @http_method
+      when :get
+        Faraday.get(@url, @request_body, HEADERS)
+      when :post
+        Faraday.post(@url, @request_body, HEADERS)
+      else
+        raise ArgumentError, "Unsupported HTTP method: #{@http_method}"
+      end
+    end
 
     def parse_body(response)
       JSON.parse(response).to_h
     end
 
+    def handle_timeout
+      message = 'SheeridAPI: timeout'
+      Sentry.capture_message(message)
+      Rails.logger.warn(message)
+      NullResponse.instance
+    end
+
+    def handle_exception(exception)
+      Sentry.capture_exception(exception)
+      Rails.logger.warn(exception)
+      NullResponse.instance
+    end
   end
 end

spec/helpers/newflow/sheerid_webhook_spec.rb

@@ -0,0 +1,39 @@
+require 'rails_helper'
+
+RSpec.describe Newflow::EducatorSignup::SheeridWebhook, type: :routine do
+  let(:verification_id) { 'test_verification_id' }
+  let(:details) { double('details', success?: true, email: '[email protected]', current_step: 'success', first_name: 'John', last_name: 'Doe', organization_name: 'Test School') }
+  let(:user) { create_newflow_user('[email protected]', 'password', terms_agreed: true, role: 'instructor') }
+  let(:verification) { create(:sheerid_verification, verification_id: verification_id, email: '[email protected]') }
+
+  before do
+    allow(SheeridAPI).to receive(:get_verification_details).with(verification_id).and_return(details)
+    allow(EmailAddress).to receive_message_chain(:verified, :find_by).with(value: '[email protected]').and_return(user)
+  end
+
+  describe '#fetch_verification_details' do
+    context 'when the API call is successful' do
+      it 'returns the verification details' do
+        result = subject.send(:fetch_verification_details, verification_id)
+        expect(result).to eq(details)
+      end
+    end
+
+    context 'when the API call fails' do
+      let(:details) { double('details', success?: false) }
+
+      before do
+        allow(subject).to receive(:fatal_error).and_return(nil)
+      end
+
+      it 'logs an error and returns nil' do
+        expect(Sentry).to receive(:capture_message).with(
+          "[SheerID Webhook] fetching verification details FAILED",
+          extra: { verification_id: verification_id, verification_details: details }
+        )
+        result = subject.send(:fetch_verification_details, verification_id)
+        expect(result).to be_nil
+      end
+    end
+  end
+end