Merge "Remove notification_bundle_base"

db_patches/patch-drop-notification_bundle_base.sql

@@ -0,0 +1,5 @@
+-- Drop unused field notification_bundle_base and the indexes that contain it
+DROP INDEX /*i*/echo_notification_user_base_read_timestamp ON /*_*/echo_notification;
+DROP INDEX /*i*/echo_notification_user_base_timestamp ON /*_*/echo_notification;
+DROP INDEX /*i*/echo_notification_user_hash_base_timestamp ON /*_*/echo_notification;
+ALTER TABLE /*_*/echo_notification DROP notification_bundle_base;

echo.sql

@@ -41,8 +41,6 @@ CREATE TABLE /*_*/echo_notification (
 	notification_timestamp binary(14) not null,
 	-- Timestamp when the user read the notification, or null if unread
 	notification_read_timestamp binary(14) null,
-	-- No longer used, should be removed (T143763)
-	notification_bundle_base boolean not null default 1,
 	-- Hash for bundling together similar notifications. Notifications that can be bundled together
 	-- will have the same hash
 	notification_bundle_hash varchar(32) binary not null,
@@ -53,14 +51,8 @@ CREATE TABLE /*_*/echo_notification (
 
 -- Index to get a user's notifications in chronological order
 CREATE INDEX /*i*/echo_user_timestamp ON /*_*/echo_notification (notification_user,notification_timestamp);
--- No longer used, should be removed
-CREATE INDEX /*i*/echo_notification_user_base_read_timestamp ON /*_*/echo_notification (notification_user, notification_bundle_base, notification_read_timestamp);
--- No longer used, should be removed
-CREATE INDEX /*i*/echo_notification_user_base_timestamp ON /*_*/echo_notification (notification_user, notification_bundle_base, notification_timestamp, notification_event);
 -- Used by NotificationMapper::fetchNewestByUserBundleHash()
 CREATE INDEX /*i*/echo_notification_user_hash_timestamp ON /*_*/echo_notification (notification_user, notification_bundle_hash, notification_timestamp);
--- No longer used, should be removed
-CREATE INDEX /*i*/echo_notification_user_hash_base_timestamp ON /*_*/echo_notification (notification_user, notification_bundle_display_hash, notification_bundle_base, notification_timestamp);
 -- Used to get all notifications for a given event
 CREATE INDEX /*i*/echo_notification_event ON /*_*/echo_notification (notification_event);
 -- Used to get read/unread notifications for a user

extension.json

@@ -917,7 +917,6 @@
 		"ApiEchoNotifications": "includes/api/ApiEchoNotifications.php",
 		"ApiEchoNotificationsTest": "tests/phpunit/api/ApiEchoNotificationsTest.php",
 		"ApiEchoUnreadNotificationPages": "includes/api/ApiEchoUnreadNotificationPages.php",
-		"BackfillReadBundles": "maintenance/backfillReadBundles.php",
 		"BackfillUnreadWikis": "maintenance/backfillUnreadWikis.php",
 		"Bundleable": "includes/Bundleable.php",
 		"Bundler": "includes/Bundler.php",

includes/EchoHooks.php

@@ -192,7 +192,7 @@ class EchoHooks {
 				"$dir/db_patches/patch-drop-echo_target_page-etp_user.sql" );
 		}
 
-		$updater->addExtensionField( 'echo_notification', 'notification_bundle_base',
+		$updater->addExtensionField( 'echo_notification', 'notification_bundle_hash',
 			"$dir/db_patches/patch-notification-bundling-field.sql" );
 		// This index was renamed twice, first from type_page to event_type and
 		// later from event_type to echo_event_type
@@ -202,8 +202,10 @@ class EchoHooks {
 		}
 		$updater->dropExtensionTable( 'echo_subscription',
 			"$dir/db_patches/patch-drop-echo_subscription.sql" );
-		$updater->dropExtensionField( 'echo_event', 'event_timestamp',
-			"$dir/db_patches/patch-drop-echo_event-event_timestamp.sql" );
+		if ( $updater->getDB()->getType() !== 'sqlite' ) {
+			$updater->dropExtensionField( 'echo_event', 'event_timestamp',
+				"$dir/db_patches/patch-drop-echo_event-event_timestamp.sql" );
+		}
 		$updater->addExtensionField( 'echo_email_batch', 'eeb_event_hash',
 			"$dir/db_patches/patch-email_batch-new-field.sql" );
 		$updater->addExtensionField( 'echo_event', 'event_page_id',
@@ -233,6 +235,10 @@ class EchoHooks {
 			"$dir/db_patches/patch-drop-echo_event-event_page_namespace.sql" );
 		$updater->dropExtensionField( 'echo_event', 'event_page_title',
 			"$dir/db_patches/patch-drop-echo_event-event_page_title.sql" );
+		if ( $updater->getDB()->getType() !== 'sqlite' ) {
+			$updater->dropExtensionField( 'echo_notification', 'notification_bundle_base',
+				"$dir/db_patches/patch-drop-notification_bundle_base.sql" );
+		}
 	}
 
 	/**

includes/api/ApiEchoNotifications.php

@@ -454,7 +454,6 @@ class ApiEchoNotifications extends ApiQueryBase {
 		$row->notification_user = $user->getId();
 		$row->notification_timestamp = $maxTimestamp;
 		$row->notification_read_timestamp = null;
-		$row->notification_bundle_base = 1;
 		$row->notification_bundle_hash = md5( 'bogus' );
 		$row->notification_bundle_display_hash = md5( 'also-bogus' );
 

includes/mapper/NotificationMapper.php

@@ -23,24 +23,6 @@ class EchoNotificationMapper extends EchoAbstractMapper {
 			$dbw,
 			__METHOD__,
 			function ( IDatabase $dbw, $fname ) use ( $row, $listeners ) {
-				// Reset the bundle base if this notification has a display hash
-				// the result of this operation is that all previous notifications
-				// with the same display hash are set to non-base because new record
-				// is becoming the bundle base
-				if ( $row['notification_bundle_display_hash'] ) {
-					$dbw->update(
-						'echo_notification',
-						[ 'notification_bundle_base' => 0 ],
-						[
-							'notification_user' => $row['notification_user'],
-							'notification_bundle_display_hash' =>
-								$row['notification_bundle_display_hash'],
-							'notification_bundle_base' => 1
-						],
-						$fname
-					);
-				}
-
 				$row['notification_timestamp'] =
 					$dbw->timestamp( $row['notification_timestamp'] );
 				$dbw->insert( 'echo_notification', $row, $fname );

includes/model/Notification.php

@@ -30,13 +30,6 @@ class EchoNotification extends EchoAbstractEntity implements Bundleable {
 	 */
 	protected $readTimestamp;
 
-	/**
-	 * Determine whether this is a bundle base.  Default is 1,
-	 * which means it's a bundle base
-	 * @var int
-	 */
-	protected $bundleBase = 1;
-
 	/**
 	 * The hash used to determine if a set of event could be bundled
 	 * @var string
@@ -176,7 +169,6 @@ class EchoNotification extends EchoAbstractEntity implements Bundleable {
 		if ( $row->notification_read_timestamp ) {
 			$notification->readTimestamp = wfTimestamp( TS_MW, $row->notification_read_timestamp );
 		}
-		$notification->bundleBase = $row->notification_bundle_base;
 		$notification->bundleHash = $row->notification_bundle_hash;
 		$notification->bundleDisplayHash = $row->notification_bundle_display_hash;
 
@@ -193,7 +185,6 @@ class EchoNotification extends EchoAbstractEntity implements Bundleable {
 			'notification_user' => $this->user->getId(),
 			'notification_timestamp' => $this->timestamp,
 			'notification_read_timestamp' => $this->readTimestamp,
-			'notification_bundle_base' => $this->bundleBase,
 			'notification_bundle_hash' => $this->bundleHash,
 			'notification_bundle_display_hash' => $this->bundleDisplayHash
 		];
@@ -235,14 +226,6 @@ class EchoNotification extends EchoAbstractEntity implements Bundleable {
 		return $this->getReadTimestamp() !== null;
 	}
 
-	/**
-	 * Getter method
-	 * @return int Notification bundle base
-	 */
-	public function getBundleBase() {
-		return $this->bundleBase;
-	}
-
 	/**
 	 * Getter method
 	 * @return string|null Notification bundle hash
@@ -316,7 +299,6 @@ class EchoNotification extends EchoAbstractEntity implements Bundleable {
 			'notification_user',
 			'notification_timestamp',
 			'notification_read_timestamp',
-			'notification_bundle_base',
 			'notification_bundle_hash',
 			'notification_bundle_display_hash',
 		] );

maintenance/backfillReadBundles.php

@@ -1,76 +0,0 @@
-<?php
-
-$IP = getenv( 'MW_INSTALL_PATH' );
-if ( $IP === false ) {
-	$IP = __DIR__ . '/../../..';
-}
-require_once "$IP/maintenance/Maintenance.php";
-
-class BackfillReadBundles extends Maintenance {
-	public function __construct() {
-		parent::__construct();
-
-		$this->mDescription = "Backfill echo_notification.notification_read_timestamp for bundles";
-
-		$this->setBatchSize( 300 );
-
-		$this->requireExtension( 'Echo' );
-	}
-
-	public function execute() {
-		$dbFactory = MWEchoDbFactory::newFromDefault();
-		$dbw = $dbFactory->getEchoDb( DB_MASTER );
-		$dbr = $dbFactory->getEchoDb( DB_REPLICA );
-		$iterator = new BatchRowIterator(
-			$dbr,
-			'echo_notification',
-			[ 'notification_user', 'notification_event' ],
-			$this->mBatchSize
-		);
-		$iterator->setFetchColumns( [ 'notification_bundle_display_hash', 'notification_read_timestamp' ] );
-
-		$unreadNonBase = $dbr->selectSQLText(
-			'echo_notification',
-			'notification_bundle_display_hash',
-			[
-				'notification_bundle_base' => 0,
-				'notification_read_timestamp IS NULL',
-				"notification_bundle_display_hash <> ''",
-			]
-		);
-
-		$iterator->addConditions( [
-			'notification_bundle_base' => 1,
-			'notification_read_timestamp IS NOT NULL',
-			"notification_bundle_display_hash IN ( $unreadNonBase )",
-		] );
-
-		$processed = 0;
-		foreach ( $iterator as $batch ) {
-			foreach ( $batch as $row ) {
-				$userId = $row->notification_user;
-				$displayHash = $row->notification_bundle_display_hash;
-				$readTimestamp = $row->notification_read_timestamp;
-
-				$dbw->update(
-					'echo_notification',
-					[ 'notification_read_timestamp' => $readTimestamp ],
-					[
-						'notification_user' => $userId,
-						'notification_bundle_display_hash' => $displayHash,
-						'notification_bundle_base' => 0,
-						'notification_read_timestamp IS NULL',
-					]
-				);
-
-				$processed += $dbw->affectedRows();
-			}
-
-			$this->output( "Updated $processed notifications.\n" );
-			$dbFactory->waitForSlaves();
-		}
-	}
-}
-
-$maintClass = BackfillReadBundles::class;
-require_once RUN_MAINTENANCE_IF_MAIN;

tests/phpunit/mapper/NotificationMapperTest.php

@@ -31,7 +31,6 @@ class EchoNotificationMapperTest extends MediaWikiTestCase {
 				'notification_user' => 1,
 				'notification_timestamp' => '20140615101010',
 				'notification_read_timestamp' => '',
-				'notification_bundle_base' => 1,
 				'notification_bundle_hash' => 'testhash',
 				'notification_bundle_display_hash' => 'testdisplayhash'
 			]
@@ -69,7 +68,6 @@ class EchoNotificationMapperTest extends MediaWikiTestCase {
 				'notification_user' => 1,
 				'notification_timestamp' => '20140615101010',
 				'notification_read_timestamp' => '20140616101010',
-				'notification_bundle_base' => 1,
 				'notification_bundle_hash' => 'testhash',
 				'notification_bundle_display_hash' => 'testdisplayhash'
 			]
@@ -123,7 +121,6 @@ class EchoNotificationMapperTest extends MediaWikiTestCase {
 			'notification_user' => 1,
 			'notification_timestamp' => '20140615101010',
 			'notification_read_timestamp' => '20140616101010',
-			'notification_bundle_base' => 1,
 			'notification_bundle_hash' => 'testhash',
 			'notification_bundle_display_hash' => 'testdisplayhash'
 		];
@@ -151,7 +148,6 @@ class EchoNotificationMapperTest extends MediaWikiTestCase {
 			'notification_user' => 1,
 			'notification_timestamp' => '20140615101010',
 			'notification_read_timestamp' => '20140616101010',
-			'notification_bundle_base' => 1,
 			'notification_bundle_hash' => 'testhash',
 			'notification_bundle_display_hash' => 'testdisplayhash'
 		];

tests/phpunit/model/NotificationTest.php

@@ -54,7 +54,6 @@ class EchoNotificationTest extends MediaWikiTestCase {
 			'notification_event' => 1,
 			'notification_timestamp' => time(),
 			'notification_read_timestamp' => '',
-			'notification_bundle_base' => 1,
 			'notification_bundle_hash' => 'testhash',
 			'notification_bundle_display_hash' => 'testdisplayhash'
 		];