Feature/common error conditions order already exists #183
Conversation
| /** | ||
| * @param {object} args | ||
| * @param {InstanceType<import('../helpers/request-state')['RequestState']>} args.state | ||
| * @param {import('../helpers/flow-helper').FlowHelperType} args.flow | ||
| * @param {import('../helpers/logger').BaseLoggerType} args.logger | ||
| */ |
| // The following requests are not using the helper functions that are used above. This is because | ||
| // those requests are cached (using pMemoize) and therefore would not go back to the Booking System. |
There was a problem hiding this comment.
@civsiv I think that these should use the helper functions because they provide useful stuff like consistent test reporting messages (and structure) as well as performing validation tests (which should always be performed on responses
If you look at core/amending-order-quote/implemented/amend-c1-test.js, there is an approach there that allows it. UUID can be shared between states by including it in new RequestState
| "request": "launch", | ||
| "runtimeArgs": [ | ||
| "run-script", | ||
| "start-tests", | ||
| "--", | ||
| "--runInBand", | ||
| "test/features/core/common-error-conditions/implemented/incomplete-customer-details-test.js" | ||
| "test/features/core/common-error-conditions/implemented/order-already-exists-test.js" |
There was a problem hiding this comment.
We should fix this #184 next time we change this, so we don't need to update this in each PR
| @@ -207,14 +207,33 @@ class RequestState { | |||
| return isResponse(this.c1Response); | |||
| } | |||
|
|
|||
| /** | |||
| * As leasing is optional, getting the totalPaymentDue should not be dependent on C1 or C2 being completed. | |||
There was a problem hiding this comment.
Although Leasing is optional C1 and C2 are not (ref: https://openactive.io/open-booking-api/EditorsDraft/#broker-contract-with-booking-system)
So totalPaymentDue from the response must always be used. See also openactive/open-booking-api#152
There was a problem hiding this comment.
Ah ok, in that case the RefImpl needs to be changed as it returns a PaymentMismatchError when it shouldn't
|
This test was not merged, but is required |
closes #167