From 335fc1fab001134d4b805d46fdc683fc7d376649 Mon Sep 17 00:00:00 2001
From: Kaldari <rkaldari@wikimedia.org>
Date: Fri, 15 Feb 2013 15:34:47 -0800
Subject: [PATCH] Getting rid of confusing abstraction regarding formatter
 classes

Specifying the class directly is much less confusing and fragile.
Plus we have way too many things named 'type'.
Also changing MWEchoArticleLinkedFormatter to EchoArticleLinkedFormatter
to match the convention of the other formatter classes.
Also making EchoBasicFormatter the default, so that the Hello World
case is easier to implement.

Change-Id: Ibd9b15008d37ad815e466ab81ba9a5b668ee2791
---
 Echo.php                              | 16 ++++++++--------
 formatters/ArticleLinkedFormatter.php |  2 +-
 formatters/NotificationFormatter.php  | 20 +++++---------------
 3 files changed, 14 insertions(+), 24 deletions(-)

diff --git a/Echo.php b/Echo.php
index 01bb56f1d..c0795f415 100644
--- a/Echo.php
+++ b/Echo.php
@@ -56,7 +56,7 @@ $wgAutoloadClasses['EchoNotificationFormatter'] = $dir . 'formatters/Notificatio
 $wgAutoloadClasses['EchoBasicFormatter'] = $dir . 'formatters/BasicFormatter.php';
 $wgAutoloadClasses['EchoEditFormatter'] = $dir . 'formatters/EditFormatter.php';
 $wgAutoloadClasses['EchoCommentFormatter'] = $dir . 'formatters/CommentFormatter.php';
-$wgAutoloadClasses['MWEchoArticleLinkedFormatter'] = $dir . 'formatters/ArticleLinkedFormatter.php';
+$wgAutoloadClasses['EchoArticleLinkedFormatter'] = $dir . 'formatters/ArticleLinkedFormatter.php';
 
 // Internal stuff
 $wgAutoloadClasses['EchoNotifier'] = $dir . 'Notifier.php';
@@ -259,17 +259,17 @@ $wgEchoEventDetails = array(
 	),
 );
 
-// Definitions of the notification event types built into Echo
+// Definitions of the notification event types built into Echo.
+// If formatter-class isn't specified, defaults to EchoBasicFormatter.
 $wgEchoNotificationFormatters = array(
 	'welcome' => array(
-		'type' => 'system',
 		'title-message' => 'notification-new-user',
 		'title-params' => array( 'agent' ),
 		'payload' => array( 'welcome' ),
 		'icon' => 'w',
 	),
 	'edit-user-talk' => array(
-		'type' => 'edit',
+		'formatter-class' => 'EchoEditFormatter',
 		'title-message' => 'notification-edit-talk-page2',
 		'title-params' => array( 'agent', 'user' ),
 		'payload' => array( 'summary' ),
@@ -283,7 +283,7 @@ $wgEchoNotificationFormatters = array(
 		'icon' => 'chat',
 	),
 	'add-comment' => array(
-		'type' => 'comment',
+		'formatter-class' => 'EchoCommentFormatter',
 		'title-message' => 'notification-add-comment2',
 		'title-message-yours' => 'notification-add-comment-yours2',
 		'title-params' => array( 'agent', 'subject', 'title', 'content-page' ),
@@ -291,7 +291,7 @@ $wgEchoNotificationFormatters = array(
 		'icon' => 'chat',
 	),
 	'add-talkpage-topic' => array(
-		'type' => 'comment',
+		'formatter-class' => 'EchoCommentFormatter',
 		'title-message' => 'notification-add-talkpage-topic2',
 		'title-message-yours' => 'notification-add-talkpage-topic-yours2',
 		'title-params' => array( 'agent', 'subject', 'title', 'content-page' ),
@@ -299,7 +299,7 @@ $wgEchoNotificationFormatters = array(
 		'icon' => 'chat',
 	),
 	'reverted' => array(
-		'type' => 'edit',
+		'formatter-class' => 'EchoEditFormatter',
 		'title-message' => 'notification-reverted2',
 		'title-params' => array( 'agent', 'title', 'difflink', 'number' ),
 		'payload' => array( 'summary' ),
@@ -314,7 +314,7 @@ $wgEchoNotificationFormatters = array(
 		'icon' => 'revert',
 	),
 	'article-linked' => array(
-		'class' => 'MWEchoArticleLinkedFormatter',
+		'formatter-class' => 'EchoArticleLinkedFormatter',
 		'title-message' => 'notification-article-linked2',
 		'title-params' => array( 'agent', 'title',  'title-linked' ),
 		'payload' => array( 'summary' ),
diff --git a/formatters/ArticleLinkedFormatter.php b/formatters/ArticleLinkedFormatter.php
index 4cd1e5fa9..82cc7c11e 100644
--- a/formatters/ArticleLinkedFormatter.php
+++ b/formatters/ArticleLinkedFormatter.php
@@ -3,7 +3,7 @@
 /**
  * Formatter for 'article-linked' notifications
  */
-class MWEchoArticleLinkedFormatter extends EchoEditFormatter {
+class EchoArticleLinkedFormatter extends EchoEditFormatter {
 
 	/**
 	 * @param $event EchoEvent
diff --git a/formatters/NotificationFormatter.php b/formatters/NotificationFormatter.php
index 6e30c2547..a6bb42d43 100644
--- a/formatters/NotificationFormatter.php
+++ b/formatters/NotificationFormatter.php
@@ -7,12 +7,6 @@
  * content or requirements.
  */
 abstract class EchoNotificationFormatter {
-	static $formatterClasses = array(
-		'basic' => 'EchoBasicFormatter',
-		'edit' => 'EchoEditFormatter',
-		'comment' => 'EchoCommentFormatter',
-		'system' => 'EchoBasicFormatter',
-	);
 	protected $validOutputFormats = array( 'text', 'flyout', 'html', 'email' );
 	protected $outputFormat = 'text';
 	protected $parameters = array();
@@ -67,24 +61,20 @@ abstract class EchoNotificationFormatter {
 	/**
 	 * Create an EchoNotificationFormatter from the supplied parameters.
 	 * @param $parameters array Associative array.
-	 * Select the class of formatter to use with the 'type' or 'class' field.
+	 * Select the class of formatter to use with the 'class' field.
 	 * For other parameters, see the appropriate class' constructor.
 	 * @throws MWException
 	 * @return EchoNotificationFormatter object.
 	 */
 	public static function factory( $parameters ) {
 		$class = null;
-		if ( isset( $parameters['type'] ) ) {
-			$type = $parameters['type'];
-			if ( isset( self::$formatterClasses[$type] ) ) {
-				$class = self::$formatterClasses[$type];
-			}
-		} elseif ( isset( $parameters['class'] ) ) {
-			$class = $parameters['class'];
+		if ( isset( $parameters['formatter-class'] ) ) {
+			$class = $parameters['formatter-class'];
 		}
 
+		// Default to basic formatter
 		if ( !$class || !class_exists( $class ) ) {
-			throw new MWException( "No valid class ($class) or type ($type) specified for " . __METHOD__ );
+			$class = 'EchoBasicFormatter';
 		}
 
 		return new $class( $parameters );