Automatically set revision user/log information/created time on entity revisions

Fixes #63, the interdiff is pretty scuffed since #57 applied with quite a bit of fuzz. I'm not sure if more work needs to be done to decouple stuff as per #32?

I've already discussed #32 with @plach then and still think that Drupal\Core doesn't have to be free of Drupal\user references, would be nice, but Drupal\user is a required module and the difference between a Drupal\Core component and a required module like user and system is IMHO minor and mostly historical/due limitations what exactly Drupal\Core components can contain.

I can see that other parts of that comment have been addressed, the logic was moved to EditorialContentEntityBase. That said, wouldn't RevisionLogEntityTrait be a better fit for this logic? EditorialContentEntityBase is just a handy aggregation of several traits and currently doesn't have its own logic.

Wonder if this could still get an issue summary update please per #32

Definitely agree that we shouldn't be referencing module code from Drupal\Core and it's a policy that has been documented in https://git.drupalcode.org/project/drupal/blob/HEAD/core/lib/Drupal/Core.... However, I think it would be fair to ask for an exception from the framework managers based on the fact that there are several pre-existing uses of the same interface in the Drupal\Core namespace. We could open a follow-up for implementing a solution for #41. It doesn't look like implementing that becomes any harder if we fix this issue separately first.

Could we move Drupal\user\EntityOwnerInterface to Drupal\Core\Entity\EntityOwnerInterface, and then have it use Drupal\Core\Session\AccountInterface rather than Drupal\user\UserInterface?

Technically it might still work, as AccountInterface has an id() method and UserInterface extends AccountInterface so it wouldn't be far removed from the current implementation.

Doing that would allow us to decouple from the user module.

> Could we move Drupal\user\EntityOwnerInterface to Drupal\Core\Entity\EntityOwnerInterface, and then have it use Drupal\Core\Session\AccountInterface rather than Drupal\user\UserInterface?

An "account" is currently an abstract concept that might or might not be a user entity, in theory it could be something else. Yes, UserInterface extends AccountInterface, but that means that a user is an account, not necessarily the opposite. Of course, most places we assume it is and every User::load($currentUser->id()) would fall apart badly if it weren't, but technically, that is how it is defined. So we'd first need to redefine that an account id does in fact need need to be a user id. At that point, having two different interfaces wouldn't actually make too much sense anymore.

We'd also definitely need to to 📌 Load user entity in Cookie AuthenticationProvider instead of using manual queries Needs work as well.

So that would have pretty big implications, and would set us back at least one major release cycle before we could rely on that.

Of course, most places we assume it is and every User::load($currentUser->id()) would fall apart badly if it weren't, but technically, that is how it is defined. So we'd first need to redefine that an account id does in fact need need to be a user id.

I was thinking about that in the back of my head, but I was also thinking that, but in theory a site could have its own "user" implementation which extends AccountInterface, and in theory not even use the user module. Again, this is just a theory, I've never practically tested that, and it's very likely that it just wouldn't work, but in theory we could support that scenario by purposefully not coupling to the user module.

Re-rolled #64 for 11.x and converted to MR. Hiding patches. Have not addressed any recent comments.

Updated version to 10.2.0.
Thanks!

Pushed an updated 11.x branch to the fork to fix the spellcheck error, see https://www.drupal.org/project/drupal/issues/3401988#comment-15400828 🐛 Spell-checking job fails with "Argument list too long" when too many files are changed Active

acbramley → changed the visibility of the branch 11.x to hidden.

Updated IS, added framework manager tag based on #68. FWIW - I really hope we can use the interface there because the magic getters/setters are not great in the current state.

Trying the patch for Drupal 10 in comment #64 and when creating a new media, the revision user is now correctly set.

But when editing the same media with another user, the revision is still set to the owner user id and not the user that is editing the media.

In media_form_alter, i have to manually set it to finally have the current user in the revision log.

  $media_entity = $form_state->getFormObject()->getEntity();
  $media_entity->setRevisionUserId(\Drupal::currentUser()->id());

NW based on #81 - great find @Tomefa

The weird part is, that code looks to have come from Node (setting the revision author to the author if it's not set) so I wonder why that same issue doesn't already apply to Node.

Will try to dig into it today.

Marking as Needs tests to cover that scenario (editing with a different user)

To answer #82 - it comes from here https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

Manually testing Media works fine for me.

However, it doesn't make sense to set the revision author in 2 places, and it especially doesn't make sense to set it to the author. I think we should remove it from the form and fix the entity base implementation.

Pushed test coverage for #81, still unsure how to reproduce it via the UI since Media goes through that same form base.

The !$this->getRevisionUser() check in ContentEntityBase also fails on API based updates so we may need to remove that and always hardset to the current user.