Skip to content

Complete session in verify review controller#1623

Merged
jmhooper merged 1 commit intomasterfrom
jmhooper-review-complete-session
Aug 29, 2017
Merged

Complete session in verify review controller#1623
jmhooper merged 1 commit intomasterfrom
jmhooper-review-complete-session

Conversation

@jmhooper
Copy link
Contributor

Why: We were creating a profile in the verify/review#create method.
That method redirects to verify/confirmations#show which then calls
#complete_session. This means that the profile exists in quasi
complete state between these requests. This commit makes a small change
so the profile is created and completed in one step. Additionally, this
commit cleans up the confirmations controller specs.


def profile
@_profile ||= Profile.find(profile_id)
@_profile ||= Profile.find_by(id: profile_id)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes a bug that was found when re-organizing the confirmation controller specs. It looks like we expect this to return nil if there is no profile. That implies that profile_id is nil. Profile.find(nil) raises an error which the user sees as a 500.

expect(pii.first_name.norm).to eq 'JOSE'
end

context 'user picked phone confrimation' do
Copy link
Contributor

Choose a reason for hiding this comment

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

minor spelling error: s/confrimation/confirmation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

**Why**: We were creating a profile in the verify/review#create method.
That method redirects to verify/confirmations#show which then calls
`#complete_session`. This means that the profile exists in quasi
complete state between these requests. This commit makes a small change
so the profile is created and completed in one step. Additionally, this
commit cleans up the confirmations controller specs.
@jmhooper jmhooper force-pushed the jmhooper-review-complete-session branch from 2f995fb to b2633ce Compare August 29, 2017 13:28
@jmhooper jmhooper merged commit f4ab0b1 into master Aug 29, 2017
@jmhooper jmhooper deleted the jmhooper-review-complete-session branch December 12, 2017 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants