Consolidate duplicated unseen change logic and fix inconsistent code
authorAaron Schulz <aschulz@wikimedia.org>
Fri, 3 May 2019 20:23:24 +0000 (13:23 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Fri, 3 May 2019 20:23:24 +0000 (13:23 -0700)
Bug: T218511
Change-Id: I42387498dff0b1fc31f006ce3ba71241de9d45d7

includes/specials/SpecialWatchlist.php
includes/watcheditem/WatchedItemStoreInterface.php

index bac059d..812f1b0 100644 (file)
@@ -199,10 +199,7 @@ class SpecialWatchlist extends ChangesListSpecialPage {
                                        'description' => 'rcfilters-filter-watchlistactivity-unseen-description',
                                        'cssClassSuffix' => 'watchedunseen',
                                        'isRowApplicableCallable' => function ( $ctx, RecentChange $rc ) {
-                                               $changeTs = $rc->getAttribute( 'rc_timestamp' );
-                                               $lastVisitTs = $this->getLatestSeenTimestamp( $rc );
-
-                                               return $lastVisitTs !== null && $changeTs >= $lastVisitTs;
+                                               return !$this->isChangeEffectivelySeen( $rc );
                                        },
                                ],
                                [
@@ -211,10 +208,7 @@ class SpecialWatchlist extends ChangesListSpecialPage {
                                        'description' => 'rcfilters-filter-watchlistactivity-seen-description',
                                        'cssClassSuffix' => 'watchedseen',
                                        'isRowApplicableCallable' => function ( $ctx, RecentChange $rc ) {
-                                               $changeTs = $rc->getAttribute( 'rc_timestamp' );
-                                               $lastVisitTs = $this->getLatestSeenTimestamp( $rc );
-
-                                               return $lastVisitTs === null || $changeTs < $lastVisitTs;
+                                               return $this->isChangeEffectivelySeen( $rc );
                                        }
                                ],
                        ],
@@ -548,10 +542,9 @@ class SpecialWatchlist extends ChangesListSpecialPage {
                        $rc->counter = $counter++;
 
                        if ( $this->getConfig()->get( 'ShowUpdatedMarker' ) ) {
-                               $lastVisitTs = $this->getLatestSeenTimestamp( $rc );
-                               $updated = ( $lastVisitTs > $rc->getAttribute( 'timestamp' ) );
+                               $unseen = !$this->isChangeEffectivelySeen( $rc );
                        } else {
-                               $updated = false;
+                               $unseen = false;
                        }
 
                        if ( isset( $watchedItemStore ) ) {
@@ -561,7 +554,7 @@ class SpecialWatchlist extends ChangesListSpecialPage {
                                $rc->numberofWatchingusers = 0;
                        }
 
-                       $changeLine = $list->recentChangesLine( $rc, $updated, $counter );
+                       $changeLine = $list->recentChangesLine( $rc, $unseen, $counter );
                        if ( $changeLine !== false ) {
                                $s .= $changeLine;
                        }
@@ -869,9 +862,19 @@ class SpecialWatchlist extends ChangesListSpecialPage {
 
        /**
         * @param RecentChange $rc
-        * @return string TS_MW timestamp
+        * @return bool User viewed the revision or a newer one
+        */
+       protected function isChangeEffectivelySeen( RecentChange $rc ) {
+               $lastVisitTs = $this->getLatestSeenTimestampIfHasUnseen( $rc );
+
+               return $lastVisitTs === null || $lastVisitTs > $rc->getAttribute( 'rc_timestamp' );
+       }
+
+       /**
+        * @param RecentChange $rc
+        * @return string|null TS_MW timestamp or null if all revision were seen
         */
-       protected function getLatestSeenTimestamp( RecentChange $rc ) {
+       private function getLatestSeenTimestampIfHasUnseen( RecentChange $rc ) {
                return $this->watchStore->getLatestNotificationTimestamp(
                        $rc->getAttribute( 'wl_notificationtimestamp' ),
                        $rc->getPerformer(),
index 349d98a..b6d7b68 100644 (file)
@@ -337,7 +337,7 @@ interface WatchedItemStoreInterface {
         * @param string|null $timestamp Value of wl_notificationtimestamp from the DB
         * @param User $user
         * @param LinkTarget $target
-        * @return string TS_MW timestamp or null
+        * @return string|null TS_MW timestamp or null if all revision were seen
         */
        public function getLatestNotificationTimestamp( $timestamp, User $user, LinkTarget $target );
 }