-
Notifications
You must be signed in to change notification settings - Fork 5
Feat/tie purchase flow #534
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
Feat/tie purchase flow #534
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is excellent 🚀 I really like the purchase-flow-state mechanism.
webroot/src/models/AcceptedAttestationToSend/AcceptedAttestationToSend.model.spec.ts
Outdated
Show resolved
Hide resolved
webroot/src/components/PrivilegePurchaseAttestation/PrivilegePurchaseAttestation.ts
Show resolved
Hide resolved
webroot/src/components/PrivilegePurchaseAttestation/PrivilegePurchaseAttestation.vue
Outdated
Show resolved
Hide resolved
@jlkravitz This is ready for your review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Few questions/comments.
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts
Outdated
Show resolved
Hide resolved
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts
Outdated
Show resolved
Hide resolved
webroot/src/components/PrivilegePurchaseSelect/PrivilegePurchaseSelect.ts
Outdated
Show resolved
Hide resolved
webroot/src/models/PurchaseFlowState/PurchaseFlowState.model.spec.ts
Outdated
Show resolved
Hide resolved
webroot/src/models/PurchaseFlowStep/PurchaseFlowStep.model.spec.ts
Outdated
Show resolved
Hide resolved
@jlkravitz Ready for re-review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! @isabeleliassen Good to merge.
Requirements List
Description List
PrivilegePurchase*
PurchaseFlowState
,PurchaseFlowStep
and step level logic to better support step handling (find next step, clean state appropriately, redirect appropriately etc)AcceptedAttestationToSend
modelProgressBar
ComponentTesting List
yarn test:unit:all
should run without errors or warningsyarn serve
should run without errors or warningsyarn build
should run without errors or warningsLicenseeDashboard
screenCloses #302
Note: The behavior above is what I decided to implement to avoid complexity surrounding auto filling potentially changed form fields due to the server attestations being updated on the server, complexity surrounding form race conditions and caching user responses vs updated attestations.
I also opted not to use localStorage to have this data persist across sessions. What I did do was set up the form flow so that any of these alterations can be made more easily if it turns out they are desired.