Add JSON:API Translation experimental module

I'll try to push this forward a bit :)

<3

The commit above adds GET test coverage, the rest is still to do.

Ok, this should be ready for reviews for reals now :)

plach → changed the visibility of the branch 3199697-add-jsonapi-translation to hidden.

THANK YOU SO MUCH, @plach! 🙏🥳

Posted an initial very high-level review, looking at the broad strokes of this impressive (and big! 2000 LoC!) MR. 🤩

Completely reviewing this requires understanding lower level pieces of our JSON:API implementation, knowledge of which has evaporated my brain over the past half decade 😅😶‍🌫️

For now, I'm especially interested in understanding why this is not extending ResourceTestBase but JsonApiFunctionalTestBase. It might be fine/safe, but I'd love to A) understand it, B) have the rationale written down somewhere 🤓

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

@Wim Leers

Thanks for the review and sorry for the belated reply!
(I'd need more birthdays to work on this regularly ;)

For now, I'm especially interested in understanding why this is not extending ResourceTestBase but JsonApiFunctionalTestBase.

I think (it's been a while for me as well), I was just hoping that as an experimental module we could get away with only generic test coverage, as providing test coverage for every entity type seemed overkill at the time, as we did not discuss any entity-type specific logic. OTOH since the scaffolding is already there now, I assume it should not be a huge amount of work to try and leverage ResourceTestBase instead. I'll give it a try, stay tuned :)

I had a look and I'm not sure that providing a ResourceTranslationTestBase class extending ResourceTestBase is the way to go: we would need to duplicate the children classes (e.g. EntityTestTest), reimplementing the abstract methods twice with the same logic.

I'd rather provide a test trait that could be used by both JsonApiTranslationFunctionalTest and any resource-specific test class, e.g. EntityTestTranslationTest extending EntityTestTest. We could then extend TestCoverageTest to check for the translation test classes presence and ensure all resources also provide translation-test coverage.

Only way i can see that happen is if we integrate these tests into resourcetestbase and adjust the jsonapi tests. But testing an experimental module in the main jsonapi module kinda feels weird. :x

Since i worked on this issue earlier, i'll keep it in the back of my mind to do a round of tweaks on.

Rebased and refactored a lot, and did all the feedback.

Unfortunately i ended up in a testfailure in this test:

    // Check that translations cannot be created for non-translatable nodes.
    $node = $this->drupalCreateNode();
    $output = $this->doTestNewTranslationRequest($node, 'it', Response::HTTP_UNPROCESSABLE_ENTITY);
    $this->assertSame('Translation is not enabled for the specified resource.', $output['errors'][0]['detail']);

This should be the result of refactoring the checks on enities to a check on the resourcetype. Might be because it not uses the entity type instead of the entity itself in checkEntityTranslatability (now renamed to checkResourceTypeTranslatability).

Time for today is up unfortunately.

Just a note. I did not handle the changes in the tests as discussed in #24

One of the mentioned issues is 🐛 Browser language detection is not cache aware Needs work . Did some work there to see if that can actually get fixed sometime soon.

All green :)

I see that tests have been added so scratched that task out.

Would you all say the other tasks have been addressed?

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Rebased, added experimental flag, went through threads and resolved what was needed.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Fixed phpstan issues and rebased (me like cache).