Issue #2082315 by Wim Leers, realityloop, mrjmd: Tracker history markers...

Issue #2082315 by Wim Leers, realityloop, mrjmd: Tracker history markers ("new" and "updated" markers, "x new replies" links) forces render caching to be per user

core/modules/tracker/js/tracker-history.js

+/**
+ * Attaches behaviors for the Tracker module's History module integration.
+ *
+ * May only be loaded for authenticated users, with the History module enabled.
+ */
+(function ($, Drupal, window) {
+
+  "use strict";
+
+  /**
+   * Render "new" and "updated" node indicators, as well as "X new" replies links.
+   */
+  Drupal.behaviors.trackerHistory = {
+    attach: function (context) {
+      // Find all "new" comment indicator placeholders newer than 30 days ago that
+      // have not already been read after their last comment timestamp.
+      var nodeIDs = [];
+      var $nodeNewPlaceholders = $(context)
+        .find('[data-history-node-timestamp]')
+        .once('history')
+        .filter(function () {
+          var nodeTimestamp = parseInt(this.getAttribute('data-history-node-timestamp'), 10);
+          var nodeID = this.getAttribute('data-history-node-id');
+          if (Drupal.history.needsServerCheck(nodeID, nodeTimestamp)) {
+            nodeIDs.push(nodeID);
+            return true;
+          }
+          else {
+            return false;
+          }
+        });
+
+      // Find all "new" comment indicator placeholders newer than 30 days ago that
+      // have not already been read after their last comment timestamp.
+      var $newRepliesPlaceholders = $(context)
+        .find('[data-history-node-last-comment-timestamp]')
+        .once('history')
+        .filter(function () {
+          var lastCommentTimestamp = parseInt(this.getAttribute('data-history-node-last-comment-timestamp'), 10);
+          var nodeTimestamp = parseInt(this.previousSibling.previousSibling.getAttribute('data-history-node-timestamp'), 10);
+          // Discard placeholders that have zero comments.
+          if (lastCommentTimestamp === nodeTimestamp) {
+            return false;
+          }
+          var nodeID = this.previousSibling.previousSibling.getAttribute('data-history-node-id');
+          if (Drupal.history.needsServerCheck(nodeID, lastCommentTimestamp)) {
+            if (nodeIDs.indexOf(nodeID) === -1) {
+              nodeIDs.push(nodeID);
+            }
+            return true;
+          }
+          else {
+            return false;
+          }
+        });
+
+      if ($nodeNewPlaceholders.length === 0 && $newRepliesPlaceholders.length === 0) {
+        return;
+      }
+
+      // Fetch the node read timestamps from the server.
+      Drupal.history.fetchTimestamps(nodeIDs, function () {
+        processNodeNewIndicators($nodeNewPlaceholders);
+        processNewRepliesIndicators($newRepliesPlaceholders);
+      });
+    }
+  };
+
+  function processNodeNewIndicators($placeholders) {
+    var newNodeString = Drupal.t('new');
+    var updatedNodeString = Drupal.t('updated');
+
+    $placeholders.each(function (index, placeholder) {
+      var timestamp = parseInt(placeholder.getAttribute('data-history-node-timestamp'), 10);
+      var nodeID = placeholder.getAttribute('data-history-node-id');
+      var lastViewTimestamp = Drupal.history.getLastRead(nodeID);
+
+      if (timestamp > lastViewTimestamp) {
+        var message = (lastViewTimestamp === 0) ? newNodeString : updatedNodeString;
+        $(placeholder).append('<span class="marker">' + message + '</span>');
+      }
+    });
+  }
+
+  function processNewRepliesIndicators($placeholders) {
+    // Figure out which placeholders need the "x new" replies links.
+    var placeholdersToUpdate = {};
+    $placeholders.each(function (index, placeholder) {
+      var timestamp = parseInt(placeholder.getAttribute('data-history-node-last-comment-timestamp'), 10);
+      var nodeID = placeholder.previousSibling.previousSibling.getAttribute('data-history-node-id');
+      var lastViewTimestamp = Drupal.history.getLastRead(nodeID);
+
+      // Queue this placeholder's "X new" replies link to be downloaded from the
+      // server.
+      if (timestamp > lastViewTimestamp) {
+        placeholdersToUpdate[nodeID] = placeholder;
+      }
+    });
+
+    // Perform an AJAX request to retrieve node view timestamps.
+    var nodeIDs = Object.keys(placeholdersToUpdate);
+    if (nodeIDs.length === 0) {
+      return;
+    }
+    $.ajax({
+      url: Drupal.url('comments/render_new_comments_node_links'),
+      type: 'POST',
+      data: {'node_ids[]': nodeIDs},
+      dataType: 'json',
+      success: function (results) {
+        for (var nodeID in results) {
+          if (results.hasOwnProperty(nodeID) && placeholdersToUpdate.hasOwnProperty(nodeID)) {
+            var url = results[nodeID].first_new_comment_link;
+            var text = Drupal.formatPlural(results[nodeID].new_comment_count, '1 new', '@count new');
+            $(placeholdersToUpdate[nodeID]).append('<br /><a href="' + url + '">' + text + '</a>');
+          }
+        }
+      }
+    });
+  }
+
+})(jQuery, Drupal, window);

core/modules/tracker/src/Tests/TrackerTest.php

@@ -10,7 +10,9 @@
 use Drupal\comment\CommentInterface;
 use Drupal\comment\Tests\CommentTestTrait;
 use Drupal\Core\Cache\Cache;
+use Drupal\Core\Session\AccountInterface;
 use Drupal\field\Entity\FieldStorageConfig;
+use Drupal\node\Entity\Node;
 use Drupal\simpletest\WebTestBase;
 use Drupal\system\Tests\Cache\AssertPageCacheContextsAndTagsTrait;
 
@@ -54,6 +56,10 @@ protected function setUp() {
     $this->user = $this->drupalCreateUser($permissions);
     $this->otherUser = $this->drupalCreateUser($permissions);
     $this->addDefaultCommentField('node', 'page');
+    user_role_grant_permissions(AccountInterface::ANONYMOUS_ROLE, array(
+      'access content',
+      'access user profiles',
+    ));
   }
 
   /**
@@ -169,97 +175,54 @@ function testTrackerUser() {
   }
 
   /**
-   * Tests for the presence of the "new" flag for nodes.
+   * Tests the metadata for the "new"/"updated" indicators.
    */
-  function testTrackerNewNodes() {
+  function testTrackerHistoryMetadata() {
     $this->drupalLogin($this->user);
 
+    // Create a page node.
     $edit = array(
       'title' => $this->randomMachineName(8),
     );
-
     $node = $this->drupalCreateNode($edit);
-    $title = $edit['title'];
-    $this->drupalGet('activity');
-    $this->assertPattern('/' . $title . '.*new/', 'New nodes are flagged as such in the activity listing.');
-
-    $this->drupalGet('node/' . $node->id());
-    // Simulate the JavaScript on the node page to mark the node as read.
-    // @todo Get rid of curlExec() once https://www.drupal.org/node/2074037
-    //   lands.
-    $this->curlExec(array(
-      CURLOPT_URL => \Drupal::url('history.read_node', ['node' => $node->id()], array('absolute' => TRUE)),
-      CURLOPT_HTTPHEADER => array(
-        'Accept: application/json',
-      ),
-    ));
-    $this->drupalGet('activity');
-    $this->assertNoPattern('/' . $title . '.*new/', 'Visited nodes are not flagged as new.');
 
-    $this->drupalLogin($this->otherUser);
+    // Verify that the history metadata is present.
     $this->drupalGet('activity');
-    $this->assertPattern('/' . $title . '.*new/', 'For another user, new nodes are flagged as such in the tracker listing.');
-
-    $this->drupalGet('node/' . $node->id());
-    // Simulate the JavaScript on the node page to mark the node as read.
-    // @todo Get rid of curlExec() once https://www.drupal.org/node/2074037
-    //   lands.
-    $this->curlExec(array(
-      CURLOPT_URL => \Drupal::url('history.read_node', ['node' => $node->id()], array('absolute' => TRUE)),
-      CURLOPT_HTTPHEADER => array(
-        'Accept: application/json',
-      ),
-    ));
-    $this->drupalGet('activity');
-    $this->assertNoPattern('/' . $title . '.*new/', 'For another user, visited nodes are not flagged as new.');
-  }
-
-  /**
-   * Tests for comment counters on the tracker listing.
-   */
-  function testTrackerNewComments() {
-    $this->drupalLogin($this->user);
-
-    $node = $this->drupalCreateNode(array(
-      'title' => $this->randomMachineName(8),
-    ));
+    $this->assertHistoryMetadata($node->id(), $node->getChangedTime(), $node->getChangedTime());
+    $this->drupalGet('activity/' . $this->user->id());
+    $this->assertHistoryMetadata($node->id(), $node->getChangedTime(), $node->getChangedTime());
+    $this->drupalGet('user/' . $this->user->id() . '/activity');
+    $this->assertHistoryMetadata($node->id(), $node->getChangedTime(), $node->getChangedTime());
 
-    // Add a comment to the page.
+    // Add a comment to the page, make sure it is created after the node by
+    // sleeping for one second, to ensure the last comment timestamp is
+    // different from before.
     $comment = array(
       'subject[0][value]' => $this->randomMachineName(),
       'comment_body[0][value]' => $this->randomMachineName(20),
     );
+    sleep(1);
     $this->drupalPostForm('comment/reply/node/' . $node->id() . '/comment', $comment, t('Save'));
-    // The new comment is automatically viewed by the current user. Simulate the
-    // JavaScript that does this.
-    // @todo Get rid of curlExec() once https://www.drupal.org/node/2074037
-    //   lands.
-    $this->curlExec(array(
-      CURLOPT_URL => \Drupal::url('history.read_node', ['node' => $node->id()], array('absolute' => TRUE)),
-      CURLOPT_HTTPHEADER => array(
-        'Accept: application/json',
-      ),
-    ));
+    // Reload the node so that comment.module's hook_node_load()
+    // implementation can set $node->last_comment_timestamp for the freshly
+    // posted comment.
+    $node = Node::load($node->id());
 
-    $this->drupalLogin($this->otherUser);
+    // Verify that the history metadata is updated.
     $this->drupalGet('activity');
-    $this->assertText('1 new', 'New comments are counted on the tracker listing pages.');
-    $this->drupalGet('node/' . $node->id());
-
-    // Add another comment as otherUser.
-    $comment = array(
-      'subject[0][value]' => $this->randomMachineName(),
-      'comment_body[0][value]' => $this->randomMachineName(20),
-    );
-    // If the comment is posted in the same second as the last one then Drupal
-    // can't tell the difference, so we wait one second here.
-    sleep(1);
-    $this->drupalPostForm('comment/reply/node/' . $node->id(). '/comment', $comment, t('Save'));
+    $this->assertHistoryMetadata($node->id(), $node->getChangedTime(), $node->get('comment')->last_comment_timestamp);
+    $this->drupalGet('activity/' . $this->user->id());
+    $this->assertHistoryMetadata($node->id(), $node->getChangedTime(), $node->get('comment')->last_comment_timestamp);
+    $this->drupalGet('user/' . $this->user->id() . '/activity');
+    $this->assertHistoryMetadata($node->id(), $node->getChangedTime(), $node->get('comment')->last_comment_timestamp);
 
-    $this->drupalLogin($this->user);
+    // Log out, now verify that the metadata is still there, but the library is
+    // not.
+    $this->drupalLogout();
     $this->drupalGet('activity');
-    $this->assertText('1 new', 'New comments are counted on the tracker listing pages.');
-    $this->assertLink(t('1 new'));
+    $this->assertHistoryMetadata($node->id(), $node->getChangedTime(), $node->get('comment')->last_comment_timestamp, FALSE);
+    $this->drupalGet('user/' . $this->user->id() . '/activity');
+    $this->assertHistoryMetadata($node->id(), $node->getChangedTime(), $node->get('comment')->last_comment_timestamp, FALSE);
   }
 
   /**
@@ -371,8 +334,6 @@ function testTrackerCronIndexing() {
     foreach ($nodes as $i => $node) {
       $this->assertText($node->label(), format_string('Node @i is displayed on the tracker listing pages.', array('@i' => $i)));
     }
-    $this->assertText('1 new', 'One new comment is counted on the tracker listing pages.');
-    $this->assertText('updated', 'Node is listed as updated');
 
     // Fetch the site-wide tracker.
     $this->drupalGet('activity');
@@ -381,7 +342,6 @@ function testTrackerCronIndexing() {
     foreach ($nodes as $i => $node) {
       $this->assertText($node->label(), format_string('Node @i is displayed on the tracker listing pages.', array('@i' => $i)));
     }
-    $this->assertText('1 new', 'New comment is counted on the tracker listing pages.');
   }
 
   /**
@@ -411,4 +371,32 @@ function testTrackerAdminUnpublish() {
     $this->drupalGet('activity');
     $this->assertText(t('No content available.'), 'A node is displayed on the tracker listing pages.');
   }
+
+  /**
+   * Passes if the appropriate history metadata exists.
+   *
+   * Verify the data-history-node-id, data-history-node-timestamp and
+   * data-history-node-last-comment-timestamp attributes, which are used by the
+   * drupal.tracker-history library to add the appropriate "new" and "updated"
+   * indicators, as well as the "x new" replies link to the tracker.
+   * We do this in JavaScript to prevent breaking the render cache.
+   *
+   * @param $node_id
+   *   A node ID, that must exist as a data-history-node-id attribute
+   * @param $node_timestamp
+   *   A node timestamp, that must exist as a data-history-node-timestamp
+   *   attribute.
+   * @param $node_last_comment_timestamp
+   *   A node's last comment timestamp, that must exist as a
+   *   data-history-node-last-comment-timestamp attribute.
+   * @param bool $library_is_present
+   *   Whether the drupal.tracker-history library should be present or not.
+   */
+  function assertHistoryMetadata($node_id, $node_timestamp, $node_last_comment_timestamp, $library_is_present = TRUE) {
+    $settings = $this->getDrupalSettings();
+    $this->assertIdentical($library_is_present, isset($settings['ajaxPageState']) && in_array('tracker/history', explode(',', $settings['ajaxPageState']['libraries'])), 'drupal.tracker-history library is present.');
+    $this->assertIdentical(1, count($this->xpath('//table/tbody/tr/td[@data-history-node-id="' . $node_id . '" and @data-history-node-timestamp="' . $node_timestamp . '"]')), 'Tracker table cell contains the data-history-node-id and data-history-node-timestamp attributes for the node.');
+    $this->assertIdentical(1, count($this->xpath('//table/tbody/tr/td[@data-history-node-last-comment-timestamp="' . $node_last_comment_timestamp . '"]')), 'Tracker table cell contains the data-history-node-last-comment-timestamp attribute for the node.');
+  }
+
 }

core/modules/tracker/tracker.libraries.yml

+history:
+  version: VERSION
+  js:
+    js/tracker-history.js: {}
+  dependencies:
+    - core/jquery
+    - core/drupal
+    - history/api

core/modules/tracker/tracker.pages.inc

@@ -63,6 +63,13 @@ function tracker_page($account = NULL) {
       else {
         $nodes[$nid]->comment_count += $statistics->comment_count;
       }
+      // Make the last comment timestamp reflect the latest comment.
+      if (!isset($nodes[$nid]->last_comment_timestamp)) {
+        $nodes[$nid]->last_comment_timestamp = $statistics->last_comment_timestamp;
+      }
+      else {
+        $nodes[$nid]->last_comment_timestamp = max($nodes[$nid]->last_comment_timestamp, $statistics->last_comment_timestamp);
+      }
     }
 
     // Display the data.
@@ -75,25 +82,8 @@ function tracker_page($account = NULL) {
       $comments = 0;
       if ($node->comment_count) {
         $comments = $node->comment_count;
-
-        if ($new = \Drupal::service('comment.manager')->getCountNewComments($node)) {
-          $comments = array(
-            '#type' => 'link',
-            '#url' => $node->urlInfo(),
-            '#title' => \Drupal::translation()->formatPlural($new, '1 new', '@count new'),
-            '#options' => array(
-              'fragment' => 'new',
-            ),
-            '#prefix' => $node->comment_count . '<br />',
-          );
-        }
       }
 
-      $mark_build = array(
-        '#theme' => 'mark',
-        '#status' => node_mark($node->id(), $node->getChangedTime()),
-      );
-
       $row = array(
         'type' => SafeMarkup::checkPlain(node_get_type_label($node)),
         'title' => array(
@@ -101,8 +91,9 @@ function tracker_page($account = NULL) {
             '#type' => 'link',
             '#url' => $node->urlInfo(),
             '#title' => $node->getTitle(),
-            '#suffix' => ' ' . drupal_render($mark_build),
           ),
+          'data-history-node-id' => $node->id(),
+          'data-history-node-timestamp' => $node->getChangedTime(),
         ),
         'author' => array(
           'data' => array(
@@ -113,6 +104,7 @@ function tracker_page($account = NULL) {
         'comments' => array(
           'class' => array('comments'),
           'data' => $comments,
+          'data-history-node-last-comment-timestamp' => $node->last_comment_timestamp,
         ),
         'last updated' => array(
           'data' => t('!time ago', array(
@@ -148,5 +140,9 @@ function tracker_page($account = NULL) {
   $page['#cache']['tags'] = $cache_tags;
   $page['#cache']['contexts'][] = 'user.node_grants:view';
 
+  if (Drupal::moduleHandler()->moduleExists('history') && \Drupal::currentUser()->isAuthenticated()) {
+    $page['#attached']['library'][] = 'tracker/history';
+  }
+
   return $page;
 }