-
Notifications
You must be signed in to change notification settings - Fork 166
changelog: Upcoming Features, Attempts API, Add idv-ssn-submitted event #12189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,8 +5,6 @@ class SsnFormatForm | |
| include ActiveModel::Model | ||
| include FormSsnFormatValidator | ||
|
|
||
| ATTRIBUTES = [:ssn].freeze | ||
|
|
||
| attr_accessor :ssn | ||
|
|
||
| def self.model_name | ||
|
|
@@ -18,12 +16,12 @@ def initialize(incoming_ssn) | |
| @updating_ssn = ssn.present? | ||
| end | ||
|
|
||
| def submit(params) | ||
| consume_params(params) | ||
| def submit(ssn:) | ||
| @ssn = ssn | ||
|
|
||
| FormResponse.new( | ||
| success: valid?, | ||
| errors: errors, | ||
| errors:, | ||
| extra: { | ||
| pii_like_keypaths: [ | ||
| [:same_address_as_id], | ||
|
|
@@ -37,18 +35,5 @@ def submit(params) | |
| def updating_ssn? | ||
| @updating_ssn | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def consume_params(params) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it seems like this was a construct developed for forms that are "consuming" a number of different params; while i might argue about whether or not it's that useful generally, it certainly seems like overkill for a form that is only using one param. |
||
| params.each do |key, value| | ||
| raise_invalid_ssn_parameter_error(key) unless ATTRIBUTES.include?(key.to_sym) | ||
| send(:"#{key}=", value) | ||
| end | ||
| end | ||
|
|
||
| def raise_invalid_ssn_parameter_error(key) | ||
| raise ArgumentError, "#{key} is an invalid ssn attribute" | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,13 +38,6 @@ | |
| expect(result.errors).to include(:ssn) | ||
| end | ||
| end | ||
|
|
||
| context 'when the form has invalid attributes' do | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. form now cannot accept invalid attributes, so this spec is unnecessary |
||
| it 'raises an error' do | ||
| expect { subject.submit(ssn: '111111111', foo: 1) } | ||
| .to raise_error(ArgumentError, 'foo is an invalid ssn attribute') | ||
| end | ||
| end | ||
| end | ||
|
|
||
| describe '#updating_ssn?' do | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is
:same_address_as_idnecessary here, or is this an artifact of the code doing something else at some point?