Skip to content

Patch to VAIP sessions controller changes#7269

Merged
gangelo merged 5 commits intolg-7446-create-cancel-links-4-of-nfrom
margolis-patch
Nov 2, 2022
Merged

Patch to VAIP sessions controller changes#7269
gangelo merged 5 commits intolg-7446-create-cancel-links-4-of-nfrom
margolis-patch

Conversation

@zachmargolis
Copy link
Contributor

See #7176

This helps address some of my comments about keeping the overall diff smaller

See commits, other things:

  • inlining shared examples that are not shared
  • fixing rubocop issues

@gangelo
Copy link
Contributor

gangelo commented Nov 1, 2022

Hi @zachmargolis, I appreciate what you've done and are trying to do; I mean this sincerely, and with the utmost respect. Simple is good; however, simplistic is not (necessarily) in every case :). I believe what you've managed to do here is treat the proofing process, the interaction between IDV/IP, and their relationship to session, simplistically. Their relationship is not simple. This obviously doesn't mean that the code needs to be complicated. Anyhow, anything short of face-to-face interaction leaves much to be desired by way of communication, so I don't want to give the wrong impression :) I've learned some important things from both Hooper and yourself since being here, so I am appreciative of both of you. Our demo is basically a wash at this point (at least for this code making it in); however, I would like to respond to this PR when I have more time tomorrow if I might :)

Thanks Zach for now, and have a good evening.

Copy link
Contributor

@gangelo gangelo left a comment

Choose a reason for hiding this comment

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

I'm okay with this, I do believe the SessionsController should be cleaned up, it's too messy IMHO.

@gangelo gangelo merged commit 50e53f1 into lg-7446-create-cancel-links-4-of-n Nov 2, 2022
@gangelo gangelo deleted the margolis-patch branch November 2, 2022 11:29
gangelo added a commit that referenced this pull request Nov 4, 2022
…entity Proofing Process (4 of 4) (#7176)

* Add an informative comment

changelog: Upcoming Features, Inherited Proofing, LG-7446 Create "Cancel" Links and Supporting Cancellation Code for Identity Proofing Process (4 of 4)

* Refactors to allow SessionsController to handle IP

Inherited Proofing (IP) session cleanup, analytics logging and
redirect to the IP url when the user decides to "start over".

* Add a route specific to inherited proofing

Not necessary, but for consistency; we're pointing to the
existing SessionsController.

* Reinstate the "start over" cancellation button

* Add cancel link on "Retrieving your info" view

* Add devise controller helper to get spec to pass

* Remove unnecessary includes

* Refactor SessionController specs

In prep for Inherited Proofing test additions.

- Add SessionController specs for Inherited Proofing

* Remove unnecessary return

* Add comments for analytics

So that when the card gets worked on, these areas can be
identified and analytics applied.

* Add feature specs

Test cancel from all Inherited Proofing views, all cancellation scenarios:
"Start over"
"No, go back"
"Exit Login.gov"

Need to revisit cancellations from the :verify_waiting step; see
notes in the spec file.

* Patch to VAIP sessions controller changes (#7269)

* Inline mixins to make the PR diff smaller and easier to understand

* Inline shared examples that are only used once (not shared)

* Remove unused shared example

* Remove line length override and fix errors

* fix typo

* Reorganize SessionsController clean up code

* Remove skipped specs per PR feedback

* Clean up sone unnecessary/redundant mocks/vars

* Remove redundant js enable on tests

* Make regexp to check rails paths more concise

* Remove regex when checking current_path

* Remove proofing-specific tests

* Fix rubocop errors

* Move tests inline

* Tidy up session mock

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.

2 participants