Closed (duplicate)
Project:
JSON:API
Version:
8.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
3 May 2018 at 22:34 UTC
Updated:
11 Jun 2018 at 12:21 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
gabesulliceHi @mstef, are you sending the request header:
Accept: application/vnd.api+json?Comment #3
mstef commentedYes, I'm passing in both Accept and Content-type with that value.
Comment #4
gabesulliceThanks for the info :)
Comment #5
wim leersWhat is the
Acceptheader you're sending?Because I can reproduce getting a HTML response when requesting
/jsonapi/node/article?filter[foobar]=baz… but only if I don't specifyAccept: application/vnd.api+json.Comment #6
mstef commentedI'm sending
Accept: application/vnd.api+json
Content-type: application/vnd.api+json
I am also getting HTML for 404s.
Comment #7
wim leersI suspect something is off in your environment/tests then. Because this test passes for me.
Comment #8
mstef commentedRunning docksal locally with latest D8. Let me know if there's something else I can test/report for you.
Comment #9
wim leers#7 also passes when
jsonapi_extras8.x-1.x or 8.x-2.x is installed.Comment #10
wim leers#8: when you apply #7, do tests pass or fail for you?
Comment #11
mstef commentedI will test as soon as possible (tomorrow, most likely).
I was able to test this issue on a Platform.sh server and I am getting the same response that I originally reported. After I test the patch, I will PM you a live endpoint you can test against.
Comment #12
e0ipsoThanks for checking that @Wim Leers! ❤️
Comment #13
wim leers#11: 👌
#12: ❤️
Comment #14
mstef commentedThis is the output after running the test in the patched file:
Comment #15
wim leersThe two failing tests are
NodeTest::testPatchPath()andNodeTest::testPatchIndividual(). That means the test I added,NodeTest::test2969398()passed.AFAICT that means you also cannot reproduce the problem you originally reported? Can you still reproduce it manually? If so, what's different about the test I wrote in #7?
(Those 2 failures are currently happening when using JSON API on Drupal 8.6 — see https://www.drupal.org/pift-ci-job/976585. I'll get that fixed soon.)
Comment #16
mstef commentededit: disregard this comment. i'll report back shortly.
it is actually working on a fresh install using a node endpoint. let me try to find the root of the problem.
Comment #17
mstef commentedComment #18
e0ipso@mstef if you are making these requests using the browser you need to make sure to have a browser extensions to pass in headers like
Accept: application/vnd.api+json. Can you try that and report back? Alternatively you can use a REST client like Postman for an easier setup.Comment #19
mstef commented@e0ipso, I know; I am and have been. I just did that without thinking for a minute..
It is working fine on a fresh 8.5.3 install. So, as you expected, it must be stemming from something on my site, but I have no idea what just yet. I will try to find out and report back. It happens there for node and all other entity endpoint as well still.
Comment #20
mstef commentedI can confirm this is stemming from JSON API Extras. If I disable that module on the affected environment, I get a proper JSON response. Once I re-enable JSON API Extras, hitting an overridden endpoint with a non-existent field as a filter or sort results in an HTML response.
Comment #21
mstef commentedHere is a before and after
Comment #22
mstef commentedSeems to happen only if the "path prefix" is changed.
Given that, maybe it's still related to JSON API, and not the "extras" module.
Comment #23
wim leers@mstef++ for that digging! That helped a lot :) In fact, it made it easy to pin this down!
The root cause is #2971745: Don't hardcode `/jsonapi/` in FormatSetter, read JSON API base path from container parameter instead, which @e0ipso added to
jsonapispecifically forjsonapi_extras(see #2949632-9: Make ResourceTypeRepository aware of the path prefix).Once #2949632: Make ResourceTypeRepository aware of the path prefix lands, #2971745 can land too, and then this will be fixed. But it requires JSON API Extras to be updated to be compatible with #2949632. That's the only reason #2949632 hasn't yet been committed.
Comment #24
e0ipso👍🏽!
Comment #25
wim leers#2949632: Make ResourceTypeRepository aware of the path prefix landed!
Comment #26
wim leersActually, this is just a pure duplicate of #2971745: Don't hardcode `/jsonapi/` in FormatSetter, read JSON API base path from container parameter instead AFAICT. It's great to have @e0ipso's +1 though :) So, closing this.