Translation cannot be accepted as there is a pending revision in the default translation

This does look like a regression of that issue, strange. A failing test would be helpful, surpried that none of our tests are catching this?

Strange, not sure if I am doing this correctly. These are the steps to reproduce the problem I think so probably something related to the ui?

Moving the test to use Content Moderation. This fails locally as the DE translation is not published.

Checking the revision ids. :)

Hm, this is tricky. The change is what I expected, and is also consistent with what we do in \Drupal\moderation_sidebar\Controller\ModerationSidebarController::sideBar.

This is what happens:

We have "Moderated node", an EN node, with a draft version "[DRAFT] Moderated node". Now, we request a translation of that draft to DE, that picks the EN draft, we translate that, and accept it. So far, everything is fine.

Now, immediately after accepting, we save the DE translation as the new default, published translation. But we still have the actual EN draft that was not yet published, because this revision has been created based on the published default EN revision "Moderated node".

Now, we generate the message, load the original node that we translated from, but now that detects that the EN draft is older than the DE translation/default revision, and no longer uses that. So we have a different node title for the confirmation message.

That alone is not a huge deal, but if you'd now like to translate that same EN draft also to ES, then it won't do that. It will pick the non-draft EN version.

What we'd need is to see if there is a real pending revision for EN, able to make a difference between a past revision that has been superseded by a new default revision, vs. one that is still not merged yet.

That said, I think this is a less severe issue than the current problem, as you'd probably either accept the EN translation first or create two translation requests initially, so we might want to live with this regression, disable/change some tests and create a follow-up issue if there's no easy solution for this.

So there might be something that could work here. There is $entity->wasDefaultRevision().

So instead of not loading the revision, try changing it to load the revision into $revision, then check wasDefaultRevision(). If yes, then it means that it is a history revision (or possibly even the current default revision), then we can ignore it.

Also, lets extend the if ($revision_id) check to also do && $revision_id != $entity->getRevisionId(), because if it the same, then we don't need to do all that extra work of loading that revision because we know it's the same anyway.

Alright, this seems to fix the tests. Nice, thanks for the explanation. :) Let's see if test bot is happy.

Looks good.

Lets add a comment to that piece of code to try to write down what we learned here. Try to summarize my comments down into 2-3 short sentences: that are looking for a pending revision, no need to check a revision if the revision id is the current default revision, and that if was the default revision at some point, then it is an old revision that is part of the default revision that we have.

Adding some comments here. Hope they are comprehensive enough.

And committed.