-
Notifications
You must be signed in to change notification settings - Fork 6
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
[EPIC] Withdrawals v3 #4832
base: main
Are you sure you want to change the base?
[EPIC] Withdrawals v3 #4832
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4832 +/- ##
==========================================
- Coverage 96.23% 96.22% -0.01%
==========================================
Files 766 770 +4
Lines 16396 16488 +92
==========================================
+ Hits 15778 15866 +88
- Misses 618 622 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
6bc2f75
to
191dde6
Compare
432b06c
to
c0a16bf
Compare
9114eca
to
d4ee0b3
Compare
d4ee0b3
to
000cce2
Compare
* add withdrawl trigger page * update error message * remove comments * update rubocop * disable safe navigation chain length cop * add specs for trigger form
…tion and panel (#4917) * modify confirm details component and remove extra information pages * add trigger to comfirm details page * undo analytics blocklist changes * update withdrawal view component spec * change for store keys * update form specs
* correctly order withdrawal reasons and clear reasons upon trigger change * add guard if trigger has not been set * readd skipped specs * remove TODO comment
@defong I've reverted back to using |
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.
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.
A couple of minor suggestions but mainly wanted to question the commented out lines in this db migration.
db/migrate/20241120115559_associate_withdrawal_with_trainee_withdrawal_reason.rb
Outdated
Show resolved
Hide resolved
Co-authored-by: Tom Trentham <[email protected]>
|
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.
Looking good.
Context
https://trello.com/c/PQLud3iQ/7827-encapsulate-withdrawal
https://trello.com/c/NpieMmSn/7846-withdrawal-data-mapping
https://trello.com/c/yd2yhmVx/7879-create-start-interstitial-page-for-withdrawals
https://trello.com/c/PV6qaldG/7907-create-why-did-you-have-to-withdraw-the-trainee-page
https://trello.com/c/zizu1xVe/7914-create-why-did-the-trainee-choose-to-withdraw-page
https://trello.com/c/oi84svc1/7885-create-why-are-you-withdrawing-the-trainee-page
https://trello.com/c/uhWRag0n/7917-create-would-this-trainee-be-interested-in-becoming-a-teacher-in-the-future-page?filter=member:chrisgannon23,member:dave_georgiou
https://trello.com/c/WmfFV8MH/7931-amend-trainee-profile-page-withdrawn-notification-and-panel
https://trello.com/c/pNcirSIp/7951-undo-withdrawal
https://trello.com/c/o8ZjhThF/7927-amend-check-withdrawals-details-page
https://trello.com/c/2Lt3BxuE/7966-withdrawals-cleanup-ticket
Changes proposed in this pull request
Refactor of the withdrawls process, including the data collected and the reasons for withdrawal
Prototype link for the journey.
Guidance to review
Important business
NB: Please notify the #twd_data_insights team and ask for a review if new fields are being added to analytics.yml