Skip to content
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

Return SP user to branded page when canceling #1436

Merged
merged 1 commit into from
May 18, 2017

Conversation

monfresh
Copy link
Contributor

Why: If a user came from a Service Provider, then clicks
"Create Account", and clicks "Cancel", they should be taken back
to the branded page they saw originally, as opposed to the non-branded
home page.

Note that this only fixes the link on one page. Fixes for the links
on subsequent pages in the account creation flow will come in a separate
PR.

@@ -13,4 +13,4 @@ p.mt-tiny.mb0#email-description = t('instructions.registration.email')

= f.button :submit, t('forms.buttons.submit.default'), class: 'btn-wide mb4'

= render 'shared/cancel', link: root_path
= render 'shared/cancel', link: decorated_session.cancel_link_url
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Look, ma: no conditionals! Thanks, @jessieay 😄

**Why**: If a user came from a Service Provider, then clicks
"Create Account", and clicks "Cancel", they should be taken back
to the branded page they saw originally, as opposed to the non-branded
home page.

Note that this only fixes the link on one page. Fixes for the links
on subsequent pages in the account creation flow will come in a separate
PR.
@monfresh monfresh force-pushed the mb-cancel-returns-to-branded-page branch from 056dc30 to 5e6fbc8 Compare May 18, 2017 17:33
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, one quick question about specs, but I think it's fine

click_link t('sign_up.registrations.create_account')
click_link t('links.cancel')

expect(current_url).to eq sign_up_start_url(request_id: sp_request_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

should we do a check on the content of the page for the SP name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could, but that's already tested elsewhere. The SP name is looked up based on the request_id. So, since we're already testing that the correct request_id is being included in the URL, we can safely assume the correct SP name will be displayed.

@monfresh monfresh merged commit 12e4b4d into master May 18, 2017
@monfresh monfresh deleted the mb-cancel-returns-to-branded-page branch May 18, 2017 22:21
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.

2 participants