Unpublished translations should fallback to the original language

Created on 7 June 2018, over 6 years ago
Updated 2 March 2023, almost 2 years ago

Problem/Motivation

Content entities have a feature that allows to control which translation is used when trying to view or do something else with it.

That is an API: \Drupal\Core\Entity\EntityRepositoryInterface::getTranslationFromContext().

Internally, that calls a hook that content_translation implements. the tricky thing is that the hook has a dynamic context operation argument, and we have different contexts in core.

In EntityViewBuilder, we use the default, which is 'entity_view'. But in EntityConverter, which is used when loading the node from the route, we are using 'entity_upcast'.

The problem is that content_translation.module currently only implements the hook for the entity_view operation. What it does is interesting, because it implements a fallback, that goes like this:

You have a site with 3 languages, EN, DE and FR. If you have an node 1 that has 2 translations, EN is the default language and published, DE is not published. If you pass that node through that API and request the published EN translation, that is returned, done. If you ask for DE, then it removes that as an allowed language and it falls back to EN, showing that. If you request FR, which doesn't exist, then it also falls back to EN and shows that.

This is the behavior when viewing node 1 through a view or entity reference field on another node, because then it goes directly to EntityViewBuilder and picks the language there with operation entity_view and the hook runs.

But, if you access that node directly, then something interesting happens. We load the node from the route, and pass it to getTranslationFromContext() with operation entity_upcast. The hook doesn't run. That means if we ask for DE, we do get the DE translation, with status 0. After that, we check access, but because this translation is unpublished, we deny access.

So this happens (as anonymous user, without access to unpublished content):

/en/node/1: EN translation is shown
/fr/node/1: Fallback to EN because FR doesn't exist, EN is shown.
/de/node/1: 403 access denied.

It just doesn't make sense to me (@berdir) that a non-existing translation and an unpublished one behave differently. But I haven't been able to convince @plach that this is a bug that must be fixed, he argued that too few people have reported this and it could be seen as an unwanted change. But if you take the above example and then for example add an unpublished FR translation as well, /fr/node/1 goes from EN to access denied. I really don't see how that would be a desired behavior for anyone.

Another example is a list of content, that shows all your content as a teaser, if possible translated, e.g. on the frontpage. That means with the setup above, on /en, /de, /fr, you always see that node, in EN. But then you click on it on /de and you get an access denied. Again, just doesn't make sense :)

Proposed resolution

Implement the hook also for entity_upcast. In fact, I'm not sure why it shouldn't be done for *any* operation.

Remaining tasks

Decide if we should do this or not.

User interface changes

API changes

Data model changes

Release notes snippet

Problem/Motivation

From #2951294.9 πŸ› Sort out and fix language fallback inconsistencies Needs work :

  1. When we have a default revision having only a published English translation, both /node/1 and /it/node/1 return a 200.
  2. When we have a default revision having a published English translation and an unpublished Italian translation, /node/1 returns a 200 and /it/node/1 returns a 403.

If I understand correctly, what @Berdir is suggesting would make case 2 behave as case 1: CT's language fallback would kick in and the English translation would be displayed also on /it/node/1.

Proposed resolution

The logic from content_translation_language_fallback_candidates_entity_view_alter should apply to entity_upcast context as well.

Remaining tasks

User interface changes

API changes

Data model changes

πŸ“Œ Task
Status

Needs work

Version

10.1 ✨

Component
Content translationΒ  β†’

Last updated 2 days ago

No maintainer
Created by

Live updates comments and jobs are added and updated live.
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.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    From the comments it seems like this isn't ready for review as the approach is still being decided. Let me know if I'm wrong.

  • πŸ‡©πŸ‡ͺGermany hchonov πŸ‡ͺπŸ‡ΊπŸ‡©πŸ‡ͺπŸ‡§πŸ‡¬

    I've just added a related issue - https://www.drupal.org/project/drupal/issues/3353243 πŸ› Unpublished entity reference translations use a fallback while unpublished translations of the main entity do not Needs work and when making a decision both issues have to be considered.

  • πŸ‡§πŸ‡ͺBelgium kristiaanvandeneynde Antwerp, Belgium

    We have a site where we don't want language fallback to happen. We made this work by relying on the behavior discussed here. By simply making sure that we have a translation for all languages, even if empty, we can choose which languages the content is available in by setting the unwanted translations to unpublished. This correctly leads to an access denied in our eyes.

    I'm not denying the IS makes a valid point about inconsistency, but I am also agreeing with @plach that we can't simply change this behavior. It would kill our site's access logic and probably a lot more site's expected behavior too.

  • last update about 1 year ago
    30,684 pass, 2 fail
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update about 1 year ago
    Composer error. Unable to continue.
  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    πŸ› Many calls to ContextRepository::getAvailableContexts() due to entity upcasting Needs review is currently implementing basically πŸ“Œ Deprecate the "entity_upcast" operation specified in EntityConverter Active , although for now more as a proof of concept to see if that works.

    Not sure what to reply to #33. Relying on this behaviour is IMHO unreliable, there are many ways to view an entity, like reference fields, views, various API integrations which will all not use the entity_upcast operation. See also my explanation over there. *Maybe* that doesn't apply to your specific site and you never reference an entity that's still being translated from another entity that already is and have no other entry points to view those entities, but it doesn't seem like something that should be relied on on.

    If that's the behavior that you want then what about instead implementing access logic that denies view access if the entity language doesn't match the current language, so you essentially never allow access to view an entity "in the wrong language" (if the user isn't allowed to edit, to allow preview). Then you don't need to maintain fake-translations.

    In our case, we handle this by displaying a message on top of the content that it's not available in the current language.

    I wish entity_upcast would never have existed, I'm pretty certain this was just a misunderstanding of what that operation is meant to be used for, it's undocumented, but I also acknowledge that people now rely on it now that it exists. I don't know what to do about that. A configurable BC layer for this seems quite a bit of complexity.

  • πŸ‡§πŸ‡ͺBelgium kristiaanvandeneynde Antwerp, Belgium

    Relying on this behaviour is IMHO unreliable ...

    We got to that point where we checked how Drupal works, saw that under certain conditions access would be denied and found that said conditions matched our client's requirements perfectly. At that point we went with it.

    Whether this is advisable for all projects is definitely up for discussion, but Drupal supported it without any mention of it being a bad idea or plans for removal of said functionality in the future. If we were to change this outside of a major version, our project and potentially others would definitely suffer.

    Which is why I still side with #8 here. We are fortunate enough to be part of the 45 followers here, but breaking BC here will definitely upset more people than those following this issue.

    In our case, we handle this by displaying a message on top of the content that it's not available in the current language.

    Our client has specific needs about who can translate what entity in certain languages. By combining the behavior exhibited here with Group, we managed to deliver said functionality perfectly. there's bound to be other people who came up with similar solutions.

    I wish entity_upcast would never have existed

    +1, but I don't have a time machine :)

  • πŸ‡§πŸ‡ͺBelgium kristiaanvandeneynde Antwerp, Belgium

    Ran into another issue where we need to be able to deny access to missing translations (which we're currently using aforementioned workaround of unpublished translations for). Documented this on the unofficial parent issue: #2951294-63: Sort out and fix language fallback inconsistencies β†’

    Should we go ahead and make that one the parent issue of all the other related issues, including this one?

  • Pipeline finished with Failed
    about 1 month ago
    #341453
  • Pipeline finished with Failed
    about 1 month ago
    Total: 536s
    #341478
  • Pipeline finished with Success
    22 days ago
    Total: 663s
    #359460
Production build 0.71.5 2024