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

Fix content type for requests that contain a body #33

Closed
wants to merge 1 commit into from

Conversation

lenvanessen
Copy link

@lenvanessen lenvanessen commented Jul 12, 2023

When you use postOfferExport() you'll end up with an error that states

Picqer\BolRetailerV10\Exception\ResponseException
The supplied content-type media type is not supported.

This is due to an issue where the Client uses the wrong Content-Type header. This API should contain the header
application/vnd.retailer.v9+json but the current code ignores the provided produces option and defaults to application/vnd.retailer.v10+json, causing issues with some endpoints.

This change addresses this issue.

@robvanaarle
Copy link
Contributor

robvanaarle commented Jul 12, 2023

Good find. This is introduced in v10, because v10 is only partly specified in the OpenAPI specs, so it was merged with v9.

However, I suspect your solution could break other methods. Client::postProductLabels for example produces a application/vnd.retailer.v9+pdf, so this will also be send as Content-Type header, but we're not sending data in PDF format to Bol.

The real problem lies deeper. The OpenAPI specifies both produces and consumes. The first should be (and is) sent as Accept header, the latter as Content-Type. But currently for Content-Type the constant API_CONTENT_TYPE_JSON is used, which works only when all Bol methods are for the same API version.

@robvanaarle
Copy link
Contributor

@lenvanessen I opened #34 with a fix with the approach I described. Does this indeed solve your issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants