Problem/Motivation

WSE overrides the controller for the node version history route via Drupal\wse\Routing\RouteSubscriber.php.

We then use form classes that use the VersionHistoryTrait to add a Workspace column to the revisions overview form.

If the Diff module is installed, there is pagination on the revisions overview form.

The Workspace column does not respect the pagination. Instead, it always shows data as if we're on the zeroth page.

This happens because the page query parameter has multiple values. For example, on the thrid page, I see this in the url: page=2%2C0. I would like that to be either page=2<code> of if not that then <code>page=2%2C2.

Steps to reproduce

  1. Install WSE with Diff and create revisions for some node in various workspaces.
  2. Either create more than 50 revisions or change the diff.general_settings.revision_pager_limit setting to some small number such that you can page the revisions overview form for the node.
  3. Note that the Workspaces column does not reflect the pager.

Proposed resolution

Option A: Update Drupal\wse\Diff\Form\WseRevisionOverviewForm such that it passes only the revisions relevant to the pagination.

Option B: Update Drupal\wse\Controller\VersionHistoryTrait such that it accesses revisions more explicitly, as opposed to simply iterating via next

Option C: Get the page query parameter to either use a single value or behave such that the second value matches the first value.

Remaining tasks

User interface changes

API changes

Data model changes

Comments

danflanagan8 created an issue. See original summary.

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
Issue summary: View changes

Actually the code in WSE looks good. I'm so confused by this.

The problem is that the pager is setting two values. For example, if I click to the third page, the pager parameter in the url is page=2%2C0. The result is that when Drupal\diff\Form\RevisionOverviewForm::getRevisionIds() is called from RevisionOverviewForm, the entity query uses 2 as the page number. But when that same method is subsequently called by Drupal\wse\Diff\Form\WseRevisionOverviewForm, the second value 0 is used.

So there's apparently stuff I don't understand about pages! How can we get the page query parameter to do what we want? That seems to be the question here.

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented

I think this may need to be fixed in diff, actually. We'll need to update Drupal\diff\Form\RevisionOverviewForm::getRevisionIds() such that the result is stored on a class property. That way when we call the same method from WSE we get the cached values instead of having the entity query run again. It's running the query a second time that is causing core's pager manager to get tricked into thinking there are multiple pagers.

  /**
   * Gets a list of node revision IDs for a specific node.
   *
   * Only returns revisions that are affected by the $node language.
   *
   * @param \Drupal\node\NodeInterface $node
   *   The node entity.
   *
   * @return int[]
   *   Node revision IDs (in descending order).
   */
  protected function getRevisionIds(NodeInterface $node): array {
    if (isset($this->revisionIds[$node->id()])) {
      return $this->revisionIds[$node->id()];
    }
    $entityType = $node->getEntityType();
    $result = $this->entityTypeManager->getStorage('node')->getQuery()
      // Access to the content has already been verified. Disable query-level
      // access checking so that revisions for unpublished content still
      // appear.
      ->accessCheck(FALSE)
      ->allRevisions()
      ->condition($entityType->getKey('langcode'), $node->language()->getId())
      ->condition($entityType->getKey('revision_translation_affected'), '1')
      ->condition($entityType->getKey('id'), $node->id())
      ->sort($entityType->getKey('revision'), 'DESC')
      ->pager($this->config->get('general_settings.revision_pager_limit'))
      ->execute();
    $this->revisionIds[$node->id()] = \array_keys($result);
    return $this->revisionIds[$node->id()];
  }
danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
Status: Active » Closed (duplicate)

Since I think it's better to make the fix in Diff, I'm going to close this issue. Is duplicate the right status? I'll go with that.

Diff issue: #3581969: Store result of RevisionOverviewForm::getRevisionIds to prevent paging bugs in subclasses

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.