Skip to content

Add link to continue to SP after recovering with personal key#5541

Merged
mitchellhenke merged 7 commits intomainfrom
mitchellhenke/ial2-personal-key-sp-continue
Oct 27, 2021
Merged

Add link to continue to SP after recovering with personal key#5541
mitchellhenke merged 7 commits intomainfrom
mitchellhenke/ial2-personal-key-sp-continue

Conversation

@mitchellhenke
Copy link
Contributor

@mitchellhenke mitchellhenke commented Oct 25, 2021

If you re-activate your profile with your personal key during an IAL2 authentication, you'll get your new personal key with no way to continue on to the service provider you are trying to reach.

Screenshot:

Old New
image image

@mitchellhenke mitchellhenke force-pushed the mitchellhenke/ial2-personal-key-sp-continue branch from 9a97c88 to 20e42bf Compare October 25, 2021 19:39
@mitchellhenke mitchellhenke force-pushed the mitchellhenke/ial2-personal-key-sp-continue branch from 20e42bf to 55903e2 Compare October 25, 2021 19:50
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically out of scope and fine to defer, but how would you feel about cleaning up this file and eliminating it as an exception in .erb-lint.yml?

- '*/app/views/accounts/_personal_key.html.erb'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm definitely good with doing that!

@mitchellhenke mitchellhenke force-pushed the mitchellhenke/ial2-personal-key-sp-continue branch from e9b5aab to 4765530 Compare October 25, 2021 20:59
@mitchellhenke mitchellhenke marked this pull request as ready for review October 25, 2021 21:42
@mitchellhenke mitchellhenke requested a review from aduth October 26, 2021 14:33
@mitchellhenke
Copy link
Contributor Author

@aduth @zachmargolis got some feedback from UX and decided to move the "Continue" into its own box, which led to some weird changes with alert box margins. The main issue is the personal key alert box will disappear on refresh, and we wanted to preserve the ability to continue even if someone refreshes, changes password, etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice consolidation of this margin class 👏

Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't tested locally yet, but... is there a file missing here for _service_provider_continue.html.erb ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

...yep

@mitchellhenke mitchellhenke force-pushed the mitchellhenke/ial2-personal-key-sp-continue branch from e13cba6 to 5f968d5 Compare October 26, 2021 14:39
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Looks good 👍

Comment on lines 37 to 49
Copy link
Contributor

Choose a reason for hiding this comment

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

Could benefit from some spec coverage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure of a good way to test this. My hope is tests of the inner functions and content is sufficient to test indirectly since it won't show any of them if this function is broken. It did catch that I hadn't captured show_gpo_partial in the initial version 😬

@mitchellhenke mitchellhenke force-pushed the mitchellhenke/ial2-personal-key-sp-continue branch from d8f2a69 to 8723de0 Compare October 26, 2021 14:56
@mitchellhenke mitchellhenke force-pushed the mitchellhenke/ial2-personal-key-sp-continue branch from 8723de0 to e030ef3 Compare October 27, 2021 14:09
@mitchellhenke mitchellhenke merged commit 1c4cadb into main Oct 27, 2021
@mitchellhenke mitchellhenke deleted the mitchellhenke/ial2-personal-key-sp-continue branch October 27, 2021 15:05
mitchellhenke pushed a commit that referenced this pull request Oct 27, 2021
* simple version of continuing to SP after recovering with personal key

* Update app/views/accounts/_personal_key.html.erb

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

* fix specs

* fix erblint

* add spec

* move sp continue into own partial

* add translations

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants