r75423 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r75422‎ \| r75423 \| r75424 >
Date:	14:18, 26 October 2010
Author:	ashley
Status:	deferred
Tags:
Comment:	Drafts: cleanup
Modified paths:	/trunk/extensions/Drafts/Drafts.classes.php (modified) (history) /trunk/extensions/Drafts/Drafts.hooks.php (modified) (history) /trunk/extensions/Drafts/Drafts.js (modified) (history) /trunk/extensions/Drafts/Drafts.pages.php (modified) (history) /trunk/extensions/Drafts/Drafts.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Drafts/Drafts.classes.php
—	—	@@ -17,14 +17,12 @@
18	18
19	19	/**
20	20	* Counts the number of existing drafts for a specific user
	21	+ *
	22	+ * @param $title Object: [optional] Title of article, defaults to all articles
	23	+ * @param $userID Integer: [optional] ID of user, defaults to current user
21	24	* @return Number of drafts which match condition parameters
22		~~- * @param object $title[optional] Title of article, defaults to all articles~~
23		~~- * @param integer $userID[optional] ID of user, defaults to current user~~
24	25	*/
25		~~- public static function num(~~
26		~~- $title = null,~~
27		~~- $userID = null~~
28		~~- ) {~~
	26	+ public static function num( $title = null, $userID = null ) {
29	27	global $wgUser;
30	28	// Get database connection
31	29	$dbr = wfGetDB( DB_SLAVE );
—	—	@@ -63,11 +61,11 @@
64	62
65	63	/**
66	64	* Removes drafts which have not been modified for a period of time defined
67		~~- * by $wgDraftsLifeSpan~~
	65	+ * by $egDraftsCleanRatio
68	66	*/
69	67	public static function clean() {
70	68	global $egDraftsCleanRatio;
71		-
	69	+
72	70	// Only perform this action a fraction of the time
73	71	if ( rand( 0, $egDraftsCleanRatio ) == 0 ) {
74	72	// Get database connection
—	—	@@ -89,10 +87,7 @@
90	88	* Re-titles drafts which point to a particlar article, as a response to the
91	89	* article being moved.
92	90	*/
93		~~- public static function move(~~
94		~~- $oldTitle,~~
95		~~- $newTitle~~
96		~~- ) {~~
	91	+ public static function move( $oldTitle, $newTitle ) {
97	92	// Get database connection
98	93	$dbw = wfGetDB( DB_MASTER );
99	94	// Updates title and namespace of drafts upon moving
—	—	@@ -110,14 +105,12 @@
111	106
112	107	/**
113	108	* Gets a list of existing drafts for a specific user
114		~~- * @return~~
115		~~- * @param object $title[optional] Title of article, defaults to all articles~~
116		~~- * @param integer $userID[optional] ID of user, defaults to current user~~
	109	+ *
	110	+ * @param $title Object: [optional] Title of article, defaults to all articles
	111	+ * @param $userID Integer: [optional] ID of user, defaults to current user
	112	+ * @return List of drafts or null
117	113	*/
118		~~- public static function get(~~
119		~~- $title = null,~~
120		~~- $userID = null~~
121		~~- ) {~~
	114	+ public static function get( $title = null, $userID = null ) {
122	115	global $wgUser;
123	116	// Removes expired drafts for a more accurate list
124	117	Drafts::clean();
—	—	@@ -167,14 +160,12 @@
168	161
169	162	/**
170	163	* Outputs a table of existing drafts
	164	+ *
	165	+ * @param $title Object: [optional] Title of article, defaults to all articles
	166	+ * @param $userID Integer: [optional] ID of user, defaults to current user
171	167	* @return Number of drafts in the table
172		~~- * @param object $title[optional] Title of article, defaults to all articles~~
173		~~- * @param integer $userID[optional] ID of user, defaults to current user~~
174	168	*/
175		~~- public static function display(~~
176		~~- $title = null,~~
177		~~- $userID = null~~
178		~~- ) {~~
	169	+ public static function display( $title = null, $userID = null ) {
179	170	global $wgOut, $wgRequest, $wgUser, $wgLang;
180	171	// Gets draftID
181	172	$currentDraft = Draft::newFromID( $wgRequest->getIntOrNull( 'draft' ) );
—	—	@@ -226,11 +217,11 @@
227	218	// Get article title text
228	219	$htmlTitle = $draft->getTitle()->getEscapedText();
229	220	// Build Article Load link
230		~~- $urlLoad = $draft->getTitle()->getFullUrl(~~
	221	+ $urlLoad = $draft->getTitle()->getFullURL(
231	222	'action=edit&draft=' . urlencode( $draft->getID() )
232	223	);
233	224	// Build discard link
234		~~- $urlDiscard = SpecialPage::getTitleFor( 'Drafts' )->getFullUrl(~~
	225	+ $urlDiscard = SpecialPage::getTitleFor( 'Drafts' )->getFullURL(
235	226	sprintf( 'discard=%s&token=%s',
236	227	urlencode( $draft->getID() ),
237	228	urlencode( $wgUser->editToken() )
—	—	@@ -314,7 +305,6 @@
315	306	class Draft {
316	307
317	308	/* Members */
318		-
319	309	private $exists = false;
320	310	private $id;
321	311	private $token;
—	—	@@ -324,7 +314,7 @@
325	315	private $starttime;
326	316	private $edittime;
327	317	private $savetime;
328		~~- private $scrolltop ;~~
	318	+ private $scrolltop;
329	319	private $text;
330	320	private $summary;
331	321	private $minoredit;
—	—	@@ -333,25 +323,22 @@
334	324
335	325	/**
336	326	* Creates a new Draft object from a draft ID
	327	+ *
	328	+ * @param $id Integer: ID of draft
	329	+ * @param $autoload Boolean: [optional] Whether to load draft information
337	330	* @return New Draft object
338		~~- * @param integer $id ID of draft~~
339		~~- * @param boolean $autoload[optional] Whether to load draft information~~
340	331	*/
341		~~- public static function newFromID(~~
342		~~- $id,~~
343		~~- $autoload = true~~
344		~~- ) {~~
	332	+ public static function newFromID( $id, $autoload = true ) {
345	333	return new Draft( $id, $autoload );
346	334	}
347	335
348	336	/**
349	337	* Creates a new Draft object from a database row
	338	+ *
	339	+ * @param $row Array: Database row to create Draft object with
350	340	* @return New Draft object
351		~~- * @param array $row Database row to create Draft object with~~
352	341	*/
353		~~- public static function newFromRow(~~
354		~~- $row~~
355		~~- ) {~~
	342	+ public static function newFromRow( $row ) {
356	343	$draft = new Draft( $row['draft_id'], false );
357	344	$draft->setToken( $row['draft_token'] );
358	345	$draft->setTitle(
—	—	@@ -393,28 +380,24 @@
394	381
395	382	/**
396	383	* Sets the edit token, like one generated by wfGenerateToken()
397		~~- * @param string $token~~
	384	+ * @param $token String
398	385	*/
399		~~- public function setToken(~~
400		~~- $token~~
401		~~- ) {~~
	386	+ public function setToken( $token ) {
402	387	$this->token = $token;
403	388	}
404		-
	389	+
405	390	/**
406	391	* @return User ID of draft creator
407	392	*/
408	393	public function getUserID() {
409	394	return $this->userID;
410	395	}
411		-
	396	+
412	397	/**
413	398	* Sets user ID of draft creator
414		~~- * @param integer $userID~~
	399	+ * @param $userID Integer: user ID
415	400	*/
416		~~- public function setUserID(~~
417		~~- $userID~~
418		~~- ) {~~
	401	+ public function setUserID( $userID ) {
419	402	$this->userID = $userID;
420	403	}
421	404
—	—	@@ -424,14 +407,12 @@
425	408	public function getTitle() {
426	409	return $this->title;
427	410	}
428		-
	411	+
429	412	/**
430	413	* Sets title of article of draft
431		~~- * @param object $title~~
	414	+ * @param $title Object
432	415	*/
433		~~- public function setTitle(~~
434		~~- $title~~
435		~~- ) {~~
	416	+ public function setTitle( $title ) {
436	417	$this->title = $title;
437	418	}
438	419
—	—	@@ -441,14 +422,12 @@
442	423	public function getSection() {
443	424	return $this->section;
444	425	}
445		-
	426	+
446	427	/**
447	428	* Sets section of the article of draft
448		~~- * @param integer $section~~
	429	+ * @param $section Integer
449	430	*/
450		~~- public function setSection(~~
451		~~- $section~~
452		~~- ) {~~
	431	+ public function setSection( $section ) {
453	432	$this->section = $section;
454	433	}
455	434
—	—	@@ -458,14 +437,12 @@
459	438	public function getStartTime() {
460	439	return $this->starttime;
461	440	}
462		-
	441	+
463	442	/**
464	443	* Sets time when draft of the article started
465		~~- * @param string $starttime~~
	444	+ * @param $starttime String
466	445	*/
467		~~- public function setStartTime(~~
468		~~- $starttime~~
469		~~- ) {~~
	446	+ public function setStartTime( $starttime ) {
470	447	$this->starttime = $starttime;
471	448	}
472	449
—	—	@@ -475,14 +452,12 @@
476	453	public function getEditTime() {
477	454	return $this->edittime;
478	455	}
479		-
	456	+
480	457	/**
481	458	* Sets time of most recent revision of article when this draft started
482		~~- * @param string $edittime~~
	459	+ * @param $edittime String
483	460	*/
484		~~- public function setEditTime(~~
485		~~- $edittime~~
486		~~- ) {~~
	461	+ public function setEditTime( $edittime ) {
487	462	$this->edittime = $edittime;
488	463	}
489	464
—	—	@@ -492,14 +467,12 @@
493	468	public function getSaveTime() {
494	469	return $this->savetime;
495	470	}
496		-
	471	+
497	472	/**
498	473	* Sets time when draft was last modified
499		~~- * @param string $savetime~~
	474	+ * @param $savetime String
500	475	*/
501		~~- public function setSaveTime(~~
502		~~- $savetime~~
503		~~- ) {~~
	476	+ public function setSaveTime( $savetime ) {
504	477	$this->savetime = $savetime;
505	478	}
506	479
—	—	@@ -509,14 +482,12 @@
510	483	public function getScrollTop() {
511	484	return $this->scrolltop;
512	485	}
513		-
	486	+
514	487	/**
515	488	* Sets scroll position of editor when draft was last modified
516		~~- * @param integer $scrolltop~~
	489	+ * @param $scrolltop Integer
517	490	*/
518		~~- public function setScrollTop(~~
519		~~- $scrolltop~~
520		~~- ) {~~
	491	+ public function setScrollTop( $scrolltop ) {
521	492	$this->scrolltop = $scrolltop;
522	493	}
523	494
—	—	@@ -526,14 +497,12 @@
527	498	public function getText() {
528	499	return $this->text;
529	500	}
530		-
	501	+
531	502	/**
532	503	* Sets text of draft version of article
533		~~- * @param string $text~~
	504	+ * @param $text String
534	505	*/
535		~~- public function setText(~~
536		~~- $text~~
537		~~- ) {~~
	506	+ public function setText( $text ) {
538	507	$this->text = $text;
539	508	}
540	509
—	—	@@ -543,14 +512,12 @@
544	513	public function getSummary() {
545	514	return $this->summary;
546	515	}
547		-
	516	+
548	517	/**
549	518	* Sets summary of changes
550		~~- * @param string $summary~~
	519	+ * @param $summary String
551	520	*/
552		~~- public function setSummary(~~
553		~~- $summary~~
554		~~- ) {~~
	521	+ public function setSummary( $summary ) {
555	522	$this->summary = $summary;
556	523	}
557	524
—	—	@@ -560,7 +527,7 @@
561	528	public function getMinorEdit() {
562	529	return $this->minoredit;
563	530	}
564		-
	531	+
565	532	/**
566	533	* Sets whether edit is considdered to be a minor change
567	534	* @param boolean $minoredit
—	—	@@ -575,22 +542,19 @@
576	543
577	544	/**
578	545	* Generic constructor
579		~~- * @param integer $id[optional] ID to use~~
580		~~- * @param boolean $autoload[optional] Whether to load from database~~
	546	+ * @param $id Integer: [optional] ID to use
	547	+ * @param $autoload Boolean: [optional] Whether to load from database
581	548	*/
582		~~- public function __construct(~~
583		~~- $id = null,~~
584		~~- $autoload = true~~
585		~~- ) {~~
	549	+ public function __construct( $id = null, $autoload = true ) {
586	550	// If an ID is a number the existence is actually checked on load
587		~~- // If an ID is false the existance is always false durring load~~
	551	+ // If an ID is false the existance is always false during load
588	552	$this->id = $id;
589	553	// Load automatically
590	554	if ( $autoload ) {
591	555	$this->load();
592	556	}
593	557	}
594		-
	558	+
595	559	/**
596	560	* Selects draft row from database and populates object properties
597	561	*/
—	—	@@ -640,7 +604,7 @@
641	605	// Updates state
642	606	$this->exists = true;
643	607	}
644		-
	608	+
645	609	/**
646	610	* Inserts or updates draft row in database
647	611	*/
—	—	@@ -704,14 +668,12 @@
705	669	// Returns success
706	670	return true;
707	671	}
708		-
	672	+
709	673	/**
710	674	* Deletes draft row from database
711		~~- * @param integer $user[optional] User ID, defaults to current user ID~~
	675	+ * @param $user Integer: [optional] User ID, defaults to current user ID
712	676	*/
713		~~- public function discard(~~
714		~~- $user = null~~
715		~~- ) {~~
	677	+ public function discard( $user = null ) {
716	678	global $wgUser;
717	679	// Uses $wgUser as a fallback
718	680	$user = $user === null ? $wgUser : $user;
Index: trunk/extensions/Drafts/Drafts.pages.php
—	—	@@ -22,7 +22,8 @@
23	23
24	24	/**
25	25	* Executes special page rendering and data processing
26		~~- * @param string $sub MediaWiki supplied sub-page path~~
	26	+ *
	27	+ * @param $sub Mixed: MediaWiki supplied sub-page path
27	28	*/
28	29	public function execute( $sub ) {
29	30	global $wgRequest, $wgOut, $wgUser;
Index: trunk/extensions/Drafts/Drafts.php
—	—	@@ -19,7 +19,7 @@
20	20	// Check environment
21	21	if ( !defined( 'MEDIAWIKI' ) ) {
22	22	echo( "This is an extension to MediaWiki and cannot be run standalone.\n" );
23		~~- die( - 1 );~~
	23	+ die( -1 );
24	24	}
25	25
26	26	/* Configuration */
—	—	@@ -28,6 +28,7 @@
29	29	$wgExtensionCredits['other'][] = array(
30	30	'path' => __FILE__,
31	31	'name' => 'Drafts',
	32	+ 'version' => '0.1.0',
32	33	'author' => 'Trevor Parscal',
33	34	'url' => 'http://www.mediawiki.org/wiki/Extension:Drafts',
34	35	'descriptionmsg' => 'drafts-desc',
Index: trunk/extensions/Drafts/Drafts.hooks.php
—	—	@@ -9,9 +9,9 @@
10	10	class DraftHooks {
11	11
12	12	/* Static Functions */
13		~~-public static function schema() {~~
	13	+ public static function schema() {
14	14	global $wgExtNewTables, $wgExtModifiedFields;
15		-
	15	+
16	16	$wgExtNewTables[] = array(
17	17	'drafts',
18	18	dirname( __FILE__ ) . '/Drafts.sql'
—	—	@@ -21,18 +21,14 @@
22	22	'draft_token',
23	23	dirname( __FILE__ ) . '/patch-draft_token.sql'
24	24	);
25		-
	25	+
26	26	return true;
27	27	}
28	28
29	29	/**
30	30	* SpecialMovepageAfterMove hook
31	31	*/
32		~~- public static function move(~~
33		~~- $this,~~
34		~~- $ot,~~
35		~~- $nt~~
36		~~- ) {~~
	32	+ public static function move( $this, $ot, $nt ) {
37	33	// Update all drafts of old article to new article for all users
38	34	Drafts::move( $ot, $nt );
39	35	// Continue
—	—	@@ -42,16 +38,8 @@
43	39	/**
44	40	* ArticleSaveComplete hook
45	41	*/
46		~~- public static function discard(~~
47		~~- $article,~~
48		~~- $user,~~
49		~~- $text,~~
50		~~- $summary,~~
51		~~- $m,~~
52		~~- $watchthis,~~
53		~~- $section,~~
54		~~- $flags,~~
55		~~- $rev~~
	42	+ public static function discard( $article, $user, $text, $summary, $m,
	43	+ $watchthis, $section, $flags, $rev
56	44	) {
57	45	global $wgRequest;
58	46	// Check if the save occured from a draft
—	—	@@ -68,9 +56,7 @@
69	57	* EditPage::showEditForm:initial hook
70	58	* Load draft...
71	59	*/
72		~~- public static function loadForm(~~
73		~~- $editpage~~
74		~~- ) {~~
	60	+ public static function loadForm( $editpage ) {
75	61	global $wgUser, $wgRequest, $wgOut, $wgTitle, $wgLang;
76	62	// Check permissions
77	63	if ( $wgUser->isAllowed( 'edit' ) && $wgUser->isLoggedIn() ) {
—	—	@@ -155,12 +141,7 @@
156	142	* Intercept the saving of an article to detect if the submission was from
157	143	* the non-javascript save draft button
158	144	*/
159		~~- public static function interceptSave(~~
160		~~- $editor,~~
161		~~- $text,~~
162		~~- $section,~~
163		~~- $error~~
164		~~- ) {~~
	145	+ public static function interceptSave( $editor, $text, $section, $error ) {
165	146	global $wgRequest;
166	147	// Don't save if the save draft button caused the submit
167	148	if ( $wgRequest->getText( 'wpDraftSave' ) !== '' ) {
—	—	@@ -175,10 +156,7 @@
176	157	* EditPageBeforeEditButtons hook
177	158	* Add draft saving controls
178	159	*/
179		~~- public static function controls(~~
180		~~- $editpage,~~
181		~~- $buttons~~
182		~~- ) {~~
	160	+ public static function controls( $editpage, $buttons ) {
183	161	global $wgUser, $wgTitle, $wgRequest;
184	162	global $egDraftsAutoSaveWait, $egDraftsAutoSaveTimeout;
185	163	// Check permissions
—	—	@@ -294,14 +272,12 @@
295	273
296	274	/**
297	275	* AjaxAddScript hook
298		~~- * Add ajax support script~~
	276	+ * Add AJAX support script
299	277	*/
300		~~- public static function addJS(~~
301		~~- $out~~
302		~~- ) {~~
	278	+ public static function addJS( $out ) {
303	279	global $wgScriptPath, $wgJsMimeType, $wgDraftsStyleVersion;
304	280	// FIXME: assumes standard dir structure
305		~~- // Add javascript to support ajax draft saving~~
	281	+ // Add JavaScript to support AJAX draft saving
306	282	$out->addScript(
307	283	Xml::element(
308	284	'script',
—	—	@@ -320,14 +296,12 @@
321	297
322	298	/**
323	299	* BeforePageDisplay hook
324		~~- * Add css style sheet~~
	300	+ * Add CSS style sheet
325	301	*/
326		~~- public static function addCSS(~~
327		~~- $out~~
328		~~- ) {~~
	302	+ public static function addCSS( $out ) {
329	303	global $wgScriptPath, $wgDraftsStyleVersion;
330	304	// FIXME: assumes standard dir structure
331		~~- // Add css for various styles~~
	305	+ // Add CSS for various styles
332	306	$out->addLink(
333	307	array(
334	308	'rel' => 'stylesheet',
—	—	@@ -342,20 +316,10 @@
343	317
344	318	/**
345	319	* AJAX function export DraftHooks::AjaxSave
346		~~- * Respond to ajax queries~~
	320	+ * Respond to AJAX queries
347	321	*/
348		~~- public static function save(~~
349		~~- $dtoken,~~
350		~~- $etoken,~~
351		~~- $id,~~
352		~~- $title,~~
353		~~- $section,~~
354		~~- $starttime,~~
355		~~- $edittime,~~
356		~~- $scrolltop,~~
357		~~- $text,~~
358		~~- $summary,~~
359		~~- $minoredit~~
	322	+ public static function save( $dtoken, $etoken, $id, $title, $section,
	323	+ $starttime, $edittime, $scrolltop, $text, $summary, $minoredit
360	324	) {
361	325	global $wgUser, $wgRequest;
362	326	// Verify token
Index: trunk/extensions/Drafts/Drafts.js
—	—	@@ -1,9 +1,9 @@
2	2	/* JavaScript for Drafts extension */
3	3
4	4	function Draft() {
5		-
	5	+
6	6	/* Private Members */
7		-
	7	+
8	8	// Reference to object's self
9	9	var self = this;
10	10	// Configuration settings
—	—	@@ -16,16 +16,14 @@
17	17	var timer = null;
18	18	// Reference to edit form draft is being edited with
19	19	var form = null;
20		-
	20	+
21	21	/* Functions */
22		-
	22	+
23	23	/**
24	24	* Sets the state of the draft
25	25	* @param {String} newState
26	26	*/
27		~~- this.setState = function(~~
28		~~- newState~~
29		~~- ) {~~
	27	+ this.setState = function( newState ) {
30	28	// Stores state information
31	29	state = newState;
32	30	// Updates UI elements
—	—	@@ -53,14 +51,14 @@
54	52	default: break;
55	53	}
56	54	}
57		-
	55	+
58	56	/**
59	57	* Gets the state of the draft
60	58	*/
61	59	this.getState = function() {
62	60	return state;
63	61	}
64		-
	62	+
65	63	/**
66	64	* Sends draft data to server to be saved
67	65	*/
—	—	@@ -75,10 +73,10 @@
76	74	// Saves current request type
77	75	var oldRequestType = sajax_request_type;
78	76	// Changes request type to post
79		~~- sajax_request_type = "POST";~~
	77	+ sajax_request_type = 'POST';
80	78	// Performs asynchronous save on server
81	79	sajax_do_call(
82		~~- "DraftHooks::save",~~
	80	+ 'DraftHooks::save',
83	81	[
84	82	form.wpDraftToken.value,
85	83	form.wpEditToken.value,
—	—	@@ -92,7 +90,7 @@
93	91	form.wpSummary.value,
94	92	form.wpMinoredit.checked ? 1 : 0
95	93	],
96		~~- new Function( "request", "wgDraft.respond( request )" )~~
	94	+ new Function( 'request', 'wgDraft.respond( request )' )
97	95	);
98	96	// Restores current request type
99	97	sajax_request_type = oldRequestType;
—	—	@@ -119,11 +117,11 @@
120	118	if ( configuration.autoSaveWait && configuration.autoSaveWait > 0 ) {
121	119	// Sets timer to save automatically after a period of time
122	120	timer = setTimeout(
123		~~- "wgDraft.save()", configuration.autoSaveWait * 1000~~
	121	+ 'wgDraft.save()', configuration.autoSaveWait * 1000
124	122	);
125	123	}
126	124	}
127		-
	125	+
128	126	/**
129	127	* Initializes the user interface
130	128	*/
—	—	@@ -165,9 +163,7 @@
166	164	* Responds to the server after a save request has been handled
167	165	* @param {Object} request
168	166	*/
169		~~- this.respond = function(~~
170		~~- request~~
171		~~- ) {~~
	167	+ this.respond = function( request ) {
172	168	// Checks that an error did not occur
173	169	if ( request.responseText > -1 ) {
174	170	// Changes state to saved
—	—	@@ -184,4 +180,4 @@
185	181	// Instantiates a draft object
186	182	var wgDraft = new Draft();
187	183	// Registers hooks
188		~~-hookEvent( "load", wgDraft.initialize );~~
	184	+hookEvent( 'load', wgDraft.initialize );

Status & tagging log

16:18, 26 October 2010 Reedy (talk | contribs) changed the status of r75423 [removed: new added: deferred]