Revision author needs to be set

Created on 21 August 2022, about 3 years ago
Updated 31 January 2024, almost 2 years ago

Problem/Motivation

In #2926744: Allow revisionable entities to be saved as new revisions β†’ a feature was added to configure Feeds to create a new revision on each import. While a revision user can be set by mapping to "Revision user", it would be nice if you can set that to the user that triggered the import.

Proposed resolution

There are multiple ways to solve this problem:

  1. Write a FeedsSource plugin that returns the current logged in user. This could then be mapped to "Revision user". The pro of this is that this source can then also be used for other things.
  2. Automatically set the revision user to the current user, unless there is being mapped to "Revision user".
  3. Provide a setting on the feed type to set the revision author. There is a setting for "owner" as well. I think I'm less in favor for this solution, because the more settings there are, the more intimidating the UI can become.

Challenge: get the current logged in user

There are two challenges in determining who triggered the import:

  1. The import can be triggered in multiple ways:
    • By clicking on the "Import" button on the feed page, import happens via a batch in the UI.
    • By clicking on the "Import in background" button on the feed page, import then happens during cron.
    • By periodic import, import then happens during cron.

    When an import happens in the UI, it will be easier to find out who triggered the import. It will then just be the current logged in user.
    When an import happens during cron (whether triggered via "Import in background" or via periodic import), the current logged in user is anonymous (user 0). Maybe it makes sense that during periodic import the revision author is "anonymous", but I'm not sure if that's the desired outcome in the event someone clicked "Import in background".

  2. During imports (both in the UI and during cron), a user switch happens. This is because some (field) validations need a user and to make sure there's no difference in validations between cron imports (user 0) and UI imports (logged in user) a switch is made to the feed author or - if the feed author is anonymous - to user 1.

    Because of this user switch, it can be extra challenging to determine who is the current logged in user. \Drupal::currentUser() will not return the current logged in user, but instead the user that was switched to. The user switch happens in \Drupal\feeds\FeedExecutable::switchAccount(). The class "Drupal\Core\Session\AccountSwitcher" doesn't to provide a method that lets you know who the original user was, so it looks like we would need to use the PHP reflection API in order to get to that information. That, or open/find a core issue to add a method called getOriginalUser() to that class.

Remaining tasks

  • Decide how to fix this issue
  • Implement a solution
  • Add test coverage (add test methods to \Drupal\Tests\feeds\Kernel\RevisionableEntityTest)
  • Review/test
  • Commit

User interface changes

Not sure yet, depends on what solution we'll go for.

API changes

Probably none.

Data model changes

None.

πŸ› Bug report
Status

Active

Version

3.0

Component

Code

Created by

πŸ‡³πŸ‡±Netherlands megachriz

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡ΈUnited States bkosborne New Jersey, USA

    Sounds like a lot of complexity around determining who the user is that triggered the import. But I think about it differently. The revision occurs because of an automated process. Sure, an individual may have triggered that process to occur, but they didn't make the change. The feed importer made the change. It's confusing to me that the revision would be set to whomever clicked a button to run the importer.

    I think the username should be to either Anonymous, or the value that was configured as the feed owner of created entities. For the latter, change the description of the configuration form to indicate that the value applies to both created entities and any revisions that the feed importer creates.

    Though, it is interesting that the revision_user can be mapped. I also hadn't realized that. I'm going to test that out.

    But, as of now, the code for creating revisions is in EntityProcessorBase:

          // Set new revision if needed.
          if ($this->configuration['revision']) {
            $entity->setNewRevision(TRUE);
            $entity->setRevisionCreationTime($this->dateTime->getRequestTime());
          }
    

    It lacks setting the revision author, so what happens is that the previous revision author is used instead, which is very confusing. I would consider this a bug because oftentimes it's not true. Seems like at the very least this should be fixed.

  • πŸ‡¨πŸ‡¦Canada No Sssweat

    I haven't had time to test with cron since I am not using it. This at least works for me for manual imports.

Production build 0.71.5 2024