Problem/Motivation

The Drupal 7 version of this module did provide a number of tokens for nodes to show differences between revisions. Let's add those tokens to the Drupal 9/10 version.

Possible use cases include providing a preview of the changes in notification mails to reviewers for content moderation state changes or with contributed modules like ECA to execute actions based on the contents of diffs.

Proposed resolution

Provide tokens that will show differences between revisions in the configured default diff layout:

  1. [node:diff]: The differences between the current revision and the latest revision of a node.
  2. [node:diff-previous-latest]: The differences between the previous (before latest) revision and the latest revision of this node.
  3. [node:diff-current]: The differences between the current revision and the actual revision of this node.
  4. [node:diff-latest]: The differences between the actual revision and the latest revision of this node.
  5. [node:diff-previous]: The differences between the previous (before actual) revision and the actual revision of this node.

Issue fork diff-3352640

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

FeyP created an issue. See original summary.

feyp’s picture

Assigned: feyp » Unassigned
Status: Active » Needs review
StatusFileSize
new48.64 KB
new68.58 KB

Attached is a patch against 8.x-1.x-dev.

feyp’s picture

StatusFileSize
new3.39 KB
new49.44 KB
new69.33 KB

Found an issue with the bubbling of cache metadata, so new patch and interdiff attached. Also updated the tests to assert correct metadata.

Like the previous patch, this requires PHP 7.4 or later, but I think this is just due to some of the type hinting. Looks like the default testing is currently for PHP 7.3. If needed, I can probably provide an updated patch that is compatible with 7.3 as well. Let me know.

Anonymous’s picture

Thanks @FeyP ...
#3 - 3352640-03.patch is working.

Tested versions
Drupal: 10.2.7
diff: 1.7
token: 1.14
php: 8.3

+1 for RTBC

acbramley’s picture

Version: 8.x-1.x-dev » 2.x-dev
Status: Needs review » Needs work

Thank you for your contribution. This issue currently does not meet the Contribution guidelines which are required to get this change committed.

feyp’s picture

Status: Needs work » Needs review

Thanks @acbramley! I created an MR against the 2.x branch, that should be all that was missing to meet the contribution guidelines for NR.

One small note on this part of the code in TokenProvider:

      $default_layout = $this->diffLayoutManager->getDefaultLayout();
      if ($default_layout === '') {
        $replacements[$original] = $this->t('(No default diff layout configured.)');
        continue;
      }

This was an empty() check in the original patch for 1.x, because I actually ran into this during development. empty() checks are no longer allowed by the phpstan configuration of this module. Thus I looked at this again and the method has now a string return type hint. So I changed this to check for an empty string instead. But: getDefaultLayout() still has this return statement return \reset($plugins); and if $plugins is an empty array, it will not return a string, but boolean false. This means that we could probably remove this check in TokenProvider entirely, since the Layout Manager will then generate a white page (wrong return type). Or we could fix the wrong return type. For now I didn't change anything about this in the layout manager, because unlike the other change in the layout manager that is a part of this patch, this bug fix is not needed to pass tests here, so it could be done in a follow-up issue, if desired.

heddn made their first commit to this issue’s fork.

acbramley’s picture

Status: Needs review » Postponed

IMO this functionality could live in a separate module. Judging by the activity on this issue, it's not something that seems to be needed by a lot of users and is a non-trivial amount of code and tests to include.

feyp’s picture

Status: Postponed » Closed (won't fix)

Alright, separate module is not gonna happen, so let's close this won't fix.