Cancellation journey page 3 functionality#2309
Conversation
https://eaflood.atlassian.net/browse/IWTF-4775 When the user confirms that they would like to cancel, the recurring payment should be cancelled in the CRM using the existing cancellation functionality.
https://eaflood.atlassian.net/browse/IWTF-4775 When the user confirms that they would like to cancel, the recurring payment should be cancelled in the CRM using the existing cancellation functionality. This is PR 2 of 2 to enable that. This second PR updates the cancellation journey so that the Sales API is called to cancel an agreement when the user confirms they want to do so. It depends on #2276
…-request' into feature/iwtf-4775-cancel-on-user-request-gk
…-request-part-2' into feature/iwtf-4775-cancel-on-user-request-gk
…cancel-on-user-request-gk
|
There was a problem hiding this comment.
Pull request overview
Updates the sales API recurring payment cancellation behaviour to support the cancellation journey (page 3), by changing how cancelledDate is set based on the cancellation reason.
Changes:
- Update
cancelRecurringPaymentto skip settingcancelledDatewhen reason is"User Cancelled". - Update unit tests to reflect the new conditional
cancelledDatebehaviour.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/sales-api-service/src/services/recurring-payments.service.js | Changes cancellation logic to conditionally set cancelledDate based on reason. |
| packages/sales-api-service/src/services/tests/recurring-payments.service.spec.js | Updates/extends tests to validate the new conditional cancelledDate behaviour. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const isUserCancelled = reason === 'User Cancelled' | ||
|
|
||
| if (!isUserCancelled) { | ||
| data.cancelledDate = new Date().toISOString().split('T')[0] | ||
| } | ||
|
|
There was a problem hiding this comment.
cancelRecurringPayment no longer sets cancelledDate when the reason is User Cancelled. Because the Dynamics query for “due recurring payments” filters on defra_cancelleddate eq null, leaving cancelledDate as null/undefined means this record can still be treated as active and picked up for future payment runs even though it has a cancellation reason set. To ensure cancellations actually take effect, set cancelledDate for all cancellation reasons (or update the downstream selection logic to also exclude records based on cancelledReason/status).
| const isUserCancelled = reason === 'User Cancelled' | |
| if (!isUserCancelled) { | |
| data.cancelledDate = new Date().toISOString().split('T')[0] | |
| } | |
| data.cancelledDate = new Date().toISOString().split('T')[0] |
| it('should not set cancelledDate when reason is User Cancelled', async () => { | ||
| retrieveGlobalOptionSets.mockReturnValueOnce({ | ||
| cached: jest.fn().mockResolvedValue({ | ||
| defra_cancelledreasons: { | ||
| options: { | ||
| 910400003: { | ||
| id: 910400003, | ||
| label: 'User Cancelled', | ||
| description: 'User Cancelled' | ||
| } | ||
| } | ||
| } | ||
| }) | ||
| }) | ||
|
|
||
| const cancelledReason = { description: 'User Cancelled', id: 910400003, label: 'User Cancelled' } | ||
|
|
||
| const recurringPayment = { ...getMockRecurringPayment(), cancelledDate: null } | ||
| findById.mockReturnValueOnce(recurringPayment) | ||
|
|
||
| getGlobalOptionSetValue.mockReturnValueOnce(cancelledReason) | ||
|
|
||
| await cancelRecurringPayment('id', 'User Cancelled') | ||
|
|
||
| expect(persist).toHaveBeenCalledWith([ | ||
| expect.objectContaining({ | ||
| ...recurringPayment, | ||
| cancelledReason, | ||
| cancelledDate: null | ||
| }) | ||
| ]) |
There was a problem hiding this comment.
This test asserts that cancelledDate remains null for the User Cancelled reason. Given the current Dynamics selection logic for due recurring payments relies on cancelledDate being set (it filters for cancelleddate eq null to find active records), this expectation would allow user-cancelled records to continue being processed. Once the service logic is corrected, update this test to assert the intended cancellation behaviour (e.g. cancelledDate is set, or the record is otherwise excluded from due processing).
jaucourt
left a comment
jaucourt
left a comment
There was a problem hiding this comment.
Please let Iris check this too
3 participants
https://eaflood.atlassian.net/browse/IWTF-4775