Bsitu has uploaded a new change for review.
https://gerrit.wikimedia.org/r/152203
Change subject: Only auto mark notification read if it doesn't have targe page
......................................................................
Only auto mark notification read if it doesn't have targe page
@Todo - the javascript will need to apply to mobile as well
User wil be able to manually mark individual notification as read,
this is coming soon
Change-Id: I9bd71d59391189d5d761ab5f1c84af0bc3554be0
---
M includes/DataOutputFormatter.php
M includes/mapper/NotificationMapper.php
M includes/mapper/TargetPageMapper.php
M model/Notification.php
M modules/overlay/ext.echo.overlay.js
M special/SpecialNotifications.php
M tests/includes/mapper/TargetPageMapperTest.php
7 files changed, 93 insertions(+), 3 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Echo
refs/changes/03/152203/1
diff --git a/includes/DataOutputFormatter.php b/includes/DataOutputFormatter.php
index e3161e4..191ca07 100644
--- a/includes/DataOutputFormatter.php
+++ b/includes/DataOutputFormatter.php
@@ -83,6 +83,15 @@
$output['read'] = $notification->getReadTimestamp();
}
+ // This is only meant for unread notifications, if a
notification has a target
+ // page, then it shouldn't be auto marked as read unless the
user visits
+ // the target page or a user marks it as read manully ( coming
soon )
+ if ( $notification->getTargetPage() ) {
+ $output['targetpage'] =
$notification->getTargetPage()->getPageId();
+ } else {
+ $output['targetpage'] = '';
+ }
+
if ( $format ) {
$output['*'] =
EchoNotificationController::formatNotification( $event, $user, $format );
}
diff --git a/includes/mapper/NotificationMapper.php
b/includes/mapper/NotificationMapper.php
index d179790..db39e4b 100644
--- a/includes/mapper/NotificationMapper.php
+++ b/includes/mapper/NotificationMapper.php
@@ -110,7 +110,7 @@
}
$res = $dbr->select(
- array( 'echo_notification', 'echo_event' ),
+ array( 'echo_notification', 'echo_event',
'echo_target_page' ),
'*',
$conds,
__METHOD__,
@@ -120,6 +120,7 @@
),
array(
'echo_event' => array( 'LEFT JOIN',
'notification_event=event_id' ),
+ 'echo_target_page' => array( 'LEFT JOIN',
'notification_event=etp_event AND notification_user=etp_user' ),
)
);
diff --git a/includes/mapper/TargetPageMapper.php
b/includes/mapper/TargetPageMapper.php
index 42f068b..196629b 100644
--- a/includes/mapper/TargetPageMapper.php
+++ b/includes/mapper/TargetPageMapper.php
@@ -83,6 +83,35 @@
);
return $res;
}
+
+ /**
+ * Fetch TargetPage records by user and set of eventIds
+ *
+ * @param User $user
+ * @param int[] $eventIds
+ */
+ public function fetchByUserEventIds( User $user, array $eventIds ) {
+ $dbr = $this->dbFactory->getEchoDb( DB_SLAVE );
+
+ $res = $dbr->select(
+ array( 'echo_target_page' ),
+ array( '*' ),
+ array(
+ 'etp_user' => $user->getId(),
+ 'etp_evemt' => $eventIds
+ ),
+ __METHOD__
+ );
+ if ( $res ) {
+ $targetPage = array();
+ foreach ( $res as $row ) {
+ $targetPage[] = EchoTargetPage::newFromRow(
$row );
+ }
+ return $targetPage;
+ } else {
+ return false;
+ }
+ }
}
diff --git a/model/Notification.php b/model/Notification.php
index f617e40..010d360 100644
--- a/model/Notification.php
+++ b/model/Notification.php
@@ -13,6 +13,12 @@
protected $event;
/**
+ * The target page object for the notification if there is one
+ * @var EchoTargetPage|null
+ */
+ protected $targetPage;
+
+ /**
* @var string
*/
protected $timestamp;
@@ -137,6 +143,10 @@
} else {
$notification->event = EchoEvent::newFromID(
$row->notification_event );
}
+
+ if ( property_exists( $row, 'etp_event' ) && $row->etp_event ) {
+ $notification->targetPage = EchoTargetPage::newFromRow(
$row );
+ }
$notification->user = User::newFromId( $row->notification_user
);
$notification->timestamp = $row->notification_timestamp;
$notification->readTimestamp =
$row->notification_read_timestamp;
@@ -217,4 +227,12 @@
public function getBundleDisplayHash() {
return $this->bundleDisplayHash;
}
+
+ /**
+ * Getter method
+ * @return EchoTargetPage|null
+ */
+ public function getTargetPage() {
+ return $this->targetPage;
+ }
}
diff --git a/modules/overlay/ext.echo.overlay.js
b/modules/overlay/ext.echo.overlay.js
index 96fbd4c..0ac76f7 100644
--- a/modules/overlay/ext.echo.overlay.js
+++ b/modules/overlay/ext.echo.overlay.js
@@ -86,7 +86,11 @@
if ( !data.read ) {
$li.addClass( 'mw-echo-unread'
);
- unread.push( id );
+ // If the notification has a
target page, it should not
+ // be auto makred as read
+ if ( !data.targetpage ) {
+ unread.push( id );
+ }
}
if ( !data['*'] ) {
diff --git a/special/SpecialNotifications.php b/special/SpecialNotifications.php
index 69fdd81..a2f5a88 100644
--- a/special/SpecialNotifications.php
+++ b/special/SpecialNotifications.php
@@ -75,7 +75,9 @@
$class = 'mw-echo-notification';
if ( !isset( $row['read'] ) ) {
$class .= ' mw-echo-unread';
- $unread[] = $row['id'];
+ if ( !$row['targetpage'] ) {
+ $unread[] = $row['id'];
+ }
}
if ( !$row['*'] ) {
diff --git a/tests/includes/mapper/TargetPageMapperTest.php
b/tests/includes/mapper/TargetPageMapperTest.php
index 0ed53fa..75859da 100644
--- a/tests/includes/mapper/TargetPageMapperTest.php
+++ b/tests/includes/mapper/TargetPageMapperTest.php
@@ -22,11 +22,38 @@
$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 $row ) {
$this->assertInstanceOf( 'EchoTargetPage', $row );
}
}
+ public function testFetchByUserEventIds() {
+ $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( array ( 'select' => false ) ) );
+ $res = $targetMapper->fetchByUserEventIds( User::newFromId( 1
), array( 1, 2, 3 ) );
+ $this->assertFalse( $res );
+
+ $dbResult = array (
+ (object)array (
+ 'etp_user' => 1,
+ 'etp_page' => 2,
+ 'etp_event' => 2
+ ),
+ (object)array (
+ 'etp_user' => 1,
+ 'etp_page' => 2,
+ 'etp_event' => 3,
+ )
+ );
+ $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( array ( 'select' => $dbResult ) ) );
+ $res = $targetMapper->fetchByUserEventIds( User::newFromId( 1
), array( 1, 2, 3 ) );
+ $this->assertTrue( is_array( $res ) );
+ $this->assertCount( 2, $res );
+ foreach ( $res as $row ) {
+ $this->assertInstanceOf( 'EchoTargetPage', $row );
+ }
+ }
+
public function provideDataTestInsert() {
return array (
array (
--
To view, visit https://gerrit.wikimedia.org/r/152203
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9bd71d59391189d5d761ab5f1c84af0bc3554be0
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Bsitu <bs...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
