Inconsistent moderation state for translated content

I don't believe the current patch works correctly when there is *not* an affected translation. In that event, the UI skips revisions that have no translation.

See similar issue in 🐛 Revisions are not visible for some nodes Needs review

I think what we want to do is ensure the proper *translated* revision is loaded, not that it exists or not.

This patch also adds the VID to the output.

BEFORE REVISED PATCH:

AFTER PATCH:

The last submitted patch, 9: diff-3269933-get-revision-with-langcode-9.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

The failing tests here are because the current tests assume that only current language revisions are loaded. However, in my quick tests, that only seems to be a condition if translations actually exist.

What I don't know -- and why I didn't update the tests -- is this:

* If a site is set for multilngual but no translations of a node exist -- what should appear on the translated node revisions tab?

To be clear, I am dealing with a site that defaults to EN but allows ES translations. Without the patch in #9, some revisions are missing for nodes that are not yet translated.

I think the attached approach -- checking against the $current_langcode -- is correct, but can't be sure.

The last submitted patch, 11: diff-3269933-get-revision-with-langcode-11.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

This issue is in fact closely related to 🐛 Revisions are not visible for some nodes Needs review , as stated in #9. I created a MR for that issue and added the change from #4 to it, so both issues are fixed with one change.

This issue appears to be related to 🐛 Revisions controller getRevisionIds() should filter by language Needs review . See patch #5 🐛 Revisions controller getRevisionIds() should filter by language Needs review that correctly checks whether the entity type is translatable and it updates the automated tests.

#11 I think listing all revisions, instead of just the affected ones, is making it hard to tell how many changes there actually were to the translation being viewed. That could be a lot of revisions even with just two languages. Our use case is similar, with mostly Swedish nodes with some English translations here and there, but there can still be a large number of revisions.
I'm also not quite sure this actually fixes the issue, see below.

+++ b/src/Form/RevisionOverviewForm.php
@@ -209,15 +210,23 @@ class RevisionOverviewForm extends FormBase {
+        // If dealing with a translation, load that version.
+        if (($revision->hasTranslation($langcode) && $revision->isRevisionTranslationAffected()) ||
+          $langcode === $current_langcode || !$revision->hasTranslation($current_langcode)
+        ) {

This says it'll load a translation if one exists, but never actually does.

This condition also looks like it will always be true because $langcode is the entity's current language, which is by default the same as $current_language.

I took a slightly different approach and added the query filtering @recrit mentioned, which also meant a few conditions could be removed, and then added a check to figure out which older revision the current revision was based on, if it does not affect the viewed translation.
(I kept the hasTranslation check to be safe, since switching to one that does not exist throws an exception.)

The interdiff was generated without white-space changes to make the indentation changes easier on the eyes.

The last submitted patch, 17: diff-3269933-17.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

I tried to be clever in the check for what would correspond to the default revision content and it already bit me, so modifying it and the message to better reflect reality.

Tweaked the wording a bit, also made the interdiff against 11 to be more clear about all the changes since, again without white-space changes and test fixes.

silvi.addweb → made their first commit to this issue’s fork.

Reroll patch #20 for diff 8.x-1.7

acbramley → closed merge request !61

This issue needs an MR (not the 8.x-1.x or 2.x branch as a source please) against the 2.x branch with tests.

Rerolled the #24 patch as a issue fork branch for 8.x-1.x, with some minor improvements (show the revision ID as a translatable text, as well as an utility revision-was-default class to dictate if the highlighted revision was a previous revision, rather than the default current revision.)