Skip to content

feat: add Accept header for integration tests - #714

Open
civsiv wants to merge 5 commits into
masterfrom
feature/fix-accept-and-content-headers
Open

feat: add Accept header for integration tests#714
civsiv wants to merge 5 commits into
masterfrom
feature/fix-accept-and-content-headers

Conversation

@civsiv

@civsiv civsiv commented Apr 2, 2025

Copy link
Copy Markdown
Contributor

No description provided.


createHeaders() {
return {
'Accept': 'application/vnd.openactive.booking+json; version=1',

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I've understood content negotiation properly, this is needed for all requests to Bookings Systems

createHeaders() {
return {
'Accept': 'application/vnd.openactive.booking+json; version=1',
'Content-Type': 'application/vnd.openactive.booking+json; version=1',

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed to say what the content type of the request is

@lukehesluke lukehesluke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@civsiv it looks like the Broker has two places where it sets the wrong Accept headers also. It sets Accept: 'application/json, application/vnd.openactive.booking+json; version=1',, which has an extraneous application/json.

It is possible there's some legacy reason for this

@lukehesluke

Copy link
Copy Markdown
Contributor

Also, packages/openactive-integration-tests/test/global-setup.js's deleteTestDataset(..) needs Accept header (https://openactive.io/test-interface/#delete-test-interfacedatasetstestdatasetidentifier)

@civsiv
civsiv enabled auto-merge (squash) April 17, 2025 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants