Closed (outdated)
Project:
JSON:API
Version:
8.x-2.x-dev
Component:
Code
Priority:
Minor
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
21 Nov 2018 at 14:21 UTC
Updated:
21 Mar 2019 at 13:15 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
wim leersIf this goes into Drupal 8.7 core, there's no need for 8.5 and 8.6 compatibility of course. This addresses most of that.
Comment #3
wim leers#2 removed all mentions of 8.5, 8.6 or 8.7. Except for three. Those in:
\Drupal\jsonapi\ForwardCompatibility\Normalizer\DateTimeIso8601Normalizer\Drupal\jsonapi\ForwardCompatibility\Normalizer\DateTimeNormalizer\Drupal\jsonapi\ForwardCompatibility\Normalizer\TimestampNormalizerBecause we had been assuming since June that #2926508: Add DateTimeNormalizer+TimestampNormalizer, deprecate TimestampItemNormalizer: @DataType-level normalizers are reusable by JSON:API would surely go into Drupal 8.6, and definitely in 8.7. It still hasn't landed 😱 😞 So for now, we need to keep those 3 classes. That means we should start pushing #2926508 forward again, it risks blocking JSON:API.
Comment #4
wim leersComment #5
wim leersComment #6
wim leers#2843147-79: Add JSON:API to core as a stable module includes #4.
Comment #7
wim leersAddresses #2843147-94: Add JSON:API to core as a stable module points 1 and 3.
Comment #8
wim leersFix CS violations.
Comment #9
gabesulliceThis core compatibility issue will land soon: #3021277-14: Test failures on 8.7 since #2869426. When it does, we should remove the interdiff in #14 (linked right there 👆)
Comment #10
wim leersThanks for being so diligent 👌
Comment #11
wim leers#3021277: Test failures on 8.7 since #2869426 landed, RC4 is out, time for an updated core patch, and hence time for addressing #9.
Comment #12
wim leersComment #13
wim leersFor https://www.drupal.org/project/jsonapi/releases/8.x-2.1.
Comment #14
gabesulliceThis removes schema validation and our dependencies on the Schemata module and the justinrainbow/json-schema library. See the failures here: #2843147-103: Add JSON:API to core as a stable module
I'm working on fixing the Postgres issue now.
Comment #15
gabesulliceOpened this for the Postgres issue: #3027626: Add sorts to our collection tests that are inspecting values in the collection, to not be dependent on per-DB default order
Comment #16
gabesulliceSpoke with @Wim Leers, we decided keeping the json-schema library would be beneficial, which means we're just removing schemata. Interdiff is against #13.
Comment #17
gabesulliceI didn't mean to delete this.
Comment #18
gabesulliceSilly mistake.
Comment #19
effulgentsia commentedJsonapiServiceProviderregisters the 'application/octet-stream' mime-type. Since 8.7'sFileServiceProviderdoes that, can this patch be updated to remove it fromJsonapiServiceProvider?Comment #20
wim leers#19: great catch!
Comment #21
wim leers#2955615: Field properties are not being denormalized introduced this:
#2957385: FieldItemNormalizer never calls @DataType-level normalizer service' ::denormalize() method landed yesterday, so we can get rid of that todo! 🎉
Comment #22
wim leersWith #3028970: Audit all @todos and #2965056: Support `include` parameter on relationship routes (turns out it accidentally works on cold caches: fix + test this) having landed, this needed updating. First, rebasing.
Comment #23
wim leersRemoved a few things that those two issues introduced/changed.
And also fixed one thing that should have been removed together with #20.
Comment #24
wim leersD'oh, lol, I queued a 8.6 and 8.5 test for #23. 🤡 Force of habit. I'll queue all possible envs for 8.7 instead :)
Comment #25
effulgentsia commented#19 hasn't been fully addressed yet. Even after applying #23,
JsonapiServiceProvider::alter()registers'application/octet-stream', which duplicates\Drupal\file\FileServiceProvider.And while you're in there: #3029626: Use ::class constant instead of string literal where possible.
Comment #26
wim leersYou're right, I forgot about #19 while working on this other things … my bad!
#3029626: Use ::class constant instead of string literal where possible landed. :)
Comment #27
wim leers#3029704: Follow-up for #3001193: testPostIndividualDxWithoutCriticalBaseFields() fails in some circumstances on PHP 5 landed, which changed the fail in #26 into a pass — that fail was unrelated to this issue/patch :)
Comment #28
wim leersOne more simplification thanks to #3029704: Follow-up for #3001193: testPostIndividualDxWithoutCriticalBaseFields() fails in some circumstances on PHP 5!
Comment #29
effulgentsia commented#28 needs a reroll. Also, #3027980-12: [upstream] Move FormatSetter from middleware to a route filter, removing conditionality on Accept header needs to be added to it.
Comment #30
effulgentsia commentedComment #31
wim leersRebased and merged #3027980-12: [upstream] Move FormatSetter from middleware to a route filter, removing conditionality on Accept header into this patch.
Comment #32
wim leersWith #2926508: Add DateTimeNormalizer+TimestampNormalizer, deprecate TimestampItemNormalizer: @DataType-level normalizers are reusable by JSON:API having landed, we can remove the forward compatibility layer for that!
Comment #33
wim leersForgot #32's interdiff.
Comment #34
effulgentsia commentedPer #2843147-133: Add JSON:API to core as a stable module, let's add a MAINTAINERS.txt entry to this patch.
Comment #35
wim leersSure, but that can't happen here, it'll have to happen in #2843147's script, since
MAINTAINERS.txtlives in core, not thejsonapimodule. Will do!Comment #36
wim leersRebased. Two additional 8.7-specific hunks were added since #32, which this needs to remove. A few hunks are no longer necessary thanks to the recent core security release backporting certain changes from to 8.5/8.6, which hence already removed some core-minor-conditional logic, leaving less work for this patch.
Comment #37
wim leersRerolled to port #2940383-54: [META] Unify file upload logic of REST and JSON:API, which itself was triggered by #2244513: Move the unmanaged file APIs to the file_system service (file.inc) landing.
Comment #38
wim leersThe PHP 7 test failed randomly:
And the PHP 5 tests are also failing for very surprising reasons:
Retesting both.
Comment #39
wim leersPart of #37 landed in HEAD in #3035666: Drupal core compatibility: file_* functions deprecated in Drupal >= 8.7. Rebasing #37.
Also, the PHP 5.5 failures on 8.7 seem to be due to an upstream change in Drupal core. Investigating that in #3035676: Follow-up for #3033123: PHP 5.5 + Drupal 8.7 test runs are suddenly failing.
Comment #40
wim leersRoot cause for the PHP 5 failures found. Fixed in #3035676: Follow-up for #3033123: PHP 5.5 + Drupal 8.7 test runs are suddenly failing. Retesting #39 on PHP 5.
Comment #41
wim leersRebase to solve conflicts after #3033359: Address Drupal core BC policy gate feedback from @xjm landed.
Comment #42
wim leersUpdated to take #2940383-60: [META] Unify file upload logic of REST and JSON:API into account: #2940383 won't happen until after JSON:API lands in core, and hence some the changes in #37 that were designed to make the JSON:API core patch be as ready to use #2940383 after it landed can be removed.
Comment #43
wim leersForgot one line.
Comment #44
wim leersRebased #43 on
ef520336bd13a0f6ce2b8e714cb1b46a98c849dd.Comment #45
wim leersGaaahhhh #3000057: Deprecate drupal_set_time_limit() and file_upload_max_size() and move to Environment component just landed. 😞 Will need to fix those deprecation errors first.
Comment #46
wim leersFixed #45 in #3036772: Drupal core compatibility: file_upload_max_size() deprecated in Drupal >= 8.7.
Updated patch.
Comment #47
wim leersFor #2843147-152: Add JSON:API to core as a stable module. (Sibling patch for #3037682: Address additional Drupal core documentation gate feedback from @xjm.)
Comment #48
wim leersRebased. And one new 8.5-specific thing that needs to be removed for the core patch, due to newly added test coverage.
Comment #49
amateescu commentedThis should fix all the recent failures related to taxonomy terms and custom menu links being converted to revisionable.
I'm not sure what's the process for generating a patch here, so I'm attaching just an interdiff that applies on top the patch from #2843147-163: Add JSON:API to core as a stable module.
Comment #50
wim leers#49: THANK YOU! 🙏Ported that to the contrib module, resolved conflicts, and made it pass on 8.5 and 8.6 too, not just Drupal 8.7. Credited you, of course. See #3015325-8: [ignore] support issue for the core patch and #3015325-10: [ignore] support issue for the core patch.
Comment #51
wim leersThis needs a reroll for #3039235: 8.7.x: update anything needed now that taxonomy terms are revisionable and will need another reroll after #3039568: Add a read-only mode to JSON:API.
Comment #52
wim leersRebased.
Comment #53
wim leersMost of the necessary new changes already were fixed in the conflict resolutions of #52. This is the only new core-minor-conditional code that was added since
b175c3a9fd5cebcb7849de7559f56a73f756d8bc.Comment #54
wim leers#2843147: Add JSON:API to core as a stable module got committed to 8.8, and will be cherry-picked to 8.7 later today 🥳