From 56c4b950878e782810f511e91eb182b74a6c1572 Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Wed, 19 Aug 2015 13:22:45 -0700 Subject: [PATCH] Clean up and refactor formatting system The workflow to format a notification is * Get EchoEvent, User, and Language * Get EchoEventFormatter implementation for notification type ** EchoEventFormatter returns structured data about each part of the notification (header, body, primary link, secondary link(s)) * Each output type will have a formatter class (e.g. EchoSpecialNotificationsFormatter, EchoPlainTextEmailFormatter) which takes a EchoEventPresentationModel and generates whatever it wants (HTML, plain-text email, etc). Included is an example conversion of the user-rights and mention formatters. The previous infrastructure will remain in place for backwards compatability until other extensions can be updated. Bug: T107823 Change-Id: I4397872a7ec062148dfcb066ddd8ab83f40486ac --- Echo.php | 3 + autoload.php | 5 + i18n/en.json | 4 +- i18n/qqq.json | 4 +- includes/DataOutputFormatter.php | 25 ++- includes/formatters/EchoEventFormatter.php | 25 +++ includes/formatters/EchoFlyoutFormatter.php | 69 +++++++ .../formatters/EventPresentationModel.php | 173 ++++++++++++++++++ .../formatters/MentionPresentationModel.php | 106 +++++++++++ includes/formatters/UserRightsFormatter.php | 1 + .../UserRightsPresentationModel.php | 54 ++++++ 11 files changed, 466 insertions(+), 3 deletions(-) create mode 100644 includes/formatters/EchoEventFormatter.php create mode 100644 includes/formatters/EchoFlyoutFormatter.php create mode 100644 includes/formatters/EventPresentationModel.php create mode 100644 includes/formatters/MentionPresentationModel.php create mode 100644 includes/formatters/UserRightsPresentationModel.php diff --git a/Echo.php b/Echo.php index 009332466..a951d2b6b 100644 --- a/Echo.php +++ b/Echo.php @@ -381,6 +381,7 @@ $wgEchoNotifications = array( 'category' => 'mention', 'group' => 'interactive', 'section' => 'alert', + 'presentation-model' => 'EchoMentionPresentationModel', 'formatter-class' => 'EchoMentionFormatter', 'title-message' => 'notification-mention', 'title-params' => array( 'agent', 'subject-anchor', 'title', 'section-title', 'main-title-text' ), @@ -400,6 +401,8 @@ $wgEchoNotifications = array( 'category' => 'user-rights', 'group' => 'neutral', 'section' => 'alert', + 'presentation-model' => 'EchoUserRightsPresentationModel', + // Legacy formatting system 'formatter-class' => 'EchoUserRightsFormatter', 'title-message' => 'notification-user-rights', 'title-params' => array( 'agent', 'user-rights-list' ), diff --git a/autoload.php b/autoload.php index 41d95f40a..9b6775c41 100644 --- a/autoload.php +++ b/autoload.php @@ -45,16 +45,20 @@ $wgAutoloadClasses += array( 'EchoEmailMode' => __DIR__ . '/includes/EmailFormatter.php', 'EchoEmailSingle' => __DIR__ . '/includes/EmailFormatter.php', 'EchoEvent' => __DIR__ . '/includes/model/Event.php', + 'EchoEventFormatter' => __DIR__ . '/includes/formatters/EchoEventFormatter.php', 'EchoEventMapper' => __DIR__ . '/includes/mapper/EventMapper.php', 'EchoEventMapperTest' => __DIR__ . '/tests/phpunit/mapper/EventMapperTest.php', + 'EchoEventPresentationModel' => __DIR__ . '/includes/formatters/EventPresentationModel.php', 'EchoExecuteFirstArgumentStub' => __DIR__ . '/tests/phpunit/mapper/NotificationMapperTest.php', 'EchoFilteredSequentialIterator' => __DIR__ . '/includes/iterator/FilteredSequentialIterator.php', + 'EchoFlyoutFormatter' => __DIR__ . '/includes/formatters/EchoFlyoutFormatter.php', 'EchoHTMLEmailDecorator' => __DIR__ . '/includes/EmailFormatter.php', 'EchoHTMLEmailFormatter' => __DIR__ . '/includes/EmailFormatter.php', 'EchoHooks' => __DIR__ . '/Hooks.php', 'EchoIteratorDecorator' => __DIR__ . '/includes/iterator/IteratorDecorator.php', 'EchoLocalCache' => __DIR__ . '/includes/cache/LocalCache.php', 'EchoMentionFormatter' => __DIR__ . '/includes/formatters/MentionFormatter.php', + 'EchoMentionPresentationModel' => __DIR__ . '/includes/formatters/MentionPresentationModel.php', 'EchoMultipleIterator' => __DIR__ . '/includes/iterator/MultipleIterator.php', 'EchoNotRecursiveIterator' => __DIR__ . '/includes/iterator/NotRecursiveIterator.php', 'EchoNotification' => __DIR__ . '/includes/model/Notification.php', @@ -87,6 +91,7 @@ $wgAutoloadClasses += array( 'EchoUserNotificationGateway' => __DIR__ . '/includes/gateway/UserNotificationGateway.php', 'EchoUserNotificationGatewayTest' => __DIR__ . '/tests/phpunit/gateway/UserNotificationGatewayTest.php', 'EchoUserRightsFormatter' => __DIR__ . '/includes/formatters/UserRightsFormatter.php', + 'EchoUserRightsPresentationModel' => __DIR__ . '/includes/formatters/UserRightsPresentationModel.php', 'FilteredSequentialIteratorTest' => __DIR__ . '/tests/phpunit/iterator/FilteredSequentialIteratorTest.php', 'MWEchoDbFactory' => __DIR__ . '/includes/EchoDbFactory.php', 'MWEchoDbFactoryTest' => __DIR__ . '/tests/phpunit/EchoDbFactoryTest.php', diff --git a/i18n/en.json b/i18n/en.json index f4069d8c6..ff0230fed 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -69,10 +69,12 @@ "notification-add-comment-yours2": "[[User:$1|$1]] {{GENDER:$1|commented}} on \"[[$3#$2|$2]]\" on your talk page.", "notification-mention": "[[User:$1|$1]] {{GENDER:$1|mentioned}} you on the $5 talk page in \"[[:$3#$2|$4]]\".", "notification-mention-flyout": "$1 {{GENDER:$1|mentioned}} you on the $5 talk page in \"[[:$3#$2|$4]]\".", + "notification-header-mention": "$1 {{GENDER:$2|mentioned}} you on the $3 talk page in \"$4\".", "notification-mention-nosection": "[[User:$1|$1]] {{GENDER:$1|mentioned}} you on the [[:$3|$2 talk page]].", "notification-mention-nosection-flyout": "$1 {{GENDER:$1|mentioned}} you on the [[:$3|$2 talk page]].", + "notification-header-mention-nosection": "$1 {{GENDER:$2|mentioned}} you on the [[:$4|$3 talk page]].", "notification-user-rights": "Your user rights [[Special:Log/rights/$1|were {{GENDER:$1|changed}}]] by [[User:$1|$1]]. $2. [[Special:ListGroupRights|Learn more]]", - "notification-user-rights-flyout": "Your user rights were {{GENDER:$1|changed}} by $1. $2. [[Special:ListGroupRights|Learn more]]", + "notification-header-user-rights": "Your user rights were {{GENDER:$2|changed}} by $1. $3. [[Special:ListGroupRights|Learn more]]", "notification-user-rights-add": "You are now a member of {{PLURAL:$2|this group|these groups}}: $1", "notification-user-rights-remove": "You are no longer a member of {{PLURAL:$2|this group|these groups}}: $1", "notification-new-user": "Welcome to {{SITENAME}}, $1! We're glad you're here.", diff --git a/i18n/qqq.json b/i18n/qqq.json index 1e68f53d2..6b2e9893c 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -90,10 +90,12 @@ "notification-add-comment-yours2": "Parameters:\n* $1 - a username, plain text; can be used for GENDER\n* $2 - discussion name\n* $3 - link to user talk page\nSee also:\n* {{msg-mw|Notification-add-comment2}}", "notification-mention": "Format for displaying notifications of a comment in a specific section including a link to another user's user page.\n\nParameters:\n* $1 - the username of the person who edited, plain text. Can be used for GENDER\n* $2 - the section title of the discussion\n* $3 - the page title of the discussion\n* $4 - the raw section title text\n* $5 - the title text without namespace (a page title in any namespace)", "notification-mention-flyout": "Flyout-specific format for displaying notifications of a comment in a specific section.\nParameters:\n* $1 - the username of the person who mentioned you, plain text. Can be used for GENDER.\n* $2 - the section title of the discussion\n* $3 - the page title of the discussion\n* $4 - the raw section title text\n* $5 - the title text without namespace (a page title in any namespace)", + "notification-header-mention": "Header text for a notification when you are mentioned by another user. $1 is that user's name (not suitable for GENDER). $2 is the user's name for use in GENDER. $3 is the name of the page without namespace they were mentioned in. $4 is a link to the section they were mentioned in.", + "notification-header-mention-nosection": "Header text for a notification when you are mentioned by another user, but not in a section of a page. $1 is that user's name (not suitable for GENDER). $2 is the user's name for use in GENDER. $3 is the name of the page without namespace they were mentioned in. $4 is the full page name, for use in a link.", "notification-mention-nosection": "Format for displaying notifications of a comment including a link to another user's user page. Parameters:\n* $1 - the username of the person who edited, plain text. Can be used for GENDER\n* $2 - the title text without namespace (a page title in any namespace)\n* $3 - the page title of the discussion", "notification-mention-nosection-flyout": "Flyout-specific format for displaying notifications of a comment.\nParameters:\n* $1 - the username of the person who edited, plain text. Can be used for GENDER\n* $2 - the title text without namespace (a page title in any namespace)\n* $3 - the page title of the discussion", "notification-user-rights": "Format for displaying notifications of a user right change in notification page.\n\nParameters:\n* $1 - the username of the person who made the user right change. Can be used for GENDER support.\n* $2 - a semicolon separated list of {{msg-mw|Notification-user-rights-add}}, {{msg-mw|Notification-user-rights-remove}}", - "notification-user-rights-flyout": "Format for displaying notifications of a user right change in notification flyout. Parameters:\n* $1 - the username of the person who made the user right change. Can be used for GENDER support\n* $2 - a semicolon separated list of {{msg-mw|notification-user-rights-add}}, {{msg-mw|notification-user-rights-remove}}", + "notification-header-user-rights": "Format for displaying notifications of a user right change in notification flyout. Parameters:\n* $1 - the username of the person who made the user right change, formatted for display. Cannot be used for GENDER\n* $2 - the raw username of the person who made the user rights change, can be used for GENDER support\n* $3 - a semicolon separated list of {{msg-mw|notification-user-rights-add}}, {{msg-mw|notification-user-rights-remove}}", "notification-user-rights-add": "Message indicating that a user was added to a user group. Parameters:\n* $1 - a comma separated list of user group names\n* $2 - the number of user groups, this is used for PLURAL support\nSee also:\n* {{msg-mw|Notification-user-rights-remove}}", "notification-user-rights-remove": "Message indicating that a user was removed from a user group. Parameters:\n* $1 - a comma separated list of user group names\n* $2 - the number of user groups, this is used for PLURAL support\nSee also:\n* {{msg-mw|Notification-user-rights-add}}", "notification-new-user": "Text of the welcome notification. Parameters:\n* $1 - the name of the new user\nSee also:\n* {{msg-mw|Guidedtour-tour-gettingstarted-start-title}}", diff --git a/includes/DataOutputFormatter.php b/includes/DataOutputFormatter.php index b2e3baa5f..01d62d1e9 100644 --- a/includes/DataOutputFormatter.php +++ b/includes/DataOutputFormatter.php @@ -5,6 +5,13 @@ */ class EchoDataOutputFormatter { + /** + * @var array type => class + */ + static $formatters = array( + 'flyout' => 'EchoFlyoutFormatter' + ); + /** * Format a notification for a user in the format specified * @@ -106,12 +113,28 @@ class EchoDataOutputFormatter { } if ( $format ) { - $output['*'] = EchoNotificationController::formatNotification( $event, $user, $format ); + $output['*'] = self::formatNotification( $event, $user, $format ); } return $output; } + protected static function formatNotification( EchoEvent $event, User $user, $format ) { + global $wgLang; + if ( isset( self::$formatters[$format] ) + && EchoEventPresentationModel::supportsPresentationModel( $event->getType() ) + ) { + // FIXME don't use $wgLang. It's ok because this is only used for the API or Special page, and not + // emails yet. + /** @var EchoEventFormatter $formatter */ + $formatter = new self::$formatters[$format]( $user, $wgLang ); + return $formatter->format( $event ); + } else { + // Legacy b/c + return EchoNotificationController::formatNotification( $event, $user, $format ); + } + } + /** * Get the date header in user's format, 'May 10' or '10 May', depending * on user's date format preference diff --git a/includes/formatters/EchoEventFormatter.php b/includes/formatters/EchoEventFormatter.php new file mode 100644 index 000000000..d330f326d --- /dev/null +++ b/includes/formatters/EchoEventFormatter.php @@ -0,0 +1,25 @@ +user = $user; + $this->language = $language; + } + + /** + * @param EchoEvent $event + * @return string HTML + */ + abstract public function format( EchoEvent $event ); +} diff --git a/includes/formatters/EchoFlyoutFormatter.php b/includes/formatters/EchoFlyoutFormatter.php new file mode 100644 index 000000000..dae1c98b9 --- /dev/null +++ b/includes/formatters/EchoFlyoutFormatter.php @@ -0,0 +1,69 @@ +language, $this->user ); + + $icon = Html::element( + 'img', + array( + 'class' => 'mw-echo-icon', + 'src' => $this->getIconURL( $model ), + ) + ); + + $html = Xml::tags( + 'div', + array( 'class' => 'mw-echo-title' ), + $model->getHeaderMessage()->parse() + ) . "\n"; + + //@todo body text + + $ts = $this->language->getHumanTimestamp( + new MWTimestamp( $event->getTimestamp() ), + null, + $this->user + ); + + $footerItems = array( $ts ); + foreach ( $model->getSecondaryLinks() as $target => $text ) { + $footerItems[] = Html::element( 'a', array( 'href' => $target ), $text ); + } + $html .= Xml::tags( + 'div', + array( 'class' => 'mw-echo-notification-footer' ), + $this->language->pipeList( $footerItems ) + ) . "\n"; + + // Add the primary link afterwards??? + list( $primaryUrl, $primaryText ) = $model->getPrimaryLink(); + $html .= Html::element( + 'a', + array( 'class' => 'mw-echo-notification-primary-link', 'href' => $primaryUrl ), + $primaryText + ) . "\n"; + + // Wrap everything in mw-echo-content class + $html = Xml::tags( 'div', array( 'class' => 'mw-echo-content' ), $html ); + + // And then add the icon in front and wrap with mw-echo-state class. + $html = Xml::tags( 'div', array( 'class' => 'mw-echo-state' ), $icon . $html ); + + return $html; + } + + private function getIconURL( EchoEventPresentationModel $model ) { + return EchoNotificationFormatter::getIconUrl( + $model->getIconType(), + $this->language->getDir() + ); + } +} diff --git a/includes/formatters/EventPresentationModel.php b/includes/formatters/EventPresentationModel.php new file mode 100644 index 000000000..b5bacac58 --- /dev/null +++ b/includes/formatters/EventPresentationModel.php @@ -0,0 +1,173 @@ +event = $event; + $this->type = $event->getType(); + $this->language = wfGetLangObj( $language ); + $this->user = $user; + } + + /** + * Convenience function to detect whether the event type + * has been updated to use the presentation model system + * + * @param string $type event type + * @return bool + */ + public static function supportsPresentationModel( $type ) { + global $wgEchoNotifications; + return isset( $wgEchoNotifications[$type]['presentation-model'] ); + } + + /** + * @param EchoEvent $event + * @param Language|string $language + * @param User $user + * @return EchoEventPresentationModel + */ + public static function factory( EchoEvent $event, $language, User $user ) { + global $wgEchoNotifications; + // @todo don't depend upon globals + + $class = $wgEchoNotifications[$event->getType()]['presentation-model']; + return new $class( $event, $language, $user ); + } + + /** + * Equivalent to IContextSource::msg for the current + * language + * + * @return Message + */ + protected function msg( /* ,,, */ ) { + /** + * @var Message $msg + */ + $msg = call_user_func_array( 'wfMessage', func_get_args() ); + $msg->inLanguage( $this->language ); + + return $msg; + } + + /** + * @return string The symbolic icon name as defined in $wgEchoNotificationIcons + */ + abstract public function getIconType(); + + /** + * Helper for EchoEvent::userCan + * + * @param int $type Revision::DELETED_* constant + * @return bool + */ + final protected function userCan( $type ) { + return $this->event->userCan( $type, $this->user ); + } + + /** + * @return array|bool ['wikitext to display', 'username for GENDER'], false if no agent + * + * We have to display wikitext so we can add CSS classes for revision deleted user. + * The goal of this function is for callers not to worry about whether + * the user is visible or not. + * @par Example: + * @code + * list( $formattedName, $genderName ) = $this->getAgentForOutput(); + * $msg->params( $formattedName, $genderName ); + * @endcode + */ + final protected function getAgentForOutput() { + $agent = $this->event->getAgent(); + if ( !$agent ) { + return false; + } + + if ( $this->userCan( Revision::DELETED_USER ) ) { + // Not deleted + return array( $agent->getName(), $agent->getName() ); + } else { + // Deleted/hidden + $msg = $this->msg( 'rev-deleted-user' )->plain(); + // HACK: Pass an invalid username to GENDER to force the default + return array( '' . $msg . '', '[]' ); + } + } + + /** + * @return string Message key that will be used in getHeaderMessage + */ + protected function getHeaderMessageKey() { + return "notification-header-{$this->type}"; + } + + /** + * Get a message object and add the performer's name as + * a parameter. It is expected that subclasses will override + * this. + * + * @return Message + */ + public function getHeaderMessage() { + $msg = $this->msg( $this->getHeaderMessageKey() ); + list( $formattedName, $genderName ) = $this->getAgentForOutput(); + $msg->params( $formattedName, $genderName ); + + return $msg; + } + + /** + * Get the body text, false if notification has no body + * + * @return bool|Message + */ + public function getBodyText() { + return false; + } + + /** + * Possibly-relative URL to the primary link for this + * + * @return [array URL, link text (non-escaped)] + */ + abstract public function getPrimaryLink(); + + /** + * Possibly-relative URLs to the secondary links + * + * @return array URL => link text + */ + public function getSecondaryLinks() { + return array(); + } +} diff --git a/includes/formatters/MentionPresentationModel.php b/includes/formatters/MentionPresentationModel.php new file mode 100644 index 000000000..5cdfc3a9a --- /dev/null +++ b/includes/formatters/MentionPresentationModel.php @@ -0,0 +1,106 @@ +sectionTitle !== null ) { + return $this->sectionTitle; + } + $sectionTitle = $this->event->getExtraParam( 'section-title' ); + if ( !$sectionTitle ) { + $this->sectionTitle = false; + return false; + } + // Check permissions + if ( !$this->userCan( Revision::DELETED_TEXT ) ) { + $this->sectionTitle = false; + return false; + } + + $this->sectionTitle = $sectionTitle; + return $this->sectionTitle; + } + + /** + * Override to switch the message key to -nosection + * if no section title was detected + * + * @return string + */ + protected function getHeaderMessageKey() { + // Messages used: + // notification-header-mention + // notification-header-mention-nosection + $key = parent::getHeaderMessageKey(); + if ( !$this->getSection() ) { + $key .= '-nosection'; + } + + return $key; + } + + public function getHeaderMessage() { + $msg = parent::getHeaderMessage(); + // @fixme this message should not say "xx talk page" + $msg->params( $this->event->getTitle()->getText() ); + $section = $this->getSection(); + $sectionTitle = $this->getTitleWithSection(); + if ( $section ) { + $msg->rawParams( + Linker::link( + $sectionTitle, + htmlspecialchars( EchoDiscussionParser::getTextSnippet( + $section, + $this->language, + 30 + ) ) + ) + ); + } else { + // For the -nosection message + $msg->params( $sectionTitle->getPrefixedText() ); + } + + return $msg; + } + + /** + * @return Title + */ + private function getTitleWithSection() { + $title = $this->event->getTitle(); + $section = $this->getSection(); + if ( $section ) { + $title = Title::makeTitle( + $title->getNamespace(), + $title->getDBkey(), + $section + ); + } + + return $title; + } + + public function getPrimaryLink() { + return array( + $this->getTitleWithSection()->getLocalURL(), + $this->msg( 'notification-link-text-view-mention' )->text() + ); + } + + public function getSecondaryLinks() { + $url = $this->event->getTitle()->getLocalURL( array( + 'oldid' => 'prev', + 'diff' => $this->event->getExtraParam( 'revid' ) + ) ); + return array( + $url => $this->msg( 'notification-link-text-view-changes' )->text() + ); + } +} diff --git a/includes/formatters/UserRightsFormatter.php b/includes/formatters/UserRightsFormatter.php index 194da3126..efc3a2d4b 100644 --- a/includes/formatters/UserRightsFormatter.php +++ b/includes/formatters/UserRightsFormatter.php @@ -70,3 +70,4 @@ class EchoUserRightsFormatter extends EchoBasicFormatter { return array( $target, $query ); } } + diff --git a/includes/formatters/UserRightsPresentationModel.php b/includes/formatters/UserRightsPresentationModel.php new file mode 100644 index 000000000..37c73806a --- /dev/null +++ b/includes/formatters/UserRightsPresentationModel.php @@ -0,0 +1,54 @@ +params( $this->getChangedGroups() ); + + return $msg; + } + + /** + * @return string + */ + private function getChangedGroups() { + $list = array(); + $extra = $this->event->getExtra(); + foreach ( array( 'add', 'remove' ) as $action ) { + if ( isset( $extra[$action] ) && $extra[$action] ) { + + // Get the localized group names, bug 55338 + $groups = array(); + foreach( $extra[$action] as $group ) { + $msg = $this->msg( 'group-' . $group ); + $groups[] = $msg->isBlank() ? $group : $msg->text(); + } + + // Messages that can be used here: + // * notification-user-rights-add + // * notification-user-rights-remove + $list[] = $this->msg( 'notification-user-rights-' . $action ) + ->params( $this->language->commaList( $groups ), count( $groups ) ) + ->text(); + } + } + + return $this->language->semicolonList( $list ); + } + + public function getPrimaryLink() { + return array( + SpecialPage::getTitleFor( 'Listgrouprights' )->getLocalURL(), + $this->msg( 'echo-learn-more' )->text() + ); + } +}