r114128 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r114127‎ \| r114128 \| r114129 >
Date:	11:30, 19 March 2012
Author:	amire80
Status:	resolved (Comments)
Tags:	i18nreview
Comment:	Added TranslationNotificationsHooks and a callback for logging. Added very basic logging. Renamed form fields variables and function name to something more meaningful. Added loading of autocomplete module.
Modified paths:	/trunk/extensions/TranslationNotifications/SpecialNotifyTranslators.php (modified) (history) /trunk/extensions/TranslationNotifications/TranslationNotifications.i18n.php (modified) (history) /trunk/extensions/TranslationNotifications/TranslationNotifications.php (modified) (history) /trunk/extensions/TranslationNotifications/TranslationNotificationsHooks.php (added) (history)

Diff [purge]

Index: trunk/extensions/TranslationNotifications/TranslationNotificationsHooks.php
—	—	@@ -0,0 +1,38 @@
	2	+<?php
	3	+/**
	4	+ * Some hooks for TranslationNotifications extension.
	5	+ *
	6	+ * @file
	7	+ * @author Amir E. Aharoni
	8	+ * @copyright Copyright © 2012, Amir E. Aharoni
	9	+ * @license http://www.gnu.org/copyleft/gpl.html GNU General Public License 2.0 or later
	10	+ */
	11	+
	12	+/**
	13	+ * Some hooks for TranslationNotifications extension.
	14	+ */
	15	+class TranslationNotificationsHooks {
	16	+ public static function formatTranslationNotificationLogEntry( $type, $action, $title, $forUI, $params ) {
	17	+ global $wgLang, $wgContLang;
	18	+
	19	+ $language = $forUI === null ? $wgContLang : $wgLang;
	20	+
	21	+ $notifiedLanguages = $params[0];
	22	+ $sentSuccess = $params[1];
	23	+ $sentFail = $params[2];
	24	+
	25	+ $link = $forUI ?
	26	+ Linker::link( $title, null, array(), array( 'oldid' => $params[0] ) ) :
	27	+ $title->getPrefixedText();
	28	+ return wfMessage( 'logentry-translationnotifications-sent' )->params(
	29	+ '', // User link in the new system
	30	+ '#', // User name for gender in the new system
	31	+ Message::rawParam( $link ),
	32	+ $notifiedLanguages,
	33	+ SpecialNotifyTranslators::$deadlineDate,
	34	+ SpecialNotifyTranslators::$priority,
	35	+ $sentSuccess,
	36	+ $sentFail
	37	+ )->inLanguage( $language )->text();
	38	+ }
	39	+}
Property changes on: trunk/extensions/TranslationNotifications/TranslationNotificationsHooks.php
___________________________________________________________________
Added: svn:eol-style
1	40	+ native
Index: trunk/extensions/TranslationNotifications/SpecialNotifyTranslators.php
—	—	@@ -17,11 +17,11 @@
18	18	*/
19	19
20	20	class SpecialNotifyTranslators extends SpecialPage {
21		~~- private static $right = 'translate-manage';~~
22		~~- private static $notificationText = '';~~
23		~~- private static $translatablePageTitle = '';~~
24		~~- private static $deadlineDate = '';~~
25		~~- private static $priority = '';~~
	21	+ public static $right = 'translate-manage';
	22	+ public static $notificationText = '';
	23	+ public static $translatablePageTitle = '';
	24	+ public static $deadlineDate = '';
	25	+ public static $priority = '';
26	26
27	27	public function __construct() {
28	28	parent::__construct( 'NotifyTranslators' );
—	—	@@ -35,7 +35,7 @@
36	36	throw new PermissionsError( self::$right );
37	37	}
38	38
39		~~- $htmlFormDataModel = $this->getDataModel();~~
	39	+ $htmlFormDataModel = $this->getFormFields();
40	40
41	41	if ( !is_array( $htmlFormDataModel ) ) {
42	42	$wgOut->addWikiMsg( $htmlFormDataModel );
—	—	@@ -43,7 +43,7 @@
44	44	}
45	45
46	46	$context = $this->getContext();
47		~~- $htmlForm = new HtmlForm( $this->getDataModel(), $context, 'translationnotifications' );~~
	47	+ $htmlForm = new HtmlForm( $this->getFormFields(), $context, 'translationnotifications' );
48	48	$htmlForm->setId( 'notifytranslators-form' );
49	49	$htmlForm->setSubmitText( $context->msg( 'translationnotifications-send-notification-button' )->text() );
50	50	$htmlForm->setSubmitID( 'translationnotifications-send-notification-button' );
—	—	@@ -58,7 +58,7 @@
59	59	*
60	60	* @return array or string with an error message key in case of error
61	61	*/
62		~~- private function getDataModel() {~~
	62	+ private function getFormFields() {
63	63
64	64	// Translatable pages dropdown
65	65	$translatablePagesIDs = TranslatablePage::getTranslatablePages();
—	—	@@ -71,7 +71,9 @@
72	72	$translatablePagesOptions[Title::newFromID( $translatablePagesID )->getText()] = $translatablePagesID;
73	73	}
74	74
75		~~- $m['TranslatablePage'] = array(~~
	75	+ $formFields = array();
	76	+
	77	+ $formFields['TranslatablePage'] = array(
76	78	'type' => 'select',
77	79	'label-message' => array( 'translationnotifications-translatablepage-title' ),
78	80	'options' => $translatablePagesOptions,
—	—	@@ -79,7 +81,7 @@
80	82	);
81	83
82	84	// Languages to notify input box
83		~~- $m['LanguagesToNotify'] = array(~~
	85	+ $formFields['LanguagesToNotify'] = array(
84	86	'type' => 'text',
85	87	'rows' => 20,
86	88	'cols' => 80,
—	—	@@ -96,7 +98,7 @@
97	99	$priorityOptions[$priorityMessage] = $priority;
98	100	}
99	101
100		~~- $m['Priority'] = array(~~
	102	+ $formFields['Priority'] = array(
101	103	'type' => 'select',
102	104	'label-message' => array( 'translationnotifications-priority' ),
103	105	'options' => $priorityOptions,
—	—	@@ -104,21 +106,21 @@
105	107	);
106	108
107	109	// Deadline date input box with datepicker
108		~~- $m['DeadlineDate'] = array(~~
	110	+ $formFields['DeadlineDate'] = array(
109	111	'type' => 'text',
110	112	'size' => 20,
111	113	'label-message' => 'translationnotifications-deadline-label',
112	114	);
113	115
114	116	// Custom text
115		~~- $m['NotificationText'] = array(~~
	117	+ $formFields['NotificationText'] = array(
116	118	'type' => 'textarea',
117	119	'rows' => 20,
118	120	'cols' => 80,
119	121	'label-message' => 'emailmessage',
120	122	);
121	123
122		~~- return $m;~~
	124	+ return $formFields;
123	125	}
124	126
125	127	/**
—	—	@@ -127,6 +129,8 @@
128	130	* TODO: document
129	131	*/
130	132	public function submitNotifyTranslatorsForm( $formData, $form ) {
	133	+ global $wgUser;
	134	+
131	135	self::$translatablePageTitle = Title::newFromID( $formData['TranslatablePage'] )->getText();
132	136	self::$notificationText = $formData['NotificationText'];
133	137	self::$priority = $formData['Priority'];
—	—	@@ -148,10 +152,31 @@
149	153	'DISTINCT'
150	154	);
151	155
	156	+ $sentSuccess = 0;
	157	+ $sentFail = 0;
152	158	foreach ( $translators as $row ) {
153		~~- $this->sendTranslationNotificationEmail( $row->up_user );~~
	159	+ $status = $this->sendTranslationNotificationEmail( $row->up_user );
	160	+ if ( $status->isGood() ) {
	161	+ $sentSuccess++;
	162	+ } else {
	163	+ $sentFail++;
	164	+ }
154	165	}
155	166
	167	+ $logger = new LogPage( 'notifytranslators' );
	168	+ $logParams = array(
	169	+ $formData['LanguagesToNotify'],
	170	+ $sentSuccess,
	171	+ $sentFail,
	172	+ );
	173	+ $logger->addEntry(
	174	+ 'sent',
	175	+ Title::newFromText( self::$translatablePageTitle ),
	176	+ '', // No comments
	177	+ $logParams,
	178	+ $wgUser
	179	+ );
	180	+
156	181	self::$translatablePageTitle = '';
157	182	self::$notificationText = '';
158	183	self::$deadlineDate = '';
Index: trunk/extensions/TranslationNotifications/TranslationNotifications.i18n.php
—	—	@@ -68,6 +68,7 @@
69	69	{{SITENAME}} staff',
70	70	'translationnotifications-email-priority' => 'The priority of this page is $1.',
71	71	'translationnotifications-email-deadline' => 'The deadline for translating this page is $1.',
	72	+ 'logentry-translationnotifications-sent' => '$1 {{GENDER:$2\|sent}} a notification about translating page $3 to languages $4 with deadline $5, with $6 priority, to {{PLURAL:$7\|one recipient\|$7 recipients}} successfully, to {{PLURAL:$8\|one recipient\|$8 recipients}} unsuccessfully',
72	73	);
73	74
74	75	/** Message documentation (Message documentation)
—	—	@@ -125,6 +126,15 @@
126	127	* $7 - A custom message that can be added by the notification sender.",
127	128	'translationnotifications-email-priority' => 'Used in {{msg-mw\|translationnotifications-email-body}}',
128	129	'translationnotifications-email-deadline' => 'Used in {{msg-mw\|translationnotifications-email-body}}',
	130	+ 'logentry-translationnotifications-sent' => '{{logentry}}
	131	+* $1 - username
	132	+* $2 - username for gender
	133	+* $3 - translatable page title
	134	+* $4 - languages list
	135	+* $5 - deadline
	136	+* $6 - priority
	137	+* $7 - number of recipients to whom the notification was sent successfully
	138	+* $8 - number of recipients to whom sending the notification failed',
129	139	);
130	140
131	141	/** Ṫuroyo (Ṫuroyo)
Index: trunk/extensions/TranslationNotifications/TranslationNotifications.php
—	—	@@ -32,12 +32,23 @@
33	33	$wgExtensionMessagesFiles['TranslationNotificationsAlias'] = "$dir/TranslationNotifications.alias.php";
34	34	$wgAutoloadClasses['SpecialTranslatorSignup'] = "$dir/SpecialTranslatorSignup.php";
35	35	$wgAutoloadClasses['SpecialNotifyTranslators'] = "$dir/SpecialNotifyTranslators.php";
	36	+$wgAutoloadClasses['TranslationNotificationsHooks'] = "$dir/TranslationNotificationsHooks.php";
36	37
37	38	$resourcePaths = array(
38	39	'localBasePath' => dirname( __FILE__ ),
39	40	'remoteExtPath' => 'TranslationNotifications'
40	41	);
41	42
	43	+// For language list autocompletion
	44	+$wgResourceModules['ext.translate.special.pagetranslation'] = array(
	45	+ 'scripts' => '../Translate/resources/ext.translate.special.pagetranslation.js',
	46	+ 'styles' => '../Translate/resources/ext.translate.special.pagetranslation.css',
	47	+ 'dependencies' => array(
	48	+ 'jquery.ui.autocomplete',
	49	+ ),
	50	+ 'position' => 'top',
	51	+) + $resourcePaths;
	52	+
42	53	$wgResourceModules['ext.translationnotifications.notifytranslators'] = array(
43	54	'scripts' => 'resources/ext.translationnotifications.notifytranslators.js',
44	55	'dependencies' => array(
—	—	@@ -52,3 +63,6 @@
53	64	'feed' => false,
54	65	'no' => true,
55	66	);
	67	+
	68	+$wgLogTypes[] = 'notifytranslators';
	69	+$wgLogActionsHandlers['notifytranslators/sent'] = 'TranslationNotificationsHooks::formatTranslationNotificationLogEntry';

Follow-up revisions

Revision	Commit summary	Author	Date
r114251	Using the refactored multiselectautocomplete module.	amire80	10:59, 20 March 2012

Comments

#Comment by Nikerabbit (talk | contribs) 12:07, 19 March 2012

Why static variables?

#Comment by Amire80 (talk | contribs) 11:05, 20 March 2012

Because they are not changed and they are used from the hooks class. Or is it a bad practice and they should be regular members with accessors etc.?

#Comment by 😂 (talk | contribs) 03:10, 21 March 2012

Public static variables are just global variables in disguise--regular members with accessors is much better.

#Comment by Santhosh.thottingal (talk | contribs) 12:40, 19 March 2012

Commit missed ext.translate.special.pagetranslation.js and ext.translate.special.pagetranslation.css

#Comment by Amire80 (talk | contribs) 11:04, 20 March 2012

Addressed in r114251.

Status & tagging log

18:15, 21 March 2012 Nikerabbit (talk | contribs) changed the status of r114128 [removed: new added: resolved]
18:12, 21 March 2012 Amire80 (talk | contribs) changed the status of r114128 [removed: fixme added: new]
18:02, 21 March 2012 Siebrand (talk | contribs) changed the tags for r114128 [added: i18nreview]
12:40, 19 March 2012 Santhosh.thottingal (talk | contribs) changed the status of r114128 [removed: new added: fixme]