jenkins-bot has submitted this change and it was merged.
Change subject: Remove etp_user
......................................................................
Remove etp_user
Target page entries used to exist for each user
that was notified for an event. They were
removed as the notifications were marked as read.
Now they remain so that the association between
pages and events can be used for moderation
regardless of the notifications read status.
This patch removes everything about
echo_target_page.etp_user from sql and php code.
Bug: T143959
Change-Id: Ib57510e6b0e9202a7e035f8ea59955dca8a0b24a
---
M Hooks.php
M autoload.php
A db_patches/patch-drop-echo_target_page-etp_user.sql
A db_patches/patch-drop-echo_target_page-etp_user.sqlite.sql
M echo.sql
M includes/jobs/NotificationDeleteJob.php
M includes/mapper/EventMapper.php
M includes/mapper/NotificationMapper.php
M includes/mapper/TargetPageMapper.php
M includes/model/Event.php
M includes/model/Notification.php
M includes/model/TargetPage.php
D maintenance/removeInvalidTargetPage.php
M tests/browser/features/support/components/notifications.rb
M tests/phpunit/mapper/NotificationMapperTest.php
M tests/phpunit/mapper/TargetPageMapperTest.php
M tests/phpunit/model/NotificationTest.php
M tests/phpunit/model/TargetPageTest.php
18 files changed, 107 insertions(+), 394 deletions(-)
Approvals:
Catrope: Looks good to me, approved
jenkins-bot: Verified
diff --git a/Hooks.php b/Hooks.php
index 5968b04..07b7d1e 100644
--- a/Hooks.php
+++ b/Hooks.php
@@ -147,6 +147,7 @@
$updater->modifyExtensionField( 'echo_event',
'event_agent', "$dir/db_patches/patch-event_agent-split.sqlite.sql" );
$updater->modifyExtensionField( 'echo_event',
'event_variant', "$dir/db_patches/patch-event_variant_nullability.sqlite.sql" );
$updater->addExtensionField( 'echo_target_page',
'etp_id', "$dir/db_patches/patch-multiple_target_pages.sqlite.sql" );
+ $updater->dropExtensionField( 'echo_target_page',
'etp_user', "$dir/db_patches/patch-drop-echo_target_page-etp_user.sqlite.sql" );
// There is no need to run the patch-event_extra-size
or patch-event_agent_ip-size because
// sqlite ignores numeric arguments in parentheses that
follow the type name (ex: VARCHAR(255))
// see http://www.sqlite.org/datatype3.html Section 2.2
for more info
@@ -156,6 +157,7 @@
$updater->modifyExtensionField( 'echo_event',
'event_extra', "$dir/db_patches/patch-event_extra-size.sql" );
$updater->modifyExtensionField( 'echo_event',
'event_agent_ip', "$dir/db_patches/patch-event_agent_ip-size.sql" );
$updater->addExtensionField( 'echo_target_page',
'etp_id', "$dir/db_patches/patch-multiple_target_pages.sql" );
+ $updater->dropExtensionField( 'echo_target_page',
'etp_user', "$dir/db_patches/patch-drop-echo_target_page-etp_user.sql" );
}
$updater->addExtensionField( 'echo_notification',
'notification_bundle_base',
@@ -1275,7 +1277,6 @@
$updateFields[] = array( 'echo_event', 'event_agent_id', 'db'
=> $dbw );
$updateFields[] = array( 'echo_notification',
'notification_user', 'db' => $dbw, 'options' => array( 'IGNORE' ) );
$updateFields[] = array( 'echo_email_batch', 'eeb_user_id',
'db' => $dbw, 'options' => array( 'IGNORE' ) );
- $updateFields[] = array( 'echo_target_page', 'etp_user', 'db'
=> $dbw, 'options' => array( 'IGNORE' ) );
return true;
}
@@ -1294,7 +1295,6 @@
$dbw = MWEchoDbFactory::newFromDefault()->getEchoDb( DB_MASTER
);
$tables['echo_notification'] = array( 'notification_user', 'db'
=> $dbw );
$tables['echo_email_batch'] = array( 'eeb_user_id', 'db' =>
$dbw );
- $tables['echo_target_page'] = array( 'etp_user', 'db' => $dbw );
return true;
}
diff --git a/autoload.php b/autoload.php
index a828fc5..b8b7eaa 100644
--- a/autoload.php
+++ b/autoload.php
@@ -111,7 +111,6 @@
'NotificationsTest' => __DIR__ . '/tests/NotificationsTest.php',
'ProcessEchoEmailBatch' => __DIR__ .
'/maintenance/processEchoEmailBatch.php',
'RemoveInvalidNotification' => __DIR__ .
'/maintenance/removeInvalidNotification.php',
- 'RemoveInvalidTargetPage' => __DIR__ .
'/maintenance/removeInvalidTargetPage.php',
'RemoveOrphanedEvents' => __DIR__ .
'/maintenance/removeOrphanedEvents.php',
'ResourceLoaderEchoImageModule' => __DIR__ .
'/includes/ResourceLoaderEchoImageModule.php',
'SpecialDisplayNotificationsConfiguration' => __DIR__ .
'/includes/special/SpecialDisplayNotificationsConfiguration.php',
diff --git a/db_patches/patch-drop-echo_target_page-etp_user.sql
b/db_patches/patch-drop-echo_target_page-etp_user.sql
new file mode 100644
index 0000000..dad413c
--- /dev/null
+++ b/db_patches/patch-drop-echo_target_page-etp_user.sql
@@ -0,0 +1,11 @@
+-- Patch to drop unused etp_user
+
+-- Drop indexes depending on etp_user
+DROP INDEX /*i*/echo_target_page_user_event ON /*_*/echo_target_page;
+DROP INDEX /*i*/echo_target_page_user_page_event ON /*_*/echo_target_page;
+
+-- Drop etp_user column
+ALTER TABLE /*_*/echo_target_page DROP etp_user;
+
+-- Add index on etp_event
+CREATE INDEX /*i*/echo_target_page_event ON /*_*/echo_target_page (etp_event);
diff --git a/db_patches/patch-drop-echo_target_page-etp_user.sqlite.sql
b/db_patches/patch-drop-echo_target_page-etp_user.sqlite.sql
new file mode 100644
index 0000000..17aec6e
--- /dev/null
+++ b/db_patches/patch-drop-echo_target_page-etp_user.sqlite.sql
@@ -0,0 +1,26 @@
+-- Patch to drop unused etp_user
+
+-- give current table temporary name
+ALTER TABLE /*_*/echo_target_page RENAME TO /*_*/temp_echo_target_page;
+
+-- recreate table without etp_user
+CREATE TABLE /*_*/echo_target_page (
+ etp_id int unsigned not null primary key auto_increment,
+ etp_page int unsigned not null default 0,
+ etp_event int unsigned not null default 0
+) /*$wgDBTableOptions*/;
+
+-- copy over old data into new table
+INSERT INTO /*_*/echo_target_page
+ (etp_page, etp_event)
+SELECT DISTINCT
+ etp_page, etp_event
+FROM
+ /*_*/temp_echo_target_page;
+
+-- drop the original table
+DROP TABLE /*_*/temp_echo_target_page;
+
+-- recreate indexes
+CREATE INDEX /*i*/echo_target_page_event ON /*_*/echo_target_page (etp_event);
+CREATE INDEX /*i*/echo_target_page_page_event ON /*_*/echo_target_page
(etp_page, etp_event);
diff --git a/echo.sql b/echo.sql
index e474fc0..9334252 100644
--- a/echo.sql
+++ b/echo.sql
@@ -48,11 +48,9 @@
CREATE TABLE /*_*/echo_target_page (
etp_id int unsigned not null primary key auto_increment,
- etp_user int unsigned not null default 0,
etp_page int unsigned not null default 0,
etp_event int unsigned not null default 0
) /*$wgDBTableOptions*/;
-CREATE INDEX /*i*/echo_target_page_user_event ON /*_*/echo_target_page
(etp_user, etp_event);
-CREATE INDEX /*i*/echo_target_page_user_page_event ON /*_*/echo_target_page
(etp_user, etp_page, etp_event);
+CREATE INDEX /*i*/echo_target_page_event ON /*_*/echo_target_page (etp_event);
CREATE INDEX /*i*/echo_target_page_page_event ON /*_*/echo_target_page
(etp_page, etp_event);
diff --git a/includes/jobs/NotificationDeleteJob.php
b/includes/jobs/NotificationDeleteJob.php
index a9152cf..7224f97 100644
--- a/includes/jobs/NotificationDeleteJob.php
+++ b/includes/jobs/NotificationDeleteJob.php
@@ -57,11 +57,6 @@
$user, $notif->getEvent()->getId()
);
if ( $res ) {
- $res = $targetMapper->deleteByUserEventOffset(
- $user, $notif->getEvent()->getId()
- );
- }
- if ( $res ) {
$notifUser = MWEchoNotifUser::newFromUser(
$user );
$notifUser->resetNotificationCount( DB_MASTER );
}
diff --git a/includes/mapper/EventMapper.php b/includes/mapper/EventMapper.php
index b907b25..06ca6be 100644
--- a/includes/mapper/EventMapper.php
+++ b/includes/mapper/EventMapper.php
@@ -29,6 +29,11 @@
$id = $dbw->insertId();
}
+ $listeners = $this->getMethodListeners( __FUNCTION__ );
+ foreach ( $listeners as $listener ) {
+ $dbw->onTransactionIdle( $listener );
+ }
+
return $id;
} else {
return false;
@@ -100,7 +105,7 @@
),
__METHOD__,
array( 'GROUP BY' => 'etp_event' ),
- array( 'echo_target_page' => array( 'JOIN',
'event_id=etp_event' ) )
+ array( 'echo_target_page' => array( 'INNER JOIN',
'event_id=etp_event' ) )
);
if ( $res ) {
foreach ( $res as $row ) {
@@ -139,19 +144,19 @@
$dbr = $this->dbFactory->getEchoDb( DB_SLAVE );
$res = $dbr->select(
- array( 'echo_target_page', 'echo_event',
'echo_notification' ),
+ array( 'echo_event', 'echo_notification',
'echo_target_page' ),
'*',
array(
- 'etp_user' => $user->getId(),
- 'etp_page' => $pageId,
'event_deleted' => 0,
+ 'notification_user' => $user->getId(),
'notification_read_timestamp' => null,
+ 'etp_page' => $pageId,
),
__METHOD__,
null,
array(
- 'echo_event' => array( 'INNER JOIN',
'etp_event=event_id' ),
- 'echo_notification' => array( 'INNER JOIN',
array( 'notification_event=etp_event', 'notification_user=etp_user' ) ),
+ 'echo_target_page' => array( 'INNER JOIN',
'etp_event=event_id' ),
+ 'echo_notification' => array( 'INNER JOIN',
array( 'notification_event=event_id' ) ),
)
);
diff --git a/includes/mapper/NotificationMapper.php
b/includes/mapper/NotificationMapper.php
index 6cc3064..3394afd 100644
--- a/includes/mapper/NotificationMapper.php
+++ b/includes/mapper/NotificationMapper.php
@@ -6,22 +6,6 @@
class EchoNotificationMapper extends EchoAbstractMapper {
/**
- * @var EchoTargetPageMapper
- */
- protected $targetPageMapper;
-
- public function __construct(
- MWEchoDbFactory $dbFactory = null,
- EchoTargetPageMapper $targetPageMapper = null
- ) {
- parent::__construct( $dbFactory );
- if ( $targetPageMapper === null ) {
- $targetPageMapper = new EchoTargetPageMapper(
$this->dbFactory );
- }
- $this->targetPageMapper = $targetPageMapper;
- }
-
- /**
* Insert a notification record
* @param EchoNotification
* @return null
@@ -29,9 +13,9 @@
public function insert( EchoNotification $notification ) {
$dbw = $this->dbFactory->getEchoDb( DB_MASTER );
- $row = $notification->toDbArray();
$listeners = $this->getMethodListeners( __FUNCTION__ );
+ $row = $notification->toDbArray();
DeferredUpdates::addUpdate( new AtomicSectionUpdate(
$dbw,
__METHOD__,
@@ -57,7 +41,6 @@
$row['notification_timestamp'] =
$dbw->timestamp(
$row['notification_timestamp'] );
$res = $dbw->insert( 'echo_notification', $row,
$fname );
-
if ( $res ) {
foreach ( $listeners as $listener ) {
$dbw->onTransactionIdle(
$listener );
diff --git a/includes/mapper/TargetPageMapper.php
b/includes/mapper/TargetPageMapper.php
index 44e9b29..d5d0b6c 100644
--- a/includes/mapper/TargetPageMapper.php
+++ b/includes/mapper/TargetPageMapper.php
@@ -10,74 +10,9 @@
* @var string[]
*/
protected static $fields = array(
- 'etp_user',
'etp_page',
'etp_event'
);
-
- /**
- * Fetch EchoTargetPage instances by user & page_id. The resulting
- * array is indexed by the event id. Each entry contains an array
- * of EchoTargetPage instances.
- *
- * @param User $user
- * @param int|int[] $pageId One or more page ids to fetch target pages
of
- * @return EchoTargetPage[][]|boolean
- */
- public function fetchByUserPageId( User $user, $pageId ) {
- $dbr = $this->dbFactory->getEchoDb( DB_SLAVE );
-
- $res = $dbr->select(
- array( 'echo_target_page', 'echo_event' ),
- array_merge( self::$fields, array( 'event_type' ) ),
- array(
- 'etp_user' => $user->getId(),
- 'etp_page' => $pageId
- ),
- __METHOD__,
- array(),
- array( 'echo_event' => array( 'JOIN',
'etp_event=event_id' ) )
- );
- if ( $res ) {
- $targetPages = array();
- foreach ( $res as $row ) {
- $targetPages[$row->etp_event][] =
EchoTargetPage::newFromRow( $row );
- }
-
- return $targetPages;
- } else {
- return false;
- }
- }
-
- /**
- * Fetch ids of events associated with a page
- *
- * @param int $pageId
- * @return int[]|false
- */
- public function fetchEventIdsByPageId( $pageId ) {
- $dbr = $this->dbFactory->getEchoDb( DB_SLAVE );
-
- $res = $dbr->select(
- array( 'echo_target_page' ),
- array( 'eventId' => 'DISTINCT etp_event' ),
- array(
- 'etp_page' => $pageId
- ),
- __METHOD__
- );
- if ( $res ) {
- $eventIds = array();
- foreach ( $res as $row ) {
- $eventIds[] = $row->eventId;
- }
-
- return $eventIds;
- } else {
- return false;
- }
- }
/**
* Insert an EchoTargetPage instance into the database
@@ -94,49 +29,4 @@
return $res;
}
-
- /**
- * Delete an EchoTargetPage instance from the database
- *
- * @param EchoTargetPage
- * @return boolean
- */
- public function delete( EchoTargetPage $targetPage ) {
- $dbw = $this->dbFactory->getEchoDb( DB_MASTER );
-
- $res = $dbw->delete(
- 'echo_target_page',
- array(
- 'etp_user' => $targetPage->getUser()->getId(),
- 'etp_page' => $targetPage->getPageId(),
- 'etp_event' => $targetPage->getEventId()
- ),
- __METHOD__
- );
-
- return $res;
- }
-
- /**
- * Delete multiple EchoTargetPage records by user & event_id offset
- *
- * @param User $user
- * @param int $eventId
- * @return boolean
- */
- public function deleteByUserEventOffset( User $user, $eventId ) {
- $dbw = $this->dbFactory->getEchoDb( DB_MASTER );
-
- $res = $dbw->delete(
- 'echo_target_page',
- array(
- 'etp_user' => $user->getId(),
- 'etp_event < ' . (int)$eventId
- ),
- __METHOD__
- );
-
- return $res;
- }
-
}
diff --git a/includes/model/Event.php b/includes/model/Event.php
index 8641d70..67a1f43 100644
--- a/includes/model/Event.php
+++ b/includes/model/Event.php
@@ -226,6 +226,47 @@
protected function insert() {
$eventMapper = new EchoEventMapper();
$this->id = $eventMapper->insert( $this );
+
+ $event = $this;
+ $targetPages = self::resolveTargetPages( $this->getExtraParam(
'target-page' ) );
+ if ( $targetPages ) {
+ $eventMapper->attachListener( 'insert',
'add-target-page', function () use ( $event, $targetPages ) {
+ $targetMapper = new EchoTargetPageMapper();
+ foreach ( $targetPages as $title ) {
+ $targetPage = EchoTargetPage::create(
$title, $event );
+ if ( $targetPage ) {
+ $targetMapper->insert(
$targetPage );
+ }
+ }
+ } );
+ }
+ }
+
+ /**
+ * @param int[]|int|false $targetPageIds
+ * @return Title[]
+ */
+ protected static function resolveTargetPages( $targetPageIds ) {
+ if ( !$targetPageIds ) {
+ return array();
+ }
+ if ( !is_array( $targetPageIds ) ) {
+ $targetPageIds = array( $targetPageIds );
+ }
+ $result = array();
+ foreach ( $targetPageIds as $targetPageId ) {
+ // Make sure the target-page id is a valid id
+ $title = Title::newFromID( $targetPageId );
+ // Try master if there is no match
+ if ( !$title ) {
+ $title = Title::newFromID( $targetPageId,
Title::GAID_FOR_UPDATE );
+ }
+ if ( $title ) {
+ $result[] = $title;
+ }
+ }
+
+ return $result;
}
/**
diff --git a/includes/model/Notification.php b/includes/model/Notification.php
index c640e2c..3114969 100644
--- a/includes/model/Notification.php
+++ b/includes/model/Notification.php
@@ -130,23 +130,7 @@
}
}
- // Create a target page object if specified by event
- $event = $this->event;
- $user = $this->user;
- $targetPages = self::resolveTargetPages( $event->getExtraParam(
'target-page' ) );
- if ( $targetPages ) {
- $notifMapper->attachListener( 'insert',
'add-target-page', function () use ( $event, $user, $targetPages ) {
- $targetMapper = new EchoTargetPageMapper();
- foreach ( $targetPages as $title ) {
- $targetPage = EchoTargetPage::create(
$user, $title, $event );
- if ( $targetPage ) {
- $targetMapper->insert(
$targetPage );
- }
- }
- } );
- }
-
- $notifUser = MWEchoNotifUser::newFromUser( $user );
+ $notifUser = MWEchoNotifUser::newFromUser( $this->user );
$section = $this->event->getSection();
// Add listener to refresh notification count upon insert
@@ -158,37 +142,10 @@
$notifMapper->insert( $this );
- if ( $event->getType() === 'edit-user-talk' ) {
+ if ( $this->event->getType() === 'edit-user-talk' ) {
$notifUser->flagCacheWithNewTalkNotification();
}
Hooks::run( 'EchoCreateNotificationComplete', array( $this ) );
- }
-
- /**
- * @param int[]|int|false $targetPageIds
- * @return Title[]
- */
- protected static function resolveTargetPages( $targetPageIds ) {
- if ( !$targetPageIds ) {
- return array();
- }
- if ( !is_array( $targetPageIds ) ) {
- $targetPageIds = array( $targetPageIds );
- }
- $result = array();
- foreach ( $targetPageIds as $targetPageId ) {
- // Make sure the target-page id is a valid id
- $title = Title::newFromID( $targetPageId );
- // Try master if there is no match
- if ( !$title ) {
- $title = Title::newFromID( $targetPageId,
Title::GAID_FOR_UPDATE );
- }
- if ( $title ) {
- $result[] = $title;
- }
- }
-
- return $result;
}
/**
diff --git a/includes/model/TargetPage.php b/includes/model/TargetPage.php
index c678947..664c11f 100644
--- a/includes/model/TargetPage.php
+++ b/includes/model/TargetPage.php
@@ -1,16 +1,11 @@
<?php
/**
- * Map a title to an echo notification so that we can mark a notification as
read
+ * Map a title to an echo event so that we can mark a notification as read
* when visiting the page. This only supports titles with ids because majority
* of notifications have page_id and searching by namespace and title is slow
*/
class EchoTargetPage extends EchoAbstractEntity {
-
- /**
- * @var User
- */
- protected $user;
/**
* @var Title|null|bool false if not initialized yet
@@ -44,20 +39,18 @@
}
/**
- * Create a EchoTargetPage instance from User, Title and EchoEvent
+ * Create a EchoTargetPage instance from Title and EchoEvent
*
- * @param User $user
* @param Title $title
* @param EchoEvent $event
* @return EchoTargetPage|null
*/
- public static function create( User $user, Title $title, EchoEvent
$event ) {
+ public static function create( Title $title, EchoEvent $event ) {
// This only support title with a page_id
- if ( $user->isAnon() || !$title->getArticleID() ) {
+ if ( !$title->getArticleID() ) {
return null;
}
$obj = new self();
- $obj->user = $user;
$obj->event = $event;
$obj->eventId = $event->getId();
$obj->eventType = $event->getType();
@@ -76,7 +69,6 @@
*/
public static function newFromRow( $row ) {
$requiredFields = array(
- 'etp_user',
'etp_page',
'etp_event'
);
@@ -86,7 +78,6 @@
}
}
$obj = new self();
- $obj->user = User::newFromId( $row->etp_user );
$obj->pageId = $row->etp_page;
$obj->eventId = $row->etp_event;
if ( isset( $row->event_type ) ) {
@@ -94,13 +85,6 @@
}
return $obj;
- }
-
- /**
- * @return User
- */
- public function getUser() {
- return $this->user;
}
/**
@@ -156,7 +140,6 @@
*/
public function toDbArray() {
return array(
- 'etp_user' => $this->user->getId(),
'etp_page' => $this->pageId,
'etp_event' => $this->eventId
);
diff --git a/maintenance/removeInvalidTargetPage.php
b/maintenance/removeInvalidTargetPage.php
deleted file mode 100644
index 76335ad..0000000
--- a/maintenance/removeInvalidTargetPage.php
+++ /dev/null
@@ -1,111 +0,0 @@
-<?php
-/**
- * Remove invalid records from echo_target_page. For instance, page has been
- * deleted or removed, notification has somehow been marked as read
- *
- * @ingroup Maintenance
- */
-require_once ( getenv( 'MW_INSTALL_PATH' ) !== false
- ? getenv( 'MW_INSTALL_PATH' ) . '/maintenance/Maintenance.php'
- : __DIR__ . '/../../../maintenance/Maintenance.php' );
-
-/**
- * Maintenance script that removes invalid target page
- *
- * @ingroup Maintenance
- */
-class RemoveInvalidTargetPage extends Maintenance {
-
- public function __construct() {
- parent::__construct();
- $this->mDescription = "Delete invalid records from
echo_target_page";
- $this->setBatchSize( 500 );
- }
-
- public function execute() {
- $dbFactory = MWEchoDbFactory::newFromDefault();
- $dbw = $dbFactory->getEchoDb( DB_MASTER );
- $dbr = $dbFactory->getEchoDb( DB_SLAVE );
-
- $count = $this->mBatchSize;
- $userId = $eventId = 0;
-
- while ( $count == $this->mBatchSize ) {
- $res = $dbr->select(
- array(
- 'echo_target_page',
- 'echo_notification'
- ),
- array(
- 'etp_page',
- 'etp_user',
- 'etp_event',
- 'notification_read_timestamp',
- 'notification_bundle_base'
- ),
- array(
- "etp_user > $userId OR ( etp_user =
$userId AND etp_event > $eventId )"
- ),
- __METHOD__,
- array(
- 'ORDER BY' => 'etp_user, etp_event',
- 'LIMIT' => $this->mBatchSize
- ),
- array(
- 'echo_notification' => array(
- 'LEFT JOIN',
- 'notification_event=etp_event
AND notification_user = etp_user'
- ),
- )
- );
- if ( !$res ) {
- $this->error( 'Could not select record from
echo_target_page.', 1 );
- }
-
- $pageIds = $titles = array();
- foreach ( $res as $row ) {
- $pageIds[$row->etp_page] = $row->etp_page;
- }
- if ( $pageIds ) {
- foreach ( Title::newFromIds( $pageIds ) as
$title ) {
- $titles[$title->getArticleID()] = true;
- }
- }
-
- // Reset the head of the iterator
- $res->rewind();
- $count = $invalidCount = 0;
- foreach ( $res as $row ) {
- if (
- // Delete if notification is read
- $row->notification_read_timestamp
- // Delete if this is no longer a base
event and
- // it's not deleted from
echo_target_page
- || $row->notification_bundle_base == '0'
- // Delete if title is no longer valid
- || !isset( $titles[$row->etp_page] )
- ) {
- $dbw->delete(
- 'echo_target_page',
- array(
- 'etp_user' =>
$row->etp_user,
- 'etp_event' =>
$row->etp_event
- ),
- __METHOD__
- );
- $invalidCount++;
- }
-
- $userId = $row->etp_user;
- $eventId = $row->etp_event;
- $count++;
- };
-
- $this->output( "Deleted $invalidCount records from
$count records\n" );
- $dbFactory->waitForSlaves();
- }
- }
-}
-
-$maintClass = 'RemoveInvalidTargetPage'; // Tells it to run the class
-require_once ( RUN_MAINTENANCE_IF_MAIN );
diff --git a/tests/browser/features/support/components/notifications.rb
b/tests/browser/features/support/components/notifications.rb
index f01ba2e..6f8d5ab 100644
--- a/tests/browser/features/support/components/notifications.rb
+++ b/tests/browser/features/support/components/notifications.rb
@@ -6,7 +6,9 @@
link(:mark_all_as_read, css:
'.mw-echo-ui-notificationsListWidget-markAllReadButton > a')
div(:popup, css: '.mw-echo-ui-notificationBadgeButtonPopupWidget-popup')
span(:title, css: '.oo-ui-popupWidget-head > .oo-ui-labelElement-label')
- div(:notifications_container, css: '.mw-echo-ui-notificationsListWidget')
+ div(
+ :notifications_container,
+ css: '.mw-echo-ui-notificationsListWidget >
.mw-echo-ui-notificationItemWidget')
def when_loaded
title_element.when_present
diff --git a/tests/phpunit/mapper/NotificationMapperTest.php
b/tests/phpunit/mapper/NotificationMapperTest.php
index 9360885..9e331dd 100644
--- a/tests/phpunit/mapper/NotificationMapperTest.php
+++ b/tests/phpunit/mapper/NotificationMapperTest.php
@@ -74,7 +74,6 @@
$tpDbResult = array(
(object)array(
- 'etp_user' => 1, // userid
'etp_page' => 7, // pageid
'etp_event' => 1, // eventid
),
diff --git a/tests/phpunit/mapper/TargetPageMapperTest.php
b/tests/phpunit/mapper/TargetPageMapperTest.php
index 32e6c12..c1d3401 100644
--- a/tests/phpunit/mapper/TargetPageMapperTest.php
+++ b/tests/phpunit/mapper/TargetPageMapperTest.php
@@ -2,34 +2,6 @@
class EchoTargetPageMapperTest extends MediaWikiTestCase {
- public function testFetchByUserPageId() {
- $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( array( 'select' => false ) ) );
- $res = $targetMapper->fetchByUserPageId( User::newFromId( 1 ),
1 );
- $this->assertFalse( $res );
-
- $dbResult = array(
- (object)array(
- 'etp_user' => 1,
- 'etp_page' => 2,
- 'etp_event' => 3
- ),
- (object)array(
- 'etp_user' => 1,
- 'etp_page' => 2,
- 'etp_event' => 7,
- )
- );
- $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( array( 'select' => $dbResult ) ) );
- $res = $targetMapper->fetchByUserPageId( User::newFromId( 1 ),
2 );
- $this->assertTrue( is_array( $res ) );
- $this->assertCount( 2, $res );
- foreach ( $res as $targetPages ) {
- $this->assertTrue( is_array( $targetPages ) );
- $this->assertCount( 1, $targetPages );
- $this->assertInstanceOf( 'EchoTargetPage', reset(
$targetPages ) );
- }
- }
-
public function provideDataTestInsert() {
return array(
array(
@@ -57,26 +29,6 @@
$target = $this->mockEchoTargetPage();
$targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( $dbResult ) );
$this->assertEquals( $result, $targetMapper->insert( $target ),
$message );
- }
-
- public function testDelete() {
- $dbResult = array( 'delete' => true );
- $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( $dbResult ) );
- $this->assertTrue( $targetMapper->delete(
$this->mockEchoTargetPage() ) );
-
- $dbResult = array( 'delete' => false );
- $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( $dbResult ) );
- $this->assertFalse( $targetMapper->delete(
$this->mockEchoTargetPage() ) );
- }
-
- public function testDeleteByUserEventOffset() {
- $dbResult = array( 'delete' => true );
- $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( $dbResult ) );
- $this->assertSame( $targetMapper->deleteByUserEventOffset(
User::newFromId( 1 ), 500 ), true );
-
- $dbResult = array( 'delete' => false );
- $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( $dbResult ) );
- $this->assertSame( $targetMapper->deleteByUserEventOffset(
User::newFromId( 1 ), 500 ), false );
}
/**
diff --git a/tests/phpunit/model/NotificationTest.php
b/tests/phpunit/model/NotificationTest.php
index 2a14121..acf7153 100644
--- a/tests/phpunit/model/NotificationTest.php
+++ b/tests/phpunit/model/NotificationTest.php
@@ -80,7 +80,6 @@
*/
protected function mockTargetPageRow() {
return array(
- 'etp_user' => 1,
'etp_page' => 2,
'etp_event' => 1
);
diff --git a/tests/phpunit/model/TargetPageTest.php
b/tests/phpunit/model/TargetPageTest.php
index b77e5b4..560c157 100644
--- a/tests/phpunit/model/TargetPageTest.php
+++ b/tests/phpunit/model/TargetPageTest.php
@@ -5,23 +5,6 @@
public function testCreate() {
$this->assertNull(
EchoTargetPage::create(
- User::newFromId( 0 ),
- $this->mockTitle( 1 ),
- $this->mockEchoEvent()
- )
- );
-
- $this->assertNull(
- EchoTargetPage::create(
- User::newFromId( 1 ),
- $this->mockTitle( 0 ),
- $this->mockEchoEvent()
- )
- );
-
- $this->assertNull(
- EchoTargetPage::create(
- User::newFromId( 0 ),
$this->mockTitle( 0 ),
$this->mockEchoEvent()
)
@@ -30,7 +13,6 @@
$this->assertInstanceOf(
'EchoTargetPage',
EchoTargetPage::create(
- User::newFromId( 1 ),
$this->mockTitle( 1 ),
$this->mockEchoEvent()
)
@@ -39,7 +21,6 @@
public function testNewFromRow() {
$row = (object)array(
- 'etp_user' => 1,
'etp_page' => 2,
'etp_event' => 3
);
@@ -54,7 +35,6 @@
*/
public function testNewFromRowWithException() {
$row = (object)array(
- 'etp_page' => 2,
'etp_event' => 3
);
$this->assertInstanceOf( 'EchoTargetPage',
EchoTargetPage::newFromRow( $row ) );
@@ -66,7 +46,11 @@
public function testToDbArray( $obj ) {
$row = $obj->toDbArray();
$this->assertTrue( is_array( $row ) );
- $this->assertArrayHasKey( 'etp_user', $row );
+
+ // Not very common to assert that a field does _not_ exist
+ // but since we are explicitly removing it, it seems to make
sense.
+ $this->assertArrayNotHasKey( 'etp_user', $row );
+
$this->assertArrayHasKey( 'etp_page', $row );
$this->assertArrayHasKey( 'etp_event', $row );
}
--
To view, visit https://gerrit.wikimedia.org/r/309051
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib57510e6b0e9202a7e035f8ea59955dca8a0b24a
Gerrit-PatchSet: 10
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Sbisson <sbis...@wikimedia.org>
Gerrit-Reviewer: Catrope <roan.katt...@gmail.com>
Gerrit-Reviewer: Sbisson <sbis...@wikimedia.org>
Gerrit-Reviewer: Springle <sprin...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
