Provide diff tokens for nodes

Created on 6 April 2023, over 1 year ago

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.
Feature request
Status

Active

Version

1.0

Component

Code

Created by

🇩🇪Germany FeyP

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

Merge Requests

Comments & Activities

  • Issue created by @FeyP
  • 🇩🇪Germany FeyP

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

  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • 🇩🇪Germany FeyP

    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.

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 8
    last update over 1 year ago
    39 pass, 1 fail
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 8
    last update over 1 year ago
    40 pass
  • 🇬🇧United Kingdom jaydenpearly

    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

  • Status changed to Needs work 6 months ago
  • 🇦🇺Australia acbramley

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

  • Merge request !93Issue #3352640: Provide diff tokens for nodes → (Open) created by FeyP
  • Pipeline finished with Failed
    6 months ago
    Total: 215s
    #211934
  • Pipeline finished with Failed
    6 months ago
    Total: 213s
    #211939
  • Pipeline finished with Success
    6 months ago
    Total: 213s
    #211944
  • Status changed to Needs review 6 months ago
  • 🇩🇪Germany FeyP

    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.

  • Pipeline finished with Success
    6 months ago
    Total: 208s
    #212472
  • Pipeline finished with Success
    6 months ago
    Total: 214s
    #212479
  • First commit to issue fork.
  • 🇦🇺Australia acbramley

    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.

  • 🇩🇪Germany FeyP

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

Production build 0.71.5 2024