Needs work
Project:
Drupal core
Version:
main
Component:
entity system
Priority:
Major
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
5 Dec 2016 at 10:29 UTC
Updated:
30 Jun 2026 at 05:25 UTC
Jump to comment: Most recent, Most recent file
Show commands
changes
Comments
Comment #2
tstoecklerWe discussed this at the Entity Field API triage meeting with core and Entity API maintainers and agreed on the current classification as "major".
Not sure about the solution yet, but this seems closely related to #2833084: $entity->original doesn't adequately address intentions when saving a revision
Comment #5
plachHere's a fix for this, test coverage is still missing.
Comment #6
plachPerformance optimization
Comment #8
dani3lr0se commentedI'm interested in helping out with this issue. Would someone be able to provide some steps to test this? What specifically should I be looking for when reviewing this? I also realize that the issue is tagged as needs tests, but am I reviewing the tests or the actual content with revisions, etc.? Thanks for your work and also for your time and help. I greatly appreciate it. :)
Comment #11
johnchqueAdding some tests for this. Somehow the untranslatable field is not displayed on translation even with the hidden option disabled.
Comment #12
johnchqueMy bad, that field should not be displayed anyway. Tests are passing locally, let's see if that's the case in testbot.
Comment #14
berdirOk, this is tricky.
\Drupal\Core\Entity\ContentEntityStorageBase::createRevision() actually specifically uses the untranslatable field values from the default revision, that's why this isn't failing, the translation still has the values of the default revision and only the translatable fields initially match the latest draft of EN. That's pretty confusing, you don't see them on edit, but you see those old values on the translation form then. Of course the expectation could also vary based on whether or not the user saves it as a draft or as published.
So it's actually specifically a paragraphs problem because we are currently using the latest revision in our entity_reference_revisions_entity_revision_create() hook and that matches user expectations IMHO. The user expectations that we have seen so far is that users create a draft in default-lang and one in a translation, then they expect to see the paragraphs from that draft and even further, if they make further changes to the default-lang draft, they expect to see the new paragraphs in the translation draft as well.
Of course it gets weird once you publish the translation *before* the default language, then that would basically already publish the untranslatable fields. But the current behavior is just as weird in the other direction (that you have a mix of latest-revision and default-revision values). I am uploading a new test patch that better shows the current behavior, but that's not relevant anymore for this issue.
Long story short, this issue is with the current behavior of createRevision() *not* relevant for validation (when only using core), but I'm considering to open an issue to change that behavior and then it will become relevant.
Comment #15
hchonovBut one can create a forward revision also without using "createRevision" and then we have a relevant problem in hasTranslationChanges :).
Comment #16
maximpodorov commentedBTW, if ContentEntityBase::hasTranslationChanges() is called several times (e.g. for different languages) and (see its code) $this->original is not set, $original is loaded from the DB every time. I suggest to cache $original in the entity. This can be related to #2961399: Support parallel translation forward revisions on untranslatable fields
Comment #17
berdirYes, performance is definitely a concern, but there is a separate issue for that: #2838602: [PP-2] Optimize CEB::hasTranslationChanges by caching its result and serving subsequent calls from the result cache - big performance boost
Comment #18
hchonovThe referenced issue still has the same problem as mentioned by @maximpodorov, as even with the cache of the method's outcome the original unchanged entity will be loaded for each translation being checked. This problem is significant only if
hasTranslationChangesis called outside the saving process, as there we set$this->originalearly in the saving process. However we have two constraint validators, which have the mentioned problem -\Drupal\content_translation\Plugin\Validation\Constraint\ContentTranslationSynchronizedFieldsConstraintValidator::validate()and\Drupal\Core\Entity\Plugin\Validation\Constraint\EntityUntranslatableFieldsConstraintValidator::validate(). I've opened an issue for this - #3007031: hasTranslationChanges on multiple languages outside of saving process is costly.Comment #19
berdirI created #3007233: Draft translations should be based on the latest revision of the source language, not the published version for my proposal above.
@hchonov: Yes, you could create a new revision without createRevision() but I think that isn't really supported/supposed to work :)
Comment #20
plachYep, besides making me cry ;) doing that would also skip
hook_entity_revision_create()and friends, which is similar to manually instantiating an entity instead of using the storage's::create()method: you can do it but it may cause troubles.Comment #22
maximpodorov commentedRe-roll of the patch from #12.
Comment #32
namisha jadhav commentedRe-roll of the patch 22
Comment #33
berdir#2839195: Add a method to access the original property at least partially addresses this, I'm not certain if the current implementation there is up to date, we can still keep this issue to add the test coverage and verify that it does.
Comment #37
danielvezaConverted the patch to an MR, and fixed the test from the patch while I was at it. Letting the pipeline run.