Problem/Motivation
When dealing with revision translations, normally a new revision is created every time a translation is modified. This is the case in particular when Content Moderation is enabled. Every time a new revision is created as default (the only possible case when CM is not enabled), that revision is listed as "current revision" in the revision list for the corresponding language. However, in all other languages the revision list does not feature any "current revision", which is weird because it almost seems like there is no live revision for that translation.
Proposed resolution
For each translation label as "current revision" the "latest" revision that was created as default, and link it to the node view page rather than the revision view page. This works because every default revision always contains a copy of all available translations.
Remaining tasks
Validate the proposed solutionWrite a patch- Reviews
User interface changes
Before:
After:
API changes
None
Data model changes
None
Comment | File | Size | Author |
---|---|---|---|
#17 | node-ml_revision_ui-2938947-15.patch | 3.45 KB | Wim Leers |
#17 | node-ml_revision_ui-2938947-15--test-only-FAIL.patch | 1.51 KB | Wim Leers |
#3 | Italian_revisions_for_Test_1_8_IT___Drupal_8_x.png | 149.58 KB | plach |
#3 | Italian_revisions_for_Test_1_8_IT___Drupal_8_x 2.png | 157.18 KB | plach |
#3 | French_revisions_for_Test_1_9_FR___Drupal_8_x.png | 136.06 KB | plach |
Comments
Comment #2
Gábor HojtsyHm, what does playing well mean? Is that issue introducing a problem or is this an existing problem that we can only fix after that issue?
Comment #3
plachComment #4
plachComment #5
plachUpdated IS
Comment #6
plachComment #7
Wim LeersThanks, the screenshots in the IS make the before/after clear.
On what change is this blocked exactly? (This seems like it's a long-pre-existing bug in HEAD that we should fix anyway. But it's possible that we need some infrastructure fixes before we can fix the UI as shown.)
Comment #8
plachThis was reviewed during today's UX call and got a +1.(misunderstood, sorry)Here's a patch.
Comment #9
Gábor HojtsyIt is not accurate to say this was reviewed in the UX call. I tried to present it but I did not fully grasp the issue to be able to by that time. Nonetheless I think this is an issue that was raised earlier and may already have an issue open. I believe @catch was missing the current revision as well in this table earlier.
Comment #10
Gábor HojtsyI *think* this is a more accurate title.
Comment #11
plachThis should be less implementation-specific.
Comment #12
Wim LeersThe changes here I expected based on the screenshots in the IS. And it includes expanded test coverage, great — that is of course also relevant for making the screenshots in the IS a reality.
But based on the IS, it's unclear why we need this?
Zero mentions of "revert" anywhere in the issue or IS so far. I'd like to see that either
Comment #13
plachReverted, it's an unrelated clean-up
Comment #14
Wim LeersConsidering @plach was working late and per #13 it's now a simple revert, I feel like I can simply do that revert myself and then RTBC that new patch, which would simply be a subset of #8.
Comment #15
Wim LeersThis change is most definitely non-essential.
This change also seems unnecessary. And running the test without this proves it to be unnecessary.
So, reverting those two hunks too.
Comment #16
Wim LeersThat now only leaves these changes:
… and those literally only change the "node revisions overview" UI, and test those changes.
So: easy to RTBC now :)
Comment #17
Wim LeersOh, just one more thing: the same patch as #15, but now also a test-only patch that should fail.
Comment #18
plach#15.2 was indeed related to the "revert form" hunk that has been reverted in #14, so all good here.
Comment #20
plachI created an issue for the reverted changes: #2939107: Follow-up for #2924724: Clean up the node revision translation revert logic to leverage the new API.
Not urgent, but reviews welcome :)
Comment #21
plachComment #24
Gábor HojtsyLooks superb. I believe @catch said that this would be an important improvement as well in some prior issue but could not track him down to verify this yet. Regardless I believe this is an important improvement, one could even say bugfix IMHO.
Comment #25
xjmComment #26
Wim LeersIt definitely is IMO.