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

Navigator FE #3522

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Navigator FE #3522

wants to merge 13 commits into from

Conversation

asmega
Copy link
Contributor

@asmega asmega commented Jan 17, 2025

Context

  • The goal of this PR is to focus on a stateful representation a journey
  • Given a set of answers, a pointer to what page the user is on or wants load:
    • are they allowed on the page, not be able to jump to a future page without answering preceding pages ✅
    • determine what the next page should be when they hit continue ✅
    • what the previous page should be when they hit the back link, therefore remove and backlink overrides ✅
    • if a user changes answer retrospectively mid-way thru a journey clear all answers no longer in the "tree" ✅
    • if a user changes answer retrospectively mid-way thru a journey where a later page causes ineligibility clear the last answer in the tree that caused ineligibility ✅
    • this process needs to be deterministic and repeatable ✅
  • To limit the amount of work the changes have been constrained to a single journey, FE
  • Things to keep in mind but probably out of scope:
    • changing answers on the check answers page affecting state

Notes:

  • Every page must now be a form even though that page is simply content
  • I've attempted to make the slug sequence additive rather than subtractive. which i think is more logical and easier to reason with. but found this to be more work than is probably worth
  • Also resolves https://dfedigital.atlassian.net/browse/CAPT-1912

Copy link
Contributor

@rjlynch rjlynch left a comment

Choose a reason for hiding this comment

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

Very nice!

Seems weird that we care about both the current slug and next slug, feel like we only need to care about one of them, the first incomplete slug (ie current slug) - though I appreciate there's a ton of code using current and next slug!

@asmega asmega force-pushed the navigator-fe branch 4 times, most recently from d89cf9f to 7926db9 Compare January 22, 2025 16:17
Copy link
Contributor

@slorek slorek left a comment

Choose a reason for hiding this comment

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

Great work 👏 I like these changes, and it's something that we have been thinking needs to be improved for some time.

I would like to refactor the next_slug and previous_slug methods to be more readable/digestible, and of course we'll need to do some testing of the journey scenarios to make sure nothing is broken.

app/forms/check_your_answers_form.rb Outdated Show resolved Hide resolved
app/forms/check_your_answers_form.rb Outdated Show resolved Hide resolved
@asmega asmega added the deploy Deploy a review app for this PR label Jan 27, 2025
- this occurs when user backtracks more than one page in the journey
  seqeunce
- then changes answer that does not affect their pathway
- this should reduce the number of SQL commits
@asmega asmega marked this pull request as ready for review January 28, 2025 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy Deploy a review app for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants