When i try to list the changes between some revision by calling the url path :/node/{node}/revisions/view/{left_vid}/{right_vid}
its not displaying the revision for some nodes

When i inspected the code its calling the controller : /var/www-drupal/modules/diff/src/Form/RevisionOverviewForm.php

and in that the buildForm function

public function buildForm(array $form, FormStateInterface $form_state, $node = NULL) {
    $account = $this->currentUser;
    $langcode = $this->languageManager->getCurrentLanguage(LanguageInterface::TYPE_CONTENT)->getId();
    $langname = $this->languageManager->getLanguageName($langcode);
    $languages = $node->getTranslationLanguages();
    $has_translations = (count($languages) > 1);
    $node_storage = $this->entityManager->getStorage('node');
    $type = $node->getType();
    $vids = array_reverse($node_storage->revisionIds($node));
    $revision_count = count($vids);

    $build['#title'] = $has_translations ? $this->t('@langname revisions for %title', ['@langname' => $langname, '%title' => $node->label()]) : $this->t('Revisions for %title', ['%title' => $node->label()]);
    $build['nid'] = array(
      '#type' => 'hidden',
      '#value' => $node->id(),
    );

    $table_header = array(
      'revision' => $this->t('Revision'),
      'operations' => $this->t('Operations'),
    );

    // Allow comparisons only if there are 2 or more revisions.
    if ($revision_count > 1) {
      $table_header += array(
        'select_column_one' => '',
        'select_column_two' => '',
      );
    }

    $rev_revert_perm = $account->hasPermission("revert $type revisions") ||
      $account->hasPermission('revert all revisions') ||
      $account->hasPermission('administer nodes');
    $rev_delete_perm = $account->hasPermission("delete $type revisions") ||
      $account->hasPermission('delete all revisions') ||
      $account->hasPermission('administer nodes');
    $revert_permission = $rev_revert_perm && $node->access('update');
    $delete_permission = $rev_delete_perm && $node->access('delete');

    // Contains the table listing the revisions.
    $build['node_revisions_table'] = array(
      '#type' => 'table',
      '#header' => $table_header,
      '#attributes' => array('class' => array('diff-revisions')),
    );

    $build['node_revisions_table']['#attached']['library'][] = 'diff/diff.general';
    $build['node_revisions_table']['#attached']['drupalSettings']['diffRevisionRadios'] = $this->config->get('general_settings.radio_behavior');

    $latest_revision = TRUE;

    // Add rows to the table.
    foreach ($vids as $vid) {
      if ($revision = $node_storage->loadRevision($vid)) {
        if ($revision->hasTranslation($langcode) && $revision->getTranslation($langcode)->isRevisionTranslationAffected()) {

Its checking the last condition to display the diff . Diff displaying for some nodes and not displaying for some others . when i commented this condition (if ($revision->hasTranslation($langcode) && $revision->getTranslation($langcode)->isRevisionTranslationAffected()) { )the revisions are listing but the author as Anonymous (not verified) .What may be the issue ?

Issue fork diff-2882334

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

MukhtarM created an issue. See original summary.

mukhtarm’s picture

Issue summary: View changes
mukhtarm’s picture

Issue summary: View changes
johnchque’s picture

Status: Active » Postponed (maintainer needs more info)

Not sure what you expect and if the revisions you want to compare have the same language, but the module is designed to filter nodes of different languages. Need more info to know what you want to compare.

luke.leber’s picture

I believe that we are also running into a similar thing here, but with a different symptom.

To reproduce what we're seeing:

1) Install Drupal 8.5+
2) Install the node, content_moderation, and diff modules.
3) Create a node bundle, 'Test type'.
4) Enroll 'Test type' into content moderation
5) Create a new 'Test type' content item (revision 1 - draft).
6) Publish the content item (revision 2 - published).
7) Create a new draft from the content item (revision 3 - draft).
8) Revert to revision 2 (revision 4 - published).
9) Visit the revision overview page for the content item.

You should notice that the latest revision does not appear in the table rendered on the page and there no revision is marked as 'Current revision'.

After some investigation, it seems that the core behavior for the 'Current revision' designation has been changed by https://www.drupal.org/project/drupal/issues/2938947

Should this contributed module follow suit?

trevorbradley’s picture

I just stumbled across this myself. I have a large, monolingual Drupal site. Many revisions weren't showing up on the revisions tab.

The missing revisions were failing on the same line of code mentioned here:

if ($revision->hasTranslation($langcode) && $revision->getTranslation($langcode)->isRevisionTranslationAffected()) {

Specifically, $node->isRevisionTranslationAffected() was returning false for the missing revisions.

When I dug into my SQL I noticed that just over 2,000 node revisions had a NULL value:

select nid, vid, revision_translation_affected from node_field_revision where revision_translation_affected;

I have no idea why the value for this field is mostly 1, with many null exceptions. I do programatically update nodes from code from time to time, and those nodes sometimes get cloned.

In my case, I'm going to fix it in two ways:

1) I'm going to create a DB update on my site that sets revision_translation_affected for all nodes to 1, effectively running the following SQL: update node_field_revision set revision_translation_affected = 1;

2) When using an update to update my nodes and set new revisions, I'm also going to ensure that the following line of code is run alongside $node->setNewRevision(TRUE);:

$node->set('revision_translation_affected',TRUE);

That seems to make the problem go away on new nodes.

sheldonreed3’s picture

I have come across a similar, if not the same issue. When using the diff module, revisions and translations, we get empty rows and extra pages on the revisions tab. This is caused when, for instance, we have multiple pages of revisions for one or more translations. If the translation that is being viewed only has a few revisions, it will have full pagination, even though it doesn't need it. Possibly several blank entries before applicable revisions are viewable. The main translation ( en ), if it has few revisions, will display the same.

In our case, this seems to be easily addressed by altering the query in RevisionOverviewForm. However, it's not really in a good, alterable or extendable spot. A quick, low impact fix would be to pull the query in to it's own protected method. Wouldn't change anything functionally for the module, but would allow us to easily modify the query if needed.

I will roll a patch shortly to do so, but if anyone has any concerns with this method that would prevent it from being adopted, please let me know.

dalin’s picture

Status: Postponed (maintainer needs more info) » Needs review
Related issues: +#2769741: Node revisions page might not list a default revision
StatusFileSize
new851 bytes

This looks like a dupe of the same issue on the Core revisions page: #2769741: Node revisions page might not list a default revision

firewaller’s picture

Seems similar to this issue

firewaller’s picture

I can't speak for the separate issue where the "Default Revision" is missing, but it sounds like @sheldonreed3 is right that the initial issue seems to be related to the way the query is structured:

  • \Drupal\diff\Form\RevisionOverviewForm::buildForm query calls all revisions, regardless of language, from the node paged by $pagerLimit (i.e. 50)
  • The node has more than 50 revisions (across other translations)
  • The condition: "if ($revision->hasTranslation($langcode) && $revision->getTranslation($langcode)->isRevisionTranslationAffected()) { ... }" filters out the revisions that are not affecting this translation
  • Depending on how your translation revisions are ordered, less than 50 (sometimes 0) revisions will appear on the revision page

It seems like you'd want to filter the query with "langcode" and "revision_translation_affected" conditions, not the query results to make this behave appropriately. Something like this:

$query = $this->entityTypeManager->getStorage('node')->getQuery()
      ->condition($node->getEntityType()->getKey('id'), $node->id())
      ->condition('langcode', $langcode)
      ->condition('revision_translation_affected', 1)
      ->pager($pagerLimit)
      ->allRevisions()
      ->sort($node->getEntityType()->getKey('revision'), 'DESC')
      // Access to the content has already been verified. Disable query-level
      // access checking so that revisions for unpublished content still
      // appear.
      ->accessCheck(FALSE)
      ->execute();
ericawright’s picture

StatusFileSize
new948 bytes

I'm seeing the same issue where the revision_translation_affected field is NULL for a large number of the revisions and so this is returning false:
$revision->getTranslation($langcode)->isRevisionTranslationAffected()

My site only uses one language and I'm wondering if perhaps that is part of the problem. However, because of that, I've implemented this patch which works so that I can now see all of the revisions for the node. I'm sure the actual issue has to do with where the revision is created, but I didn't have any success in my investigation of that and I needed access to the revisions.

I wanted to add: it seems like the revisions have the revisions_translation_affected field set to NULL when the change to the content is a URL alias change, so I'm going to look into why that might be impacting this.

Status: Needs review » Needs work

The last submitted patch, 11: 2882334-diff-missing_some_revisions-11.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

pgrandeg’s picture

StatusFileSize
new617 bytes

Same issue here. My website is using multiple languages and the content has a lot of revisions. The results was inconsistent number of revision page and empty revision pages.
Thank you @sheldonreed3 #7 and @firewaller #10, you are absolutely right, the issue is regarding query construction. Here is the patch you recommended to us.

pookmish’s picture

I have a similar issue as @ericawright. A single language site. It seems if I change the title of the node, the revision will be displayed in the list. But if I change the only content in the fields (paragraphs fields), the revision is not displayed in the list. The patch in #13 did not resolve my issue, however the patch in #10 did.

It could be a combination of different things depending on the scenario for the language settings and how the node is being saved.

ruzanna_h’s picture

The same "No visible changes" message when change done in 'Geolocation' or 'International Phone' flelds.
Is this possible to fix?

dalin’s picture

@ruzanna_h Your issue sounds like something different. My guess is that you need to work with those particular modules to add support for the Diff module.

edysmp’s picture

In my case i want the revisions to be listed even if there is not any changes but in the Revision log message, like save a progress message.

aaron.ferris’s picture

Similar to #11 but in my case this is coming from the Scanner module, feels as though this is more of a band aid than a fix to the root problem but im yet to see why nodes saved in the Scanner module aren't setting revision_translation_affected.

Patch from #11 does work for me, but as above pretty sure the root is the scanner module. (in my use case - single language site)

Also worth mentioning that Core has the exact same issue as the Diff module - its the source of the node creation that's the problem.

Core:
NodeController->revisionOverview()

  if ($revision->hasTranslation($langcode) && $revision->getTranslation($langcode)->isRevisionTranslationAffected()) {
        $username = [
          '#theme' => 'username',
          '#account' => $revision->getRevisionUser(),
        ];

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

mrshowerman’s picture

Status: Needs work » Needs review

Created MR based on #13.
It removes the now unnecessary if ($revision->hasTranslation($langcode) && $revision->getTranslation($langcode)->isRevisionTranslationAffected()) { condition, and also sets the correct language on the revision before building the row (see #3269933-4: Inconsistent moderation state for translated content).

recrit’s picture

@mrshoweerman This issue appears to be related to #3418442: Revisions should be filtered by language in the database query. See patch #5 that correctly checks whether the entity type is translatable and it updates the automated tests. These fixes could be combined with this fix in 2882334

mrshowerman’s picture

Addressed #22.
I tried to fix the same issue as in #2769741: Node revisions page might not list a default revision here too by using an orConditionGroup() that's basically saying "either the revision affects the current translation, or it is the default revision", but that produced two additional LEFT joins of the same tables that are already used, which resulted in the conditions being applied to different versions of the same table.
Maybe someone else can help me out with that one.

heddn’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll, +Needs tests

For this to be reviewed and accepted, please review https://git.drupalcode.org/project/diff#contribution-guidelines. Since this is an older contribution, it needs a reroll/rebase. Additionally, we'll need some tests.

dieterholvoet’s picture

StatusFileSize
new795 bytes

Here's a simpler version of the patch posted in #11, for sites with only 1 language.

hamza_niazi’s picture

StatusFileSize
new2.05 KB

Pager must be applied AFTER filtering invalid revisions. We first load all revision IDs, then manually filter those that are translation-affected. Only then do we apply pager slicing.

acbramley’s picture

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

Postponing this on #2722307: Move translation based conditions into database query on revisions overview page once that is in we can apply the same logic to Diff.

acbramley’s picture

#3418442: Revisions should be filtered by language in the database query is ready for review, it would be great if anyone here could test those changes and report back if the issues are fixed.

tuutti’s picture

Status: Postponed » Needs work
StatusFileSize
new38.43 KB

Just applying #3418442: Revisions should be filtered by language in the database query doesn't seem to fix this. The revision list should have three revisions for English, but it's only showing the first one:

An image showing revisions

We've been using a custom version of this core patch #3390329: Latest revision is not displayed as latest revision when there are revisions impacting other translations for years. I'll try to port it to 2.x version at some point.

acbramley’s picture

Status: Needs work » Postponed

edit: removed

acbramley’s picture

Status: Postponed » Needs work
tuutti’s picture

I dug a bit more, and it looks like this is actually caused by nodes getting an incorrect (null) value in the revision_translation_affected field.

The revision page works just fine on the latest Core without any contrib modules (I didn’t test with the Diff module). In our case, the real culprit is probably the Entity Reference Revisions module, or some other module in conjunction with it.

acbramley’s picture

This often happens when reverting revisions, it could also be related to #3535230: Reverted revision is not listed on the version history when using Set as current revision

acbramley’s picture

Status: Needs work » Postponed (maintainer needs more info)

Please try and see if this is reproducible on Drupal 11.3.5 and the latest diff beta.

If it is, please provide steps to reproduce this.

trevorbradley’s picture

@acbramley - sorry for not getting back to you promptly - 2.5 year long project launches tomorrow. :D

Co-incidentally I upgraded diff to 2.0.0-beta5 about 2 weeks ago. I see no evidence this is persisting.

I think it's resolved!

trevorbradley’s picture

Oh weird - 7 years ago was not this project. :D ... but it was definitely seen on this project as well before the recent upgrade.

drumm’s picture

Issue tags: +affects drupal.org

We ran into this on new.drupal.org, with the null value in the revision_translation_affected field.

SELECT from_unixtime(changed), revision_translation_affected FROM node_field_revision WHERE nid = {nid};

Correlates exactly with the missing revisions. The content type we saw this on does have paragraphs. We are using 8.x-1.10 since the 2.x releases are still in beta.

luke.leber’s picture

I alluded to this in https://www.drupal.org/project/diff/issues/2452523#mr112-note734699 -- I think this is a core problem. With content moderation enabled it's much more prolific. In a non-multilingual context, saving an entity without making any changes to its fields (such as simply moving the moderation state from Pending to Published, for example) seems to produce a null in the revision_translation_affected column.

This even messes up the core revision UI. See comment #91 in #2452523: Offer a revisions tab for all entities.

fjgarlin’s picture

The underlying issue seems to be fixed in core for Drupal 11, but not backported to D10 (#2722307: Move translation based conditions into database query on revisions overview page).

We continued having the issue even with the diff module disabled. The issue was that the content type is not set as translatable, and that in combination with paragraphs seems to store the wrong value on that table. The D11 fix checks if the content type is translatable first, so the issue should no longer happen.

acbramley’s picture

Thanks @fjgarlin can this one be closed then?

fjgarlin’s picture

I'm not sure if this can be closed or whether it needs to replicate the logic from core regarding translations. It might need to implement a similar logic so probably changes are still needed here.

acbramley’s picture

@fjgarlin we already did that in #3418442: Revisions should be filtered by language in the database query

You should be using the 2.x branch.

fjgarlin’s picture

Oh, I see, that's great! We were using the 8.x-1.x as that's the recommended/stable release for "diff". Are there plans to tag 2.x stable?

acbramley’s picture

@fjgarlin I had been hoping more people would be able to test the betas but it's probably time to just release it and see what happens :) https://www.drupal.org/project/diff/releases/2.0.0