Add JSON:API Translation experimental module

Created on 22 February 2021, over 3 years ago
Updated 27 May 2024, 20 days ago

Problem/Motivation

Split from #2794431-121: [META] Formalize translations support . Postponed on #3199696: Add language support to ResourceObject .

Steps to reproduce

Proposed resolution

Remaining tasks

  1. Write tests
  2. Test suggestion: Test what happens when a header is sent with specific language to a canonical url which should return the default language.
  3. Address feedback #8 in https://www.drupal.org/project/drupal/issues/2794431#comment-13935684 🌱 [META] Formalize translations support Needs work

User interface changes

None

API changes

Data model changes

None

Release notes snippet

Feature request
Status

Needs work

Version

11.0 🔥

Component
JSON API 

Last updated 2 days ago

Created by

🇮🇹Italy plach Venezia

Live updates comments and jobs are added and updated live.
  • API addition

    Enhances an existing API or introduces a new subsystem. Depending on the size and impact, possibly backportable to earlier major versions.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇮🇹Italy plach Venezia

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

  • 🇳🇱Netherlands bbrala Netherlands

    <3

  • Pipeline finished with Failed
    8 months ago
    Total: 177s
    #35700
  • last update 8 months ago
    Custom Commands Failed
  • last update 8 months ago
    Custom Commands Failed
  • Pipeline finished with Canceled
    8 months ago
    Total: 8s
    #35907
  • Pipeline finished with Failed
    8 months ago
    #35908
  • last update 8 months ago
    30,274 pass, 3 fail
  • Pipeline finished with Failed
    8 months ago
    Total: 613s
    #35913
  • last update 8 months ago
    Custom Commands Failed
  • Pipeline finished with Failed
    8 months ago
    Total: 162s
    #36180
  • last update 8 months ago
    Custom Commands Failed
  • Pipeline finished with Failed
    8 months ago
    Total: 171s
    #36181
  • last update 8 months ago
    30,435 pass
  • Pipeline finished with Success
    8 months ago
    Total: 711s
    #36183
  • Status changed to Needs work 8 months ago
  • 🇮🇹Italy plach Venezia
  • last update 8 months ago
    Custom Commands Failed
  • 🇮🇹Italy plach Venezia

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

  • last update 8 months ago
    Custom Commands Failed
  • Pipeline finished with Canceled
    8 months ago
    Total: 76s
    #36979
  • last update 8 months ago
    Custom Commands Failed
  • Pipeline finished with Failed
    8 months ago
    Total: 133s
    #36981
  • Pipeline finished with Failed
    8 months ago
    Total: 177s
    #36989
  • last update 8 months ago
    Custom Commands Failed
  • Pipeline finished with Failed
    8 months ago
    #37006
  • last update 8 months ago
    30,437 pass, 1 fail
  • Pipeline finished with Failed
    8 months ago
    Total: 600s
    #37017
  • last update 8 months ago
    30,438 pass
  • Pipeline finished with Success
    8 months ago
    Total: 596s
    #37026
  • Pipeline finished with Failed
    4 months ago
    #100404
  • Pipeline finished with Failed
    4 months ago
    Total: 511s
    #100414
  • Pipeline finished with Success
    4 months ago
    Total: 516s
    #100466
  • Status changed to Needs review 4 months ago
  • 🇮🇹Italy plach Venezia

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

  • 🇮🇹Italy plach Venezia

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

  • 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺

    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 🤓

  • Status changed to Needs work 4 months ago
  • 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.

  • 🇮🇹Italy plach Venezia

    @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 :)

  • 🇮🇹Italy plach Venezia

    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.

  • 🇳🇱Netherlands bbrala Netherlands

    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

  • 🇳🇱Netherlands bbrala Netherlands

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

Production build 0.69.0 2024