Conversation
mokhld
changed the title
Picks up conditionalAmounts, emailField, and paymentAmountSchema min(0) support needed by the DF-832 engine changes. DF-832
mokhld
force-pushed
the
feat/df-832-pay-v2
branch
2 times, most recently
from
21b2e56 to
a273ccb
Compare
mokhld
changed the title
- Resolve payment amount from conditionalAmounts on PaymentField (first match wins, evaluated in order; falls back to base options.amount) - Prepopulate GOV.UK Pay email from the emailField reference on PaymentField (sent as top-level email to GOV.UK Pay) - Skip payment page in forward navigation; users reach it via CYA "Pay and submit" redirect instead - Zero resolved amount bypasses payment entirely; CYA shows "Accept and submit" and submits without a GOV.UK Pay redirect - Reconcile stored pre-auth when the user edits answers after CYA and the resolved amount changes - GOV.UK Pay callback appends ?paymentComplete=true so the summary handler auto-submits the form on return Renames checkPaymentAmount param definitionAmount -> expectedAmount for clarity now that the expected amount comes from resolveAmount rather than the static definition. DF-832
Based on the production fishing permit form, extended with duration + site access to exercise compound AND conditions across all 8 pricing combinations (12-month/8-day/1-day x Adult/Concession/Junior). Registered at /payment-v2-test via localFormsService. Can be removed before a prod merge if the team prefers. DF-832
Adds unit test coverage for the four Sonar-flagged blocks added in DF-832: - getNextPath skips payment pages during forward navigation (users reach them via the CYA "Pay and submit" button, not by walking the form) - getViewModel overrides the displayed payment amount by calling PaymentField.resolveAmount with the full form evaluationState, covering matched condition, zero-amount condition, and base fallback - getViewModel sets allowSaveAndExit=false on payment pages - getViewModel drives showSubmitButton off paymentState.preAuth.status, covering both the pre-auth present and missing cases Adds a V2 form fixture with a PaymentField + conditionalAmounts + conditions so the tests can exercise the real FormModel / FormContext path rather than mocking internals. DF-832
Adds unit test coverage for the three Sonar-flagged blocks in SummaryPageController: - GET handler auto-submits when the GOV.UK Pay callback appends ?paymentComplete=true, skipping reconcile + the CYA render; and takes the normal render path otherwise - reconcilePaymentState resets the payment component state when the user edits answers after pre-auth and the resolved amount no longer matches the stored amount, and is a no-op when the amounts match or no paymentState is stored yet - submitForm's hasPaymentBeenCaptured loop: re-throws downstream failures as PaymentSubmissionError when a payment has been captured, and re-throws the original error otherwise Reuses the payment-v2-conditional V2 fixture introduced in the QuestionPageController tests. DF-832
mokhld
force-pushed
the
feat/df-832-pay-v2
branch
from
02cc019 to
23ccb51
Compare
| // getViewModel() only has the current page's payload, not full form state. | ||
| // The full state is available via context.evaluationState. | ||
| for (const comp of components) { | ||
| if ('amount' in comp.model && 'paymentState' in comp.model) { |
There was a problem hiding this comment.
Could these properties be missing, in which case it might not display the payment amount?
There was a problem hiding this comment.
That's a great point. I don't think any changes are needed here because we unconditionally spread the amount and the payment state into its own object. The check isn't defending against missing data; it's more of a duck type. The payments date could be undefined, but the key is always there.
| if (isPaymentPage(page.pageDef)) { | ||
| return false | ||
| } | ||
|
|
There was a problem hiding this comment.
A tangential thought here - do we need to move the position of the payment page to AFTER CYA in the designer, not that V2 has the payment after CYA?
There was a problem hiding this comment.
Another great spot. I don't think we need it in this PR because the engine is going to handle the ordering independent of page position in the definition. This is more of a designer UI cosmetic thing that will probably be handled in that PR.
| // Payment page is skipped in the page walk, so proceed() would redirect | ||
| // to an earlier page. For PaymentIncomplete, redirect directly to the | ||
| // payment page using its href. | ||
| if ( |
There was a problem hiding this comment.
What about 'Back' link from CYA? I presume you've covered that this should not take a user back to the Payment page?
There was a problem hiding this comment.
I don't think we need any changes here because the CYA backlink walks the pages but skips the payment page, so the payment will never end up in context or paths. I also manually tested this.
jbarnsley10
left a comment
jbarnsley10
left a comment
There was a problem hiding this comment.
Excellent work. I just want to check the query param 'paymentComplete=true' can't be circumvented by a user adding it? I think you do reconcilePayment after that anyway so it will still check that the correct amount has been paid, if at all.
Approving in principal. Obvs this needs thorough testing
I think you're right to definitely ask, but it's safe because the query param will only skip the CYA render, and it can't skip payment verification or capture. The only path with the form completing without a payment is when |
|
2 participants
Proposed change
Adds condition-based payment amount resolution and GOV.UK Pay email prepopulation to PaymentField. Payment page is skipped in forward navigation and reached via the CYA "Pay and submit" redirect; zero resolved amount bypasses payment entirely and shows "Accept and submit" on CYA.
Jira ticket: DF-832
Type of change
Checklist
README.mdanddocs/*(where appropriate, e.g. new features).npm run test).npm run lint).npm run format).