LG-4398: Support ActiveModel::Errors instance for FormResponse (#5048)

* LG-4398: Support ActiveModel::Errors instance for FormResponse **Why**: An ActiveModel::Errors instance can represent both the "message" and "detail" of an error, where the message is the translated, human-readable text shown to the user, and the detail is typically the locale-independent, machine-readable unique identifier of a particular error. By supporting an ActiveModel::Errors instance in FormResponse, we can retain current behaviors of showing and logging a human-readable error message, while also allowing us to identify and group instances of an error across locales in a CloudWatch query. With few exceptions, this also aligns to how we most often use FormResponse, where it's initialized using errors from an instance of ActiveModel::Errors. The difference is a simplification to pass the errors instance itself, rather than the result of its "messages" method (i.e. `errors: errors` instead of `errors: errors.messages`). It may be possible to consolidate to avoid overloading and instead supporting _only_ the ActiveModel::Errors instance in this class, but (a) there are a handful of existing cases where we create hashes ad-hoc for logging and (b) in those cases, the ActiveModel::Errors equivalent implementation is relatively clunky. * Convert existing FormResponse to leverage ActiveModel::Errors support, omitting errors * Set error_details as private accessor in FormResponse * Update specs to accommodate new FormResponse usage, error_details value
18F · May 13, 2021 · 9b87bd7 · 9b87bd7
1 parent 84a8ca9
commit 9b87bd7
Show file tree

Hide file tree

Showing 93 changed files with 548 additions and 352 deletions.
diff --git a/app/controllers/event_disavowal_controller.rb b/app/controllers/event_disavowal_controller.rb
@@ -8,7 +8,6 @@ def new
       Analytics::EVENT_DISAVOWAL,
       FormResponse.new(
         success: true,
-        errors: {},
         extra: EventDisavowal::BuildDisavowedEventAnalyticsAttributes.call(disavowed_event),
       ).to_h,
     )

diff --git a/app/controllers/users/personal_keys_controller.rb b/app/controllers/users/personal_keys_controller.rb
@@ -65,7 +65,6 @@ def send_new_personal_key_notifications
     def form_response(emails:, telephony_responses:)
       FormResponse.new(
         success: true,
-        errors: {},
         extra: {
           emails: emails.count,
           sms_message_ids: telephony_responses.map { |resp| resp.to_h[:message_id] },

diff --git a/app/forms/add_user_email_form.rb b/app/forms/add_user_email_form.rb
@@ -23,7 +23,7 @@ def submit(user, params)
       @success = false
     end
 
-    FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+    FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
   end
 
   def email_address_record(email)

diff --git a/app/forms/backup_code_setup_form.rb b/app/forms/backup_code_setup_form.rb
@@ -8,7 +8,7 @@ def initialize(user)
   end
 
   def submit
-    FormResponse.new(success: valid?, errors: errors.messages, extra: extra_analytics_attributes)
+    FormResponse.new(success: valid?, errors: errors, extra: extra_analytics_attributes)
   end
 
   private

diff --git a/app/forms/backup_code_verification_form.rb b/app/forms/backup_code_verification_form.rb
@@ -10,7 +10,6 @@ def submit(params)
     @backup_code = params[:backup_code]
     FormResponse.new(
       success: valid_backup_code?,
-      errors: {},
       extra: extra_analytics_attributes,
     )
   end

diff --git a/app/forms/delete_user_email_form.rb b/app/forms/delete_user_email_form.rb
@@ -13,7 +13,7 @@ def initialize(user, email_address)
   def submit
     success = valid? && email_address_destroyed
     notify_subscribers if success
-    FormResponse.new(success: success, errors: errors.messages)
+    FormResponse.new(success: success, errors: errors)
   end
 
   private

diff --git a/app/forms/edit_phone_form.rb b/app/forms/edit_phone_form.rb
@@ -16,7 +16,7 @@ def submit(params)
     ingest_submitted_params(params)
     success = valid?
     update_phone_configuration if success
-    FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+    FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
   end
 
   def masked_number

diff --git a/app/forms/event_disavowal/password_reset_from_disavowal_form.rb b/app/forms/event_disavowal/password_reset_from_disavowal_form.rb
@@ -15,7 +15,7 @@ def submit(params)
 
       success = valid?
       handle_valid_password if success
-      FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+      FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
     end
 
     private

diff --git a/app/forms/idv/address_form.rb b/app/forms/idv/address_form.rb
@@ -18,7 +18,7 @@ def initialize(user)
     def submit(params)
       consume_params(params)
 
-      FormResponse.new(success: valid?, errors: errors.messages)
+      FormResponse.new(success: valid?, errors: errors)
     end
 
     private

diff --git a/app/forms/idv/api_document_verification_form.rb b/app/forms/idv/api_document_verification_form.rb
@@ -23,7 +23,7 @@ def submit
 
       FormResponse.new(
         success: valid?,
-        errors: errors.messages,
+        errors: errors,
         extra: {
           remaining_attempts: remaining_attempts,
         },

diff --git a/app/forms/idv/api_document_verification_status_form.rb b/app/forms/idv/api_document_verification_status_form.rb
@@ -14,7 +14,7 @@ def initialize(async_state:, document_capture_session:)
     def submit
       FormResponse.new(
         success: valid?,
-        errors: errors.messages,
+        errors: errors,
         extra: {
           remaining_attempts: remaining_attempts,
         },

diff --git a/app/forms/idv/api_image_upload_form.rb b/app/forms/idv/api_image_upload_form.rb
@@ -54,7 +54,7 @@ def throttled_else_increment
     def validate_form
       response = Idv::DocAuthFormResponse.new(
         success: valid?,
-        errors: errors.messages,
+        errors: errors,
         extra: extra_attributes,
       )
 

diff --git a/app/forms/idv/consent_form.rb b/app/forms/idv/consent_form.rb
@@ -7,7 +7,7 @@ class ConsentForm
     def submit(params)
       @ial2_consent_given = params[:ial2_consent_given] == 'true'
 
-      FormResponse.new(success: valid?, errors: errors.messages)
+      FormResponse.new(success: valid?, errors: errors)
     end
 
     def ial2_consent_given?

diff --git a/app/forms/idv/doc_pii_form.rb b/app/forms/idv/doc_pii_form.rb
@@ -16,7 +16,7 @@ def initialize(pii)
     def submit
       Idv::DocAuthFormResponse.new(
         success: valid?,
-        errors: errors.messages,
+        errors: errors,
       )
     end
 

diff --git a/app/forms/idv/document_capture_form.rb b/app/forms/idv/document_capture_form.rb
@@ -26,7 +26,7 @@ def self.model_name
     def submit(params)
       consume_params(params)
 
-      FormResponse.new(success: valid?, errors: errors.messages, extra: extra_analytics_attributes)
+      FormResponse.new(success: valid?, errors: errors, extra: extra_analytics_attributes)
     end
 
     private

diff --git a/app/forms/idv/otp_delivery_method_form.rb b/app/forms/idv/otp_delivery_method_form.rb
@@ -8,7 +8,7 @@ class OtpDeliveryMethodForm
 
     def submit(params)
       self.otp_delivery_preference = params[:otp_delivery_preference]
-      FormResponse.new(success: valid?, errors: errors.messages, extra: extra_analytics_attributes)
+      FormResponse.new(success: valid?, errors: errors, extra: extra_analytics_attributes)
     end
 
     private

diff --git a/app/forms/idv/phone_confirmation_otp_verification_form.rb b/app/forms/idv/phone_confirmation_otp_verification_form.rb
@@ -15,7 +15,7 @@ def submit(code:)
       else
         increment_second_factor_attempts
       end
-      FormResponse.new(success: success, errors: {}, extra: extra_analytics_attributes)
+      FormResponse.new(success: success, extra: extra_analytics_attributes)
     end
 
     private

diff --git a/app/forms/idv/phone_form.rb b/app/forms/idv/phone_form.rb
@@ -17,7 +17,7 @@ def submit(params)
       success = valid?
       self.phone = params[:phone] unless success
 
-      FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+      FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
     end
 
     def user_has_multiple_phones?

diff --git a/app/forms/idv/ssn_form.rb b/app/forms/idv/ssn_form.rb
@@ -22,7 +22,7 @@ def initialize(user)
     def submit(params)
       consume_params(params)
 
-      FormResponse.new(success: valid?, errors: errors.messages, extra: extra_analytics_attributes)
+      FormResponse.new(success: valid?, errors: errors, extra: extra_analytics_attributes)
     end
 
     def ssn_is_unique?

diff --git a/app/forms/idv/ssn_format_form.rb b/app/forms/idv/ssn_format_form.rb
@@ -18,7 +18,7 @@ def initialize(user)
     def submit(params)
       consume_params(params)
 
-      FormResponse.new(success: valid?, errors: errors.messages)
+      FormResponse.new(success: valid?, errors: errors)
     end
 
     private

diff --git a/app/forms/new_phone_form.rb b/app/forms/new_phone_form.rb
@@ -23,7 +23,7 @@ def submit(params)
     success = valid?
     self.phone = submitted_phone unless success
 
-    FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+    FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
   end
 
   def delivery_preference_sms?

diff --git a/app/forms/openid_connect_authorize_form.rb b/app/forms/openid_connect_authorize_form.rb
@@ -57,7 +57,7 @@ def initialize(params)
   def submit
     @success = valid?
 
-    FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+    FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
   end
 
   def verified_at_requested?

diff --git a/app/forms/openid_connect_logout_form.rb b/app/forms/openid_connect_logout_form.rb
@@ -32,7 +32,7 @@ def submit
 
     identity.deactivate if success
 
-    FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+    FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
   end
 
   private

diff --git a/app/forms/openid_connect_token_form.rb b/app/forms/openid_connect_token_form.rb
@@ -39,7 +39,7 @@ def submit
 
     clear_authorization_code if success
 
-    FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+    FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
   end
 
   def response

diff --git a/app/forms/otp_delivery_selection_form.rb b/app/forms/otp_delivery_selection_form.rb
@@ -23,7 +23,7 @@ def submit(params)
       change_otp_delivery_preference_to_sms
     end
 
-    FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+    FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
   end
 
   private

diff --git a/app/forms/otp_verification_form.rb b/app/forms/otp_verification_form.rb
@@ -7,7 +7,6 @@ def initialize(user, code)
   def submit
     FormResponse.new(
       success: valid_direct_otp_code?,
-      errors: {},
       extra: extra_analytics_attributes,
     )
   end

diff --git a/app/forms/password_form.rb b/app/forms/password_form.rb
@@ -12,7 +12,7 @@ def submit(params)
 
     self.password = submitted_password
 
-    FormResponse.new(success: valid?, errors: errors.messages, extra: extra_analytics_attributes)
+    FormResponse.new(success: valid?, errors: errors, extra: extra_analytics_attributes)
   end
 
   private

diff --git a/app/forms/password_reset_email_form.rb b/app/forms/password_reset_email_form.rb
@@ -14,7 +14,7 @@ def resend
 
   def submit
     FormResponse.new(
-      success: valid?, errors: errors.messages,
+      success: valid?, errors: errors,
       extra: extra_analytics_attributes
     )
   end

diff --git a/app/forms/personal_key_form.rb b/app/forms/personal_key_form.rb
@@ -16,7 +16,7 @@ def submit
 
     reset_sensitive_fields unless success
 
-    FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+    FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
   end
 
   private

diff --git a/app/forms/register_user_email_form.rb b/app/forms/register_user_email_form.rb
@@ -43,7 +43,7 @@ def submit(params, instructions = nil)
       self.success = process_errors(request_id)
     end
 
-    FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+    FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
   end
 
   def email_taken?

diff --git a/app/forms/reset_password_form.rb b/app/forms/reset_password_form.rb
@@ -18,7 +18,7 @@ def submit(params)
 
     handle_valid_password if success
 
-    FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+    FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
   end
 
   private

diff --git a/app/forms/security_event_form.rb b/app/forms/security_event_form.rb
@@ -48,7 +48,7 @@ def submit
       end
     end
 
-    FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+    FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
   end
 
   def error_code

diff --git a/app/forms/totp_setup_form.rb b/app/forms/totp_setup_form.rb
@@ -19,7 +19,7 @@ def submit
 
     process_valid_submission if success
 
-    FormResponse.new(success: success, errors: {}, extra: extra_analytics_attributes)
+    FormResponse.new(success: success, extra: extra_analytics_attributes)
   end
 
   private

diff --git a/app/forms/totp_verification_form.rb b/app/forms/totp_verification_form.rb
@@ -8,7 +8,6 @@ def submit
     cfg = if_valid_totp_code_return_config
     FormResponse.new(
       success: cfg.present?,
-      errors: {},
       extra: extra_analytics_attributes(cfg&.id),
     )
   end

diff --git a/app/forms/two_factor_authentication/phone_deletion_form.rb b/app/forms/two_factor_authentication/phone_deletion_form.rb
@@ -16,7 +16,7 @@ def initialize(user, configuration)
     def submit
       success = configuration.blank? || valid? && configuration_destroyed
 
-      FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+      FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
     end
 
     private

diff --git a/app/forms/two_factor_login_options_form.rb b/app/forms/two_factor_login_options_form.rb
@@ -16,7 +16,7 @@ def submit(params)
 
     success = valid?
 
-    FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+    FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
   end
 
   private

diff --git a/app/forms/two_factor_options_form.rb b/app/forms/two_factor_options_form.rb
@@ -17,7 +17,7 @@ def submit(params)
     success = valid?
     update_otp_delivery_preference_for_user if success && user_needs_updating?
 
-    FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+    FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
   end
 
   def selected?(type)

diff --git a/app/forms/update_email_language_form.rb b/app/forms/update_email_language_form.rb
@@ -16,7 +16,7 @@ def submit(params)
 
     FormResponse.new(
       success: valid?,
-      errors: errors.messages,
+      errors: errors,
     )
   end
 end
diff --git a/app/forms/update_user_password_form.rb b/app/forms/update_user_password_form.rb
@@ -13,7 +13,7 @@ def submit(params)
     self.password = params[:password]
     success = valid?
     process_valid_submission if success
-    FormResponse.new(success: success, errors: errors.messages)
+    FormResponse.new(success: success, errors: errors)
   end
 
   private

diff --git a/app/forms/verify_account_form.rb b/app/forms/verify_account_form.rb
@@ -21,7 +21,7 @@ def submit
     else
       reset_sensitive_fields
     end
-    FormResponse.new(success: result, errors: errors.messages)
+    FormResponse.new(success: result, errors: errors)
   end
 
   protected

diff --git a/app/forms/webauthn_setup_form.rb b/app/forms/webauthn_setup_form.rb
@@ -28,7 +28,7 @@ def submit(protocol, params)
       PushNotification::HttpPush.deliver(event)
     end
 
-    FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+    FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
   end
 
   # this gives us a hook to override the domain embedded in the attestation test object

diff --git a/app/forms/webauthn_verification_form.rb b/app/forms/webauthn_verification_form.rb
@@ -25,7 +25,7 @@ def submit(protocol, params)
     success = valid? && valid_assertion_response?(protocol)
     FormResponse.new(
       success: success,
-      errors: errors.messages,
+      errors: errors,
       extra: extra_analytics_attributes,
     )
   end

diff --git a/app/forms/webauthn_visit_form.rb b/app/forms/webauthn_visit_form.rb
@@ -3,7 +3,7 @@ class WebauthnVisitForm
 
   def submit(params)
     check_params(params)
-    FormResponse.new(success: errors.empty?, errors: errors.messages)
+    FormResponse.new(success: errors.empty?, errors: errors)
   end
 
   def check_params(params)

diff --git a/app/services/account_reset/cancel.rb b/app/services/account_reset/cancel.rb
@@ -16,7 +16,7 @@ def call
         update_account_reset_request
       end
 
-      FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+      FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
     end
 
     private

diff --git a/app/services/account_reset/create_request.rb b/app/services/account_reset/create_request.rb
@@ -11,7 +11,6 @@ def call
 
       FormResponse.new(
         success: true,
-        errors: {},
         extra: extra_analytics_attributes,
       )
     end
-Original file line number
+Diff line change
@@ Expand Up / @@ -8,7 +8,7 @@ def initialize(user) @@
       end
       def submit
-        FormResponse.new(success: valid?, errors: errors.messages, extra: extra_analytics_attributes)
+        FormResponse.new(success: valid?, errors: errors, extra: extra_analytics_attributes)
       end
       private
@@ Expand Down @@