Skip to content
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: B/P detailed errors #489

Merged
merged 9 commits into from
Jun 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/openactive-broker-microservice/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/openactive-broker-microservice/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"license": "MIT",
"dependencies": {
"@openactive/data-model-validator": "^2.0.52",
"@openactive/data-models": "^2.0.262",
"@openactive/data-models": "^2.0.271",
"@openactive/openactive-openid-test-client": "file:../openactive-openid-test-client",
"@openactive/rpde-validator": "^2.0.9",
"@openactive/test-interface-criteria": "file:../test-interface-criteria",
Expand Down
6 changes: 3 additions & 3 deletions packages/openactive-integration-tests/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/openactive-integration-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"eslintConfig": {},
"dependencies": {
"@openactive/data-model-validator": "^2.0.52",
"@openactive/data-models": "^2.0.262",
"@openactive/data-models": "^2.0.271",
"@openactive/openactive-openid-test-client": "file:../openactive-openid-test-client",
"@openactive/test-interface-criteria": "file:../test-interface-criteria",
"async-mutex": "^0.3.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ This feature is **required** by the Open Booking API specification, and so must
| [incomplete-customer-details](./implemented/incomplete-customer-details-test.js) | Expect an IncompleteCustomerDetailsError when customer details are missing the required email property | Run each of C2 and B for a valid opportunity, with customer details missing the required email property, expecting an IncompleteCustomerDetailsError to be returned (C1 is ignored because customer details are not accepted for C1) | [TestOpportunityBookable](https://openactive.io/test-interface#TestOpportunityBookable) x8 |
| [incomplete-order-item-no-offer](./implemented/incomplete-order-item-no-offer-test.js) | Test for IncompleteOrderItemError with missing `acceptedOffer` | Test for IncompleteOrderItemError (at C1, C2 and B). If there is a missing `acceptedOffer` property on the OrderItem. | [TestOpportunityBookable](https://openactive.io/test-interface#TestOpportunityBookable) x4 |
| [incomplete-order-item-no-opportunity](./implemented/incomplete-order-item-no-opportunity-test.js) | Test for IncompleteOrderItemError with missing `orderedItem` | Test for IncompleteOrderItemError (at C1, C2 and B). If there is a missing `orderedItem` property on the OrderItem. | [TestOpportunityBookable](https://openactive.io/test-interface#TestOpportunityBookable) x4 |
| [opportunity-in-past](./implemented/opportunity-in-past-test.js) | Expect an OpportunityOfferPairNotBookableError when opportunity is in the past | Runs C1, C2 and B for an opportunity in the past, expecting an OpportunityOfferPairNotBookableError to be returned at C1 and C2, and an UnableToProcessOrderItemError to be returned at B | [TestOpportunityBookableInPast](https://openactive.io/test-interface#TestOpportunityBookableInPast) x3, [TestOpportunityBookable](https://openactive.io/test-interface#TestOpportunityBookable) x1 |
| [opportunity-in-past](./implemented/opportunity-in-past-test.js) | Expect an OpportunityOfferPairNotBookableError when opportunity is in the past | Runs C1, C2 and B for an opportunity in the past, expecting an OpportunityOfferPairNotBookableError to be returned at C1, C2, and B | [TestOpportunityBookableInPast](https://openactive.io/test-interface#TestOpportunityBookableInPast) x3, [TestOpportunityBookable](https://openactive.io/test-interface#TestOpportunityBookable) x1 |
| [unknown-endpoint](./implemented/unknown-endpoint-test.js) | Expect an UnknownOrIncorrectEndpointError for requests to unknown endpoints | Send a request to an endpoint that does not exist, and expect an UnknownOrIncorrectEndpointError to be returned | |


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const {
FlowStageUtils,
FlowStageRecipes,
} = require('../../../../helpers/flow-stages');
const { itShouldIncludeErrorForOnlyPrimaryOrderItems, itShouldReturnAnOpenBookingError } = require('../../../../shared-behaviours/errors');
const { itShouldIncludeErrorForOnlyPrimaryOrderItems } = require('../../../../shared-behaviours/errors');

/**
* @typedef {import('../../../../helpers/flow-stages/c1').C1FlowStageType} C1FlowStageType
Expand Down Expand Up @@ -48,7 +48,7 @@ FeatureHelper.describeFeature(module, {
FlowStageUtils.describeRunAndRunChecks({ doCheckIsValid: false, doCheckSuccess: false }, c2, () => {
itShouldIncludeIncompleteOrderItemErrorWhereRelevant(c2);
});
FlowStageUtils.describeRunAndCheckIsValid(bookRecipe.firstStage, () => {
itShouldReturnAnOpenBookingError('UnableToProcessOrderItemError', 409, () => bookRecipe.firstStage.getOutput().httpResponse);
FlowStageUtils.describeRunAndRunChecks({ doCheckIsValid: false, doCheckSuccess: false }, bookRecipe.firstStage, () => {
itShouldIncludeIncompleteOrderItemErrorWhereRelevant(bookRecipe.firstStage);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const {
FlowStageUtils,
FlowStageRecipes,
} = require('../../../../helpers/flow-stages');
const { itShouldIncludeErrorForOnlyPrimaryOrderItems, itShouldReturnAnOpenBookingError } = require('../../../../shared-behaviours/errors');
const { itShouldIncludeErrorForOnlyPrimaryOrderItems } = require('../../../../shared-behaviours/errors');

/**
* @typedef {import('../../../../helpers/flow-stages/c1').C1FlowStageType} C1FlowStageType
Expand Down Expand Up @@ -48,7 +48,7 @@ FeatureHelper.describeFeature(module, {
FlowStageUtils.describeRunAndRunChecks({ doCheckIsValid: false, doCheckSuccess: false }, c2, () => {
itShouldIncludeIncompleteOrderItemErrorWhereRelevant(c2);
});
FlowStageUtils.describeRunAndCheckIsValid(bookRecipe.firstStage, () => {
itShouldReturnAnOpenBookingError('UnableToProcessOrderItemError', 409, () => bookRecipe.firstStage.getOutput().httpResponse);
FlowStageUtils.describeRunAndRunChecks({ doCheckIsValid: false, doCheckSuccess: false }, bookRecipe.firstStage, () => {
itShouldIncludeIncompleteOrderItemErrorWhereRelevant(bookRecipe.firstStage);
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const { FeatureHelper } = require('../../../../helpers/feature-helper');
const { FlowStageRecipes, FlowStageUtils } = require('../../../../helpers/flow-stages');
const { itShouldIncludeErrorForOnlyPrimaryOrderItems, itShouldReturnAnOpenBookingError } = require('../../../../shared-behaviours/errors');
const { itShouldIncludeErrorForOnlyPrimaryOrderItems } = require('../../../../shared-behaviours/errors');

/**
* @typedef {import('../../../../helpers/flow-stages/c1').C1FlowStageType} C1FlowStageType
Expand All @@ -13,7 +13,7 @@ FeatureHelper.describeFeature(module, {
testFeatureImplemented: true,
testIdentifier: 'opportunity-in-past',
testName: 'Expect an OpportunityOfferPairNotBookableError when opportunity is in the past',
testDescription: 'Runs C1, C2 and B for an opportunity in the past, expecting an OpportunityOfferPairNotBookableError to be returned at C1 and C2, and an UnableToProcessOrderItemError to be returned at B',
testDescription: 'Runs C1, C2 and B for an opportunity in the past, expecting an OpportunityOfferPairNotBookableError to be returned at C1, C2, and B',
// The primary opportunity criteria to use for the primary OrderItem under test
testOpportunityCriteria: 'TestOpportunityBookableInPast',
// The secondary opportunity criteria to use for multiple OrderItem tests
Expand Down Expand Up @@ -43,6 +43,6 @@ FeatureHelper.describeFeature(module, {
itShouldIncludeOpportunityOfferPairNotBookableErrorWhereRelevant(c2);
});
FlowStageUtils.describeRunAndCheckIsValid(bookRecipe.firstStage, () => {
itShouldReturnAnOpenBookingError('UnableToProcessOrderItemError', 409, () => bookRecipe.firstStage.getOutput().httpResponse);
itShouldIncludeOpportunityOfferPairNotBookableErrorWhereRelevant(bookRecipe.firstStage);
});
});
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
const chai = require('chai');
const { FeatureHelper } = require('../../../../helpers/feature-helper');
const { FlowStageRecipes, FlowStageUtils } = require('../../../../helpers/flow-stages');
const { itShouldReturnAnOpenBookingError } = require('../../../../shared-behaviours/errors');

FeatureHelper.describeFeature(module, {
testCategory: 'details-capture',
Expand All @@ -21,10 +20,10 @@ FeatureHelper.describeFeature(module, {
FlowStageUtils.describeRunAndCheckIsSuccessfulAndValid(fetchOpportunities);
FlowStageUtils.describeRunAndCheckIsSuccessfulAndValid(c1);

FlowStageUtils.describeRunAndCheckIsValid(c2, () => {
function itShouldReturnAnIncompleteIntakeFormError(flowStage) {
it('should return an IncompleteIntakeFormError on the OrderItem', () => {
const positionsOfOrderItemsThatNeedIntakeForms = Object.keys(c1.getOutput().positionOrderIntakeFormMap).map(parseInt);
const orderItemsThatNeedIntakeForms = c2.getOutput().httpResponse.body.orderedItem
const orderItemsThatNeedIntakeForms = flowStage.getOutput().httpResponse.body.orderedItem
.filter(orderItem => positionsOfOrderItemsThatNeedIntakeForms.includes(orderItem.position));

for (const orderItem of orderItemsThatNeedIntakeForms) {
Expand All @@ -34,8 +33,13 @@ FeatureHelper.describeFeature(module, {
chai.expect(incompleteIntakeFormErrors).to.have.lengthOf.above(0);
}
});
}

FlowStageUtils.describeRunAndCheckIsValid(c2, () => {
itShouldReturnAnIncompleteIntakeFormError(c2);
});

FlowStageUtils.describeRunAndCheckIsValid(bookRecipe.firstStage, () => {
itShouldReturnAnOpenBookingError('UnableToProcessOrderItemError', 409, () => bookRecipe.firstStage.getOutput().httpResponse);
itShouldReturnAnIncompleteIntakeFormError(bookRecipe.firstStage);
});
});
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
const chai = require('chai');
const { FeatureHelper } = require('../../../../helpers/feature-helper');
const { FlowStageRecipes, FlowStageUtils } = require('../../../../helpers/flow-stages');
const { itShouldReturnAnOpenBookingError } = require('../../../../shared-behaviours/errors');

FeatureHelper.describeFeature(module, {
testCategory: 'details-capture',
Expand All @@ -22,10 +21,10 @@ FeatureHelper.describeFeature(module, {
FlowStageUtils.describeRunAndCheckIsSuccessfulAndValid(fetchOpportunities);
FlowStageUtils.describeRunAndCheckIsSuccessfulAndValid(c1);

FlowStageUtils.describeRunAndCheckIsValid(c2, () => {
it('should return an IncompleteAttendeeDetailsError on the OrderItem', () => {
function itShouldReturnAnInvalidIntakeFormError(flowStage) {
it('should return an InvalidIntakeFormError on the OrderItem', () => {
const positionsOfOrderItemsThatNeedIntakeForms = Object.keys(c1.getOutput().positionOrderIntakeFormMap).map(parseInt);
const orderItemsThatNeedIntakeForms = c2.getOutput().httpResponse.body.orderedItem
const orderItemsThatNeedIntakeForms = flowStage.getOutput().httpResponse.body.orderedItem
.filter(orderItem => positionsOfOrderItemsThatNeedIntakeForms.includes(orderItem.position));

for (const orderItem of orderItemsThatNeedIntakeForms) {
Expand All @@ -35,8 +34,12 @@ FeatureHelper.describeFeature(module, {
chai.expect(incompleteIntakeFormErrors).to.have.lengthOf.above(0);
}
});
}

FlowStageUtils.describeRunAndCheckIsValid(c2, () => {
itShouldReturnAnInvalidIntakeFormError(c2);
});
FlowStageUtils.describeRunAndCheckIsValid(bookRecipe.firstStage, () => {
itShouldReturnAnOpenBookingError('UnableToProcessOrderItemError', 409, () => bookRecipe.firstStage.getOutput().httpResponse);
itShouldReturnAnInvalidIntakeFormError(bookRecipe.firstStage);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ const _ = require('lodash');
const chai = require('chai');
const { FeatureHelper } = require('../../../../helpers/feature-helper');
const { FlowStageRecipes, FlowStageUtils } = require('../../../../helpers/flow-stages');
const { itShouldReturnAnOpenBookingError } = require('../../../../shared-behaviours/errors');

FeatureHelper.describeFeature(module, {
testCategory: 'details-capture',
Expand All @@ -22,12 +21,12 @@ FeatureHelper.describeFeature(module, {
FlowStageUtils.describeRunAndCheckIsSuccessfulAndValid(fetchOpportunities);
FlowStageUtils.describeRunAndCheckIsSuccessfulAndValid(c1);

FlowStageUtils.describeRunAndCheckIsValid(c2, () => {
function itShouldReturnAnIncompleteAttendeeDetailsError(flowStage) {
it('should return an IncompleteAttendeeDetailsError on the OrderItem', () => {
const positionsOfOrderItemsThatNeedAttendeeDetails = c1.getOutput().httpResponse.body.orderedItem
.filter(orderItem => !_.isNil(orderItem.attendeeDetailsRequired))
.map(orderItem => orderItem.position);
const orderItemsThatNeedAttendeeDetails = c2.getOutput().httpResponse.body.orderedItem
const orderItemsThatNeedAttendeeDetails = flowStage.getOutput().httpResponse.body.orderedItem
.filter(orderItem => positionsOfOrderItemsThatNeedAttendeeDetails.includes(orderItem.position));

for (const orderItem of orderItemsThatNeedAttendeeDetails) {
Expand All @@ -37,8 +36,13 @@ FeatureHelper.describeFeature(module, {
chai.expect(incompleteIntakeFormErrors).to.have.lengthOf.above(0);
}
});
}

FlowStageUtils.describeRunAndCheckIsValid(c2, () => {
itShouldReturnAnIncompleteAttendeeDetailsError(c2);
});

FlowStageUtils.describeRunAndCheckIsValid(bookRecipe.firstStage, () => {
itShouldReturnAnOpenBookingError('UnableToProcessOrderItemError', 409, () => bookRecipe.firstStage.getOutput().httpResponse);
itShouldReturnAnIncompleteAttendeeDetailsError(bookRecipe.firstStage);
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const { FeatureHelper } = require('../../../../helpers/feature-helper');
const { FlowStageRecipes, FlowStageUtils } = require('../../../../helpers/flow-stages');
const { itShouldReturnAnOpenBookingError, itShouldIncludeErrorForOnlyPrimaryOrderItems } = require('../../../../shared-behaviours/errors');
const { itShouldIncludeErrorForOnlyPrimaryOrderItems } = require('../../../../shared-behaviours/errors');

/**
* @typedef {import('../../../../helpers/flow-stages/c1').C1FlowStageType} C1FlowStageType
Expand Down Expand Up @@ -70,6 +70,6 @@ FeatureHelper.describeFeature(module, {
itShouldIncludeOpportunityIsInConflictErrorWhereRelevant(c2);
});
FlowStageUtils.describeRunAndCheckIsValid(bookRecipe.firstStage, () => {
itShouldReturnAnOpenBookingError('UnableToProcessOrderItemError', 409, () => bookRecipe.firstStage.getOutput().httpResponse);
itShouldIncludeOpportunityIsInConflictErrorWhereRelevant(bookRecipe.firstStage);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,14 @@ function shouldBeValidResponse(getter, name, logger, options, opportunityCriteri
const statusCode = response.response && response.response.statusCode;
const statusMessage = response.response && response.response.statusMessage;

// Note C1Response and C2Response are permitted to return 409 errors of type `OrderQuote`, instead of `OpenBookingError`
if ((statusCode < 200 || statusCode >= 300) && !(statusCode === 409 && (options.validationMode === 'C1Response' || options.validationMode === 'C2Response'))) {
optionsWithRemoteJson.validationMode = 'OpenBookingError';
// Note C1Response, C2Response, BResponse and PResponse are permitted to return 409 errors of type `OrderQuote`, `OrderProposal`, or `Order` instead of `OpenBookingError`
if (statusCode < 200 || statusCode >= 300) {
// TODO: Test suite should assert whether the response is expected to be an OrderItemError, instead of basing validation on the response status code
if (statusCode === 409 && (options.validationMode === 'C1Response' || options.validationMode === 'C2Response' || options.validationMode === 'BResponse' || options.validationMode === 'PResponse')) {
optionsWithRemoteJson.validationMode = `${options.validationMode}OrderItemError`;
} else {
optionsWithRemoteJson.validationMode = 'OpenBookingError';
}

// little nicer error message for completely failed responses.
if (!body) {
Expand Down