Ongoing patch to maintain compatibility with JSON:API so that JSON:API Extras will have a release ready as soon as the next JSON:API release is made.

CommentFileSizeAuthor
#11 3035134-11.patch5.46 KBwim leers
#11 interdiff.txt539 byteswim leers
#8 interdiff-3-7.txt3.27 KBwim leers
#7 3035134--compatibility-2-4.patch5.29 KBe0ipso
#2 jsonapi-extras-3032679-23.patch997 bytesgabesullice
#3 jsonapi_extras-3033473-11.patch2.7 KBgabesullice
#3 patch-for-3035149-resource-object-data.txt1.38 KBgabesullice
#3 3035134-3-commit.patch3.83 KBgabesullice
#3 interdiff-composer.txt668 bytesgabesullice
#3 3035134-3-do-not-commit.patch11.04 KBgabesullice

Comments

gabesullice created an issue. See original summary.

gabesullice’s picture

gabesullice
Primary language English
commented
StatusFileSize
new jsonapi-extras-3032679-23.patch997 bytes

#3032679: Clean-up: rename EntityCollection to Data requires either the attached patch or #3035045: Make EntityToJsonApi use subrequests so that it never breaks again to be committed.

gabesullice’s picture

gabesullice
Primary language English
commented
Related issues: +#3035045: Make EntityToJsonApi use subrequests so that it never breaks again
StatusFileSize
new 3035134-3-do-not-commit.patch11.04 KB
new interdiff-composer.txt668 bytes
new 3035134-3-commit.patch3.83 KB
new patch-for-3035149-resource-object-data.txt1.38 KB
new jsonapi_extras-3033473-11.patch2.7 KB

Given @e0ipso's response in #3035045: Make EntityToJsonApi use subrequests so that it never breaks again, I'm going to assume that's the preferred approach. Therefore, this patch will not incorporate the patch attached to #2. Therefore, any patch in this issue must be applied on top of #3035045.

First, up, we need to account for #3033473: Clean-up: Remove LinkManager, luckily, we already have a patch for that. Attached.

Next, we need to account for #3035149: Use ResourceObjectData with cardinality 1 for individual responses. Easy, that's a 2-line, test only change.

Finally, I'm adding a composer change to the do-not-commit patch so that it gets tested with the latest JSON:API commit.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#3039600: Update JSON:API Extras' route for compatibility with JSON:API 2.4

#3039600: Update JSON:API Extras' route for compatibility with JSON:API 2.4 will also need to happen for this.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Per #3, #3035045: Make EntityToJsonApi use subrequests so that it never breaks again should land first, and hence I've rerolled #3035045 to address @e0ipso's feedback, see #3035045-8: Make EntityToJsonApi use subrequests so that it never breaks again, hopefully that'll get committed soon :)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review

#3035045: Make EntityToJsonApi use subrequests so that it never breaks again landed :) Re-testing #3, but I suspect it'll need a refill reroll (hahah autocorrect :D).

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
StatusFileSize
new 3035134--compatibility-2-4.patch5.29 KB

This should be down to a single fail. It's a 403 I'm not sure where it's coming from. Printing the response does not yield an explanation.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
StatusFileSize
new interdiff-3-7.txt3.27 KB

Here's an interdiff showing the changing between #3 and #7. (#3's changes in EntityToJsonApiTest are dropped because #3035045: Make EntityToJsonApi use subrequests so that it never breaks again made them irrelevant.)

+1 to #7 👍

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#3042124: [regression] Empty response body when user is an administrator and an exception is thrown; some traces cannot be encoded because of recursion detection.

Reproduced the 403 that #7 mentioned.

It's the same problem we're seeing in #3042124: [regression] Empty response body when user is an administrator and an exception is thrown; some traces cannot be encoded because of recursion detection..

Status: Needs review » Needs work

The last submitted patch, 7: 3035134--compatibility-2-4.patch, failed testing. View results

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review
Related issues: +#3038308: Avoid translations DELETE data loss and unintended changes with PATCH and test all methods against entity route parameter translation upcasting, +#3027890: When trying to use `Authorization: Basic …` header and basic_auth module is not installed, warn user
StatusFileSize
new interdiff.txt539 bytes
new 3035134-11.patch5.46 KB

After more hours of debugging than I'd like, I discovered the root cause: #3038308: Avoid translations DELETE data loss and unintended changes with PATCH and test all methods against entity route parameter translation upcasting removed the basic_auth module as one of the installed-by-default modules. JSON:API Extras was using Basic Auth for authentication. So this is just another occurrence of #3027890: When trying to use `Authorization: Basic …` header and basic_auth module is not installed, warn user.

  • e0ipso committed fe9179f on 8.x-3.x authored by Wim Leers 
    Issue #3035134 by gabesullice, Wim Leers, e0ipso: Compatibility with...
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Needs review » Fixed

w00t! Made some cosmetic changes to avoid PHPCS issues on commit.

Thanks!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Great! Next: #3036904-14: Decorate JSON:API's normalizers and fake its namespace so JSON:API can eliminate its Serializer's fallback behavior. :)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Oh, and #3039600: Update JSON:API Extras' route for compatibility with JSON:API 2.4 of course!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.