Disallow saving the current default revision as a non-default revision

Created a merge request, this should fail.

Fixed a few bugs in the tests, now there's a sensible amount of fails.

don't really understand what Drupal.KernelTests.Core.Entity.RevisionableContentEntityBaseTest is doing, but I don't think what it does is sensible.

\Drupal\KernelTests\Core\Entity\EntityDuplicateTest::testDuplicateNonDefaultRevision() is 🐛 ContentEntityBase::createDuplicate() should reset default revision flag Needs work , forgot about that, that was RTBC but then somehow it got lost once it was set to needs work. Probably a blocker for this.

\Drupal\Tests\content_moderation\Kernel\ContentModerationSyncingTest::testUpdatingPreviousRevisionDuringSync() is tricky, it's resaving an old revision that was default but no longer is. I'll need to double check if we have a way to detect the difference between current default revision and previous default revision. The migrate tests might be similar, not sure yet.

\Drupal\Tests\menu_ui\Functional\MenuUiContentModerationTest::testMenuUiWithPendingRevisions is the fail I expected. I have one idea that we can try, but it will complicate things a bit and it depends on content_moderation specifically. The idea is that we'd create that menu link to the latest route, where access is denied if you don't have access to preview the draft, and change it back. But it complicates things, we always need to look for that route, or at least if it's a non-default-revision node being edited and update the target on publish.

I have one idea that we can try, but it will complicate things a bit and it depends on content_moderation specifically.

If this idea works and it's not super hard to implement, I think it would be worth doing. It would be temporary anyway, until 📌 Allow for / implement simplified content workflow with workspaces Active is finished and Content Moderation will be able to properly track an entity and its dependencies.

Maybe postponed is more appropriate, but with known-failing tests, this is NW. Super minor nit as an MR suggestion.

This stuff really has rabbit holes inside rabbit holes.

The migrate EntityRevision destination plugin calls setDefaultRevision(FALSE) since 2014, i honestly don't know why. It loads an existing revision or the default revision, neither should have a reason to change whether or not it's currently a default revision, it should just update that revision. Lets see if removing that breaks any tests.

For the menu_ui stuff, the changes that only partially worked inside 📌 MenuTreeStorage shouldn't invalidate cache tags if the menu didn't change Active actually passed then here. The reason for that is again the partial and incorrect fix that was done in #2850022: Duplicating a non-default revision should produce a default revision for a newly created entity → , which allowed the partial and incorrect implementation in #3041326: Remove 'title' and 'description' from MenuSettingsConstraintValidator when used with content moderation by creating a draft of menu link content when a draft of it's parent content is created → . What we did is accept the changed value for isDefaultRevision() but then the return value ignored the current value and returned always TRUE if the entity is *currently* new. That means that it correctly saves the change as the default revision but then in post save, in \Drupal\menu_link_content\Entity\MenuLinkContent::postSave(), it now sees that it's actually supposed to be not a default revision, so it doesn't update the menu tree storage. Meaning, the menu link content entity exists as a regular enabled default revision, but it's just not in the menu tree. Saving it through any other means than the node form on a draft would then however resave it properly and it would become visible.

What I did there now is as suggested above, point those menu links to the /latest page, then they show up but only for users who are allowed to see the latest version. It's either that or not allowing to create new menu links on a draft. And I changed isDefaultRevision() to ignore a new value on a new entity, so that it doesn't change in the middle of saving.

I also removed the first check about new and not default revision, because both old and new implementation of isDefautRevision() do not allow for that to happen.

I see lots of new fails though, so I'll need to investigate where that comes from.

This should come back green now and is ready for feedback/reviews. Comes with extensive explanations on the MR about the changes. Could likely do with some adjusted inline comments, open for suggestions.