We added support for CRUD operations on content entities via REST without test coverage. Test coverage followed in. Translation support never materialized — this issue is the proof of that:
- this was created in November 2013 (when REST was still in heavy development).
- Then it was only commented on again almost a year later, in Q4 2014! From #3–#25, there was a solid discussion about how this should work. But no patches.
- One comment in all of 2015: #26, just updating this issue summary.
- Then another year later, I commented, in February 2016, in #27, bumping it from
- Now we're another year later, and still no work happened.
As such, we have no official, and no documented way to handle translations of entities. Do we return all translations at once or not? How do you modify existing translations? And so on.
So, that brings us to today, where Drupal 8 has been out for more than a year (since November 2015), Drupal 8.0, 8.1 and 8.2 have been released, and 8.3's beta is being tagged in a few days. Drupal 8.3 will ship without this too. That's 4 minor releases without support for this. Time to change that. 8.4 MUST have this fixed.
How to move forward: just add test coverage?
However, that also means that we can't just do anything. We cannot break backwards compatibility. Unfortunately, the #21/#22 proposal would break BC.
As I wrote in #46:
Just had a discussion about this with @damiankloip & @tedbow. (On January 30, but only got around to finish this now.)
There are two key observations that were made:
- Doing the multipart approach @dixon_ suggested in #22 may very well bethe most correctapproach (the ideal solution), but doing so is unquestionably a BC break.
- Ted showed in #32 that this already works today. In a way that is natural/intuitive/guessable to a Drupal developer: via a langcode path prefix. Changing that now would also effectively be a BC break, since it's extremely likely that multilingual sites are already suing this.
So doing the ideal isolation would result in a double BC break, hence that approach must be postponed to D9.
The pragmatic solution is therefore to just expand the test coverage in
EntityResourceTestBaseto test all HTTP methods against all entity types in multiple languages.
Why does the existing code already kinda work? Thanks to
\Drupal\Core\ParamConverter\EntityConverter::convert(), which calls
In other words, the ideal solution is without question off the table. But is adding tests enough? There are several problems/questions to answer:
- #49 points out that
getTranslationFromContext()supports not just path-based language negotiation, but also domain etc. Do we want to support that?
- #51 points out that the HAL normalization and hence the HAL+JSON format in fact does not return a single language, but all translations of every single field. So the HAL normalization is already kind of doing what #22 wanted to do. If a particular language is requested, does that mean that HAL should return only that translation?
- Add test coverage for
serialization-normalizer-based formats (currently only
json): test that
/de/node/1?_format=jsonreturns the German translation, and also allows modifying that translation
- Add test coverage for
hal-normalizer-based formats (currently only
hal_json): test that
/node/1?_format=hal_jsonreturns all translations, and
/de/node/1?_format=hal_jsonreturns only the German translation
- Language negotiation mechanisms other than the path-based one: TBD
User interface changes
Data model changes
|#80||rest-translations-hal-2135829-80.patch||20.52 KB||Wim Leers|