Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
All credit goes to @effulgentsia for this brilliant idea and proof of concept.
We both pair programmed on this patch.
This will eliminate a lot of complexity in both modules, at the cost of a very dirty trick.
Comment | File | Size | Author |
---|---|---|---|
#14 | 3036904-14.patch | 22.24 KB | Wim Leers |
| |||
#10 | 3036904--poison-namespaces-allow-serializer--10.patch | 22.99 KB | Wim Leers |
#10 | interdiff.txt | 777 bytes | Wim Leers |
#8 | 3036904--poison-namespaces-allow-serializer--8.patch | 22.33 KB | e0ipso |
#3 | 3036904-3.patch | 22.09 KB | gabesullice |
|
Comments
Comment #2
gabesulliceComment #3
gabesulliceFixing a CS violation.
Comment #4
Wim LeersWow, really impressive work here!
Of course, the person to review this, is @e0ipso.
👏 Decoration FTW!
😮 → 🤔 → 🥳
This rename should've already happened before, it just hasn't yet. No huge deal, but if you don't know that this is simply a "forgotten rename", then this is of course very confusing.
Comment #5
effulgentsia CreditAttribution: effulgentsia at Acquia commentedI think we can remove the
[]
from here. It's a remnant from earlier work that I did on this where I was adding to theDrupal\jsonapi
namespace rather than defining a more specific namespace.Comment #6
e0ipsoI'll be looking into this soon.
Comment #7
e0ipsoThis is a neat trick! I don't see any big issues that prevent from merging, then refining if necessary.
However, it makes me doubt the effectiveness of the prevention measures we put in JSON:API. I think this all rely on hopefully people won't notice that you can do it like this.
I will re-roll de patch and then merge.
Comment #8
e0ipsoKicking tests. No interdiff because re-roll.
Comment #10
Wim LeersThere's two failures in #8:
::testWrite()
fails because as of JSON:API 2.4, "read-only mode" is turned on by default: #3039568: Add a read-only mode to JSON:APIjsonapi.link_manager
no longer exists.Attached is the fix for the first failure. For the second failure, we should commit #3035045: Make EntityToJsonApi use subrequests so that it never breaks again. Marking this issue as postponed on that.
Comment #11
e0ipso#3035045: Make EntityToJsonApi use subrequests so that it never breaks again is resolved. Un-postponing and kicking off tests.
Comment #12
Wim LeersI think we want #3035134: Compatibility with upcoming JSON:API 2.4 release: JSON:API Extras 3.5 to land first.
Comment #13
Wim LeersApplied #10 on top of #3042124-11: [regression] Empty response body when user is an administrator and an exception is thrown; some traces cannot be encoded because of recursion detection. — tests are green :) As soon as that lands, this can move forward too. 🥳
Comment #14
Wim Leers#3035134: Compatibility with upcoming JSON:API 2.4 release: JSON:API Extras 3.5 landed. Uploading a rebased #10.
Comment #15
e0ipsoYou beat me to it @Wim Leers!
@Wim Leers++
Comment #17
e0ipsoFixed! Thanks all.