diff --git a/modules/logger/mw.echo.Logger.js b/modules/logger/mw.echo.Logger.js index 0eaf01c11..84261cf24 100644 --- a/modules/logger/mw.echo.Logger.js +++ b/modules/logger/mw.echo.Logger.js @@ -88,7 +88,7 @@ * @param {int} [eventId] Notification event id * @param {string} [eventType] notification type * @param {boolean} [mobile] True if interaction was on a mobile device - * @param {string} [notifWiki] Wiki the notification came from + * @param {string} [notifWiki] Foreign wiki the notification came from */ mw.echo.Logger.prototype.logInteraction = function ( action, context, eventId, eventType, mobile, notifWiki ) { var myEvt; @@ -115,7 +115,7 @@ myEvt.mobile = mobile; } - if ( notifWiki && notifWiki !== mw.config.get( 'wgDBname' ) ) { + if ( notifWiki && notifWiki !== mw.config.get( 'wgDBname' ) && notifWiki !== 'local' ) { myEvt.notifWiki = notifWiki; } @@ -130,17 +130,22 @@ * @param {string} type Notification type; 'alert' or 'message' * @param {number[]} notificationIds Array of notification ids * @param {string} context 'flyout'/'archive' or undefined for the badge - * @param {boolean} [mobile] True if interaction was on a mobile device + * @param {string} [notifWiki='local'] Foreign wiki the notifications came from + * @param {boolean} [mobile=false] True if interaction was on a mobile device */ - mw.echo.Logger.prototype.logNotificationImpressions = function ( type, notificationIds, context, mobile ) { - var i, len; + mw.echo.Logger.prototype.logNotificationImpressions = function ( type, notificationIds, context, notifWiki, mobile ) { + var i, len, key; for ( i = 0, len = notificationIds.length; i < len; i++ ) { - if ( !this.notificationsIdCache[ notificationIds[ i ] ] ) { + key = notifWiki && notifWiki !== mw.config.get( 'wgDBname' ) && notifWiki !== 'local' ? + notificationIds[ i ] + '-' + notifWiki : + notificationIds[ i ]; + + if ( !this.notificationsIdCache[ key ] ) { // Log notification impression - this.logInteraction( 'notification-impression', context, notificationIds[ i ], type, mobile ); + this.logInteraction( 'notification-impression', context, notificationIds[ i ], type, mobile, notifWiki ); // Cache - this.notificationsIdCache[ notificationIds[ i ] ] = true; + this.notificationsIdCache[ key ] = true; } } }; diff --git a/modules/ooui/mw.echo.ui.NotificationBadgeWidget.js b/modules/ooui/mw.echo.ui.NotificationBadgeWidget.js index 4c50439e7..d78a798ea 100644 --- a/modules/ooui/mw.echo.ui.NotificationBadgeWidget.js +++ b/modules/ooui/mw.echo.ui.NotificationBadgeWidget.js @@ -301,9 +301,6 @@ // the case where the promise is already underway. this.notificationsModel.fetchNotifications() .then( function () { - var i, - items = widget.notificationsWidget.getItems(); - if ( widget.popup.isVisible() ) { widget.popup.clip(); @@ -313,20 +310,6 @@ if ( widget.markReadWhenSeen ) { widget.notificationsModel.autoMarkAllRead(); } - - // Log impressions - // TODO: Only log the impressions of notifications that - // are actually visible - for ( i = 0; i < items.length; i++ ) { - if ( items[ i ].getModel ) { - mw.echo.logger.logInteraction( - mw.echo.Logger.static.actions.notificationImpression, - mw.echo.Logger.static.context.popup, - items[ i ].getModel().getId(), - items[ i ].getModel().getCategory() - ); - } - } } widget.emit( 'finishLoading' ); diff --git a/modules/ooui/mw.echo.ui.NotificationGroupItemWidget.js b/modules/ooui/mw.echo.ui.NotificationGroupItemWidget.js index 68509f97a..b277ee240 100644 --- a/modules/ooui/mw.echo.ui.NotificationGroupItemWidget.js +++ b/modules/ooui/mw.echo.ui.NotificationGroupItemWidget.js @@ -125,26 +125,8 @@ // Query all sources this.model.fetchAllNotificationsInGroups() - .then( function ( /* Groups */ ) { - var source, items, i, - models = widget.model.getSubModels(); - + .then( function () { widget.popPending(); - - // Log impressions of all items from each group - for ( source in models ) { - items = models[ source ].getItems(); - for ( i = 0; i < items.length; i++ ) { - mw.echo.logger.logInteraction( - mw.echo.Logger.static.actions.notificationImpression, - mw.echo.Logger.static.context.popup, - items[ i ].getId(), - items[ i ].getCategory(), - false, - source - ); - } - } } ); // Log the expand action diff --git a/modules/ooui/mw.echo.ui.NotificationsWidget.js b/modules/ooui/mw.echo.ui.NotificationsWidget.js index b34f13698..41b5b2c5d 100644 --- a/modules/ooui/mw.echo.ui.NotificationsWidget.js +++ b/modules/ooui/mw.echo.ui.NotificationsWidget.js @@ -86,7 +86,12 @@ if ( isSuccess ) { // Log impressions - mw.echo.logger.logNotificationImpressions( this.type, result.ids, mw.echo.Logger.static.context.popup ); + mw.echo.logger.logNotificationImpressions( + undefined, // type: we don't know + result.ids, + mw.echo.Logger.static.context.popup, + this.getModel().getSource() + ); } };