Join modules together (Extras & Defaults)

This is exciting! 👏🏽

I'll test this tomorrow probably.

Great! Let me know when you're done with your testing. I'll add my review then.

I gave this a testrun and it works great.

I would suppose to add the following:

• On an article node. If i default include a field_media and on the the media i default include the field_image i expect to have field_media.field_image on the includes if i query the node
• this would also help tremendously with other includes like paragraphs and dynamic entity references like media (image or video for example)

This would help to get rid of default_includes like the following:

field_author
field_author.field_image
field_author.field_image.field_image
field_category
field_content
field_content.field_image
field_content.field_image.field_image
field_hero_image
field_hero_image.field_image

because this would be enough:

field_author
field_category
field_content
field_hero_image

Here a little bit more elaboration why i think this would be a great next step and some more possible improvments

• Until now we did not know if the included entity has other includes. Now we do and we should use this knowledge
• I would not use a submodule for this but just add this as a normal feature
• I think we should attach a checkbox to all reference fields on the overwrites page instead of having a default includes text field
• Filters will probably need to live in the textarea as they will be much more complicated to include for each field.

I made several adjustments to bring it up to speed to the latest available APIs. I also took the liberty to change the format slightly (see screenshot). On top of that I created some automated tests to make sure everything works as expected.

Finally, before we commit this we should turn those TODO items into drupal.org issues. In addition to that we'll need to have an issue to use service decoration instead of service swapping in jsonapi_defaults.

Please let me know about this.

For some reason this is creating test patch failures for JSON API Extras. We'll need to figure this out before merging.

@mkolar I am not actively working on this anymore, will you take over bringing this to the finish line?

Would this go into a new 8.x-3.x branch?

@Wim Leers I'm not sure about that. I didn't really think about it. Any reason to require a major version change?

Re-roll

Will be able to push a bit in here in the next few days.

That's fantastic news @ndobromirov!

I have created a new patch that is compliant with JSON API 2.

Since the JSON API doesn't allow other modules to define normalizers I have created a new controller that overrides the one defined in JSON API. From this controller, I have added a method that overrides the one from JSON API which injects the includes.

Applying a new patch and interdiff with the previous one, that I will hide.

There was an issue with the patch that I used as a starting point from #13 - when saving the include configurations they were not split per line.

@e0ipso: do you plan to add this to JSON API Extras 8.x-2.x or 8.x-3.x?

I would consider this 2.x material, as it's an pure API addition. 3.x was added as a means to start clearing backwards compatibility overhead.
On top of that what's the update path for people in contrib that are still using jsonapi_defaults?

jsonapi_defaults works with jsonapi 1.x and jsonapi_extras 2.x (almost). Unless there is jsonapi_defaults in the 2.x extras, there is no way people can reach 3.x safely and incrementally (1 -> 2 -> 3).

On top of that what's the update path for people in contrib that are still using jsonapi_defaults?

jsonapi_defaults works with jsonapi 1.x and jsonapi_extras 2.x (almost). Unless there is jsonapi_defaults in the 2.x extras, there is no way people can reach 3.x safely and incrementally (1 -> 2 -> 3).

Excellent point!

I have created a new patch based on #20.

The difference is that we are changing the way we alter the JSONAPI controller to inject the includes: previously we were relying on route subscriber and now we are overriding the controller defined as a service.

Applying an interdiff with the changes from the previous patch.

Patch does not apply against latest dev :(.
@Rosen, can you try resolving the problematic tests as well?

Applying a new patch created from #20.

Changes:
Added a dependency on jsonapi_defaults submodule to jsonapi v2.
Fixed coding standard issues.

The tests are failing, but it is not related to the applied patch.

When executed against the latest dev version they are failing with same output.

I will try to resolve the tests, but I will appreciate if someone gets involved too.

As a hint, I saw that when the tests are running the \Drupal\jsonapi_extras\ResourceType\ConfigurableResourceTypeRepository::isJsonApi2x returns TRUE no matter that it tests against JSONAPI v1.

Let's try against 8.x-3.x.

@e0ipso see this comment here: https://www.drupal.org/project/jsonapi_extras/issues/3017255#comment-128... , once merged we could retest the latest patch.

Let's see if we can apply against 3.x instead.

@rhristov it seems we are a bit closer on this. Can you take a look at the failing test and do some manual testing of your own?

I had to remove you from the composer.json since we only list there the original authors of the modules. That was part of the agreement in #2985845: Join efforts: Consider merging with JSON API Extras. I hope you understand. Your contribution is really valued! and credit will be granted on commit.

Alright, let's see if now we are good!

Great! Tests are green. Can someone else test this patch manually before committing?

Will add it to our builds some time today and ping back.

Will add it to our builds some time today and ping back.

Awesome! Thanks for that.

Great work everyone!

@mkolar, can you mark JSON API Defauluts as deprecated in the module page and point people here to JSON API Extras?