From 3a351cfb4fab9fb9d81ecdcb2638f29c843c26be Mon Sep 17 00:00:00 2001
From: Nicholas Ray <nray@wikimedia.org>
Date: Fri, 14 May 2021 14:44:10 -0600
Subject: [PATCH] Move talk page notification data from 'user-menu' to
 'notifications'

**Note**: This change will affect the order of the yellow talk page
message notification on legacy Vector/other skins by moving it from
after the `#pt-notifications-notice` element to before the
`#pt-notifications-alert` element. This was done because the
notification is related to the list of messages that appear when the
bell icon is clicked so having it in close proximity to that icon is
hopefully more intuitive than having it next to the unrelated inbox
icon. [1]

Per T274428, we need this notification to be inside the `notifications`
array instead of inside the `user-menu` array.

Additionally:

* Per T274428, update the notifications message copy to "You have a new
Talk page message"

* Remove the `onPersonalUrls` hook method inside EchoHooks,
unregister its use as a hook in extension.json, and update its
references in Echo.

[1] T274428#7113896

Bug: T274428
Change-Id: I5ae0ec089bcf0eec1ec7ac13f60e811f54e1d8e1
---
 extension.json           |  1 -
 i18n/en.json             |  2 +-
 includes/EchoHooks.php   | 39 ++++++++++++++++-----------------------
 modules/ext.echo.init.js |  2 +-
 4 files changed, 18 insertions(+), 26 deletions(-)

diff --git a/extension.json b/extension.json
index 4e9f24773..174f50700 100644
--- a/extension.json
+++ b/extension.json
@@ -458,7 +458,6 @@
 		"SkinMinervaReplaceNotificationsBadge": "EchoHooks::onSkinMinervaReplaceNotificationsBadge",
 		"LoadExtensionSchemaUpdates": "EchoHooks::onLoadExtensionSchemaUpdates",
 		"GetPreferences": "EchoHooks::getPreferences",
-		"PersonalUrls": "EchoHooks::onPersonalUrls",
 		"BeforePageDisplay": "EchoHooks::beforePageDisplay",
 		"ResourceLoaderRegisterModules": "EchoHooks::onResourceLoaderRegisterModules",
 		"UserGroupsChanged": "EchoHooks::onUserGroupsChanged",
diff --git a/i18n/en.json b/i18n/en.json
index 59ccbca19..f5eb04ca5 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -64,7 +64,7 @@
 	"echo-pref-dont-email-read-notifications": "Don't include read notifications in summary emails",
 	"echo-learn-more": "Learn more",
 	"echo-log": "Public log",
-	"echo-new-messages": "You have new messages",
+	"echo-new-messages": "You have a new Talk page message",
 	"echo-category-title-edit-user-talk": "Talk page {{PLURAL:$1|message|messages}}",
 	"echo-category-title-article-linked": "Page {{PLURAL:$1|link|links}}",
 	"echo-category-title-reverted": "Edit {{PLURAL:$1|revert|reverts}}",
diff --git a/includes/EchoHooks.php b/includes/EchoHooks.php
index ddfe40932..8a25d26e1 100644
--- a/includes/EchoHooks.php
+++ b/includes/EchoHooks.php
@@ -1013,29 +1013,6 @@ public static function onSkinMinervaReplaceNotificationsBadge( $user, $title, &$
 		$badge = $parser->processTemplate( 'NotificationBadge', $data );
 	}
 
-	/**
-	 * Handler for PersonalUrls hook.
-	 * Marks the talk page link when the user has a new message.
-	 * @see https://www.mediawiki.org/wiki/Manual:Hooks/PersonalUrls
-	 * @param array &$personal_urls Array of URLs to append to.
-	 * @param Title &$title Title of page being visited.
-	 * @param SkinTemplate $sk
-	 */
-	public static function onPersonalUrls( &$personal_urls, &$title, $sk ) {
-		$user = $sk->getUser();
-		if ( !$user->isRegistered() ) {
-			return;
-		}
-
-		// If the user has new messages, display a talk page alert
-		if ( self::shouldDisplayTalkAlert( $user, $title )
-			&& Hooks::run( 'BeforeDisplayOrangeAlert', [ $user, $title ] )
-		) {
-			$personal_urls['mytalk']['text'] = $sk->msg( 'echo-new-messages' )->text();
-			$personal_urls['mytalk']['class'] = [ 'mw-echo-alert' ];
-		}
-	}
-
 	/**
 	 * Determine if a talk page alert should be displayed.
 	 * We need to check:
@@ -1148,6 +1125,22 @@ public static function onSkinTemplateNavigationUniversal( $skinTemplate, &$links
 			$alertLinkClasses[] = 'mw-echo-notifications-badge-long-label';
 		}
 
+		if (
+			self::shouldDisplayTalkAlert( $user, $title ) &&
+			MediaWikiServices::getInstance()
+				->getHookContainer()->run( 'BeforeDisplayOrangeAlert', [ $user, $title ] )
+		) {
+			// Move `mytalk` from `user-menu` to `notifications`.
+			$links['notifications']['mytalk'] = array_merge(
+				$links['user-menu']['mytalk'],
+				[
+					'text' => $skinTemplate->msg( 'echo-new-messages' )->text(),
+					'class' => [ 'mw-echo-alert' ]
+				]
+			);
+			unset( $links['user-menu']['mytalk'] );
+		}
+
 		$links['notifications']['notifications-alert'] = [
 			'href' => $url,
 			'text' => $alertText,
diff --git a/modules/ext.echo.init.js b/modules/ext.echo.init.js
index df94a55fc..de557f756 100644
--- a/modules/ext.echo.init.js
+++ b/modules/ext.echo.init.js
@@ -265,7 +265,7 @@ function initDesktop() {
 				if ( hasUnseenAlerts || hasUnseenMessages ) {
 					// Clicked on the flyout due to having unread notifications
 					// This is part of tracking how likely users are to click a badge with unseen notifications.
-					// The other part is the 'echo.unseen' counter, see EchoHooks::onPersonalUrls().
+					// The other part is the 'echo.unseen' counter, see EchoHooks::onSkinTemplateNavigationUniversal().
 					mw.track( 'counter.MediaWiki.echo.unseen.click' );
 				}
 			}, function () {