Skip to content

LG-14127 Stop deleting TMx session ID when undoing SSN step #11091

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.

Sign up for GitHub

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

Merged
jmhooper merged 3 commits into from
Aug 21, 2024
Merged

LG-14127 Stop deleting TMx session ID when undoing SSN step #11091

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eaffc82
LG-14127 Stop deleting TMx session ID when undoing SSN step
jmhooper Aug 15, 2024
aad6079
introduce regression spec
jmhooper Aug 20, 2024
27ed92d
quick fix
jmhooper Aug 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions app/controllers/idv/in_person/ssn_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,7 @@ def self.step_info
controller: self,
next_steps: [:ipp_verify_info],
preconditions: ->(idv_session:, user:) { idv_session.ipp_document_capture_complete? },
undo_step: ->(idv_session:, user:) do
idv_session.ssn = nil
idv_session.threatmetrix_session_id = nil
end,
undo_step: ->(idv_session:, user:) { idv_session.ssn = nil },
)
end

Expand Down
5 changes: 1 addition & 4 deletions app/controllers/idv/ssn_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,7 @@ def self.step_info
controller: self,
next_steps: [:verify_info],
preconditions: ->(idv_session:, user:) { idv_session.remote_document_capture_complete? },
undo_step: ->(idv_session:, user:) do
idv_session.ssn = nil
idv_session.threatmetrix_session_id = nil
end,
undo_step: ->(idv_session:, user:) { idv_session.ssn = nil },
)
end

Expand Down
16 changes: 16 additions & 0 deletions spec/controllers/idv/link_sent_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,22 @@
expect(subject.idv_session.applicant).to be_nil
end

# This is a regression spec that was introduced for a bug that occured
# when calling `undo_step` on the SSN controller step info caused the
# TMx session ID to be deleted when this step was resubmitted
#
# See https://cm-jira.usa.gov/browse/LG-14127
# See https://github.com/18F/identity-idp/pull/11091#discussion_r1718831233
it 'does not delete the TMx session ID' do
subject.idv_session.ssn = '900-12-1234'
subject.idv_session.threatmetrix_session_id = 'super-cool-test-value'

put :update

expect(subject.idv_session.ssn).to be_nil
expect(subject.idv_session.threatmetrix_session_id).to_not be_nil
end

it 'sends analytics_submitted event' do
put :update

Expand Down
5 changes: 0 additions & 5 deletions spec/policies/idv/flow_policy_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
idv_session.had_barcode_attention_error = true

idv_session.ssn = Idp::Constants::MOCK_IDV_APPLICANT_WITH_SSN[:ssn]
idv_session.threatmetrix_session_id = SecureRandom.uuid

idv_session.address_edited = true
end
Expand All @@ -68,7 +67,6 @@
expect(idv_session.had_barcode_attention_error).to be_nil

expect(idv_session.ssn).to be_nil
expect(idv_session.threatmetrix_session_id).to be_nil
Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we assert that it is not nil?

Copy link
Copy Markdown
Contributor Author

@jmhooper jmhooper Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can, though with this change the threatmetrix_sesison_id is no longer something the FlowPolicy cares about. Setting it and adding an assertion that it is not nil may be confusing with the context of this change.

Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok would it make sense in the context of preserving the value to add a spec elsewhere in the PR?

Copy link
Copy Markdown
Contributor Author

@jmhooper jmhooper Aug 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a test in aad6079


expect(idv_session.address_edited).to be_nil
end
Expand All @@ -90,7 +88,6 @@
idv_session.had_barcode_attention_error = true

idv_session.ssn = nil
idv_session.threatmetrix_session_id = SecureRandom.uuid

idv_session.address_edited = true

Expand Down Expand Up @@ -141,8 +138,6 @@

idv_session.had_barcode_read_failure
idv_session.had_barcode_attention_error

idv_session.threatmetrix_session_id
}
end
end
Expand Down