r107522 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r107521‎ \| r107522 \| r107523 >
Date:	20:37, 28 December 2011
Author:	reedy
Status:	resolved (Comments)
Tags:
Comment:	Cleanup onLoadExtensionSchemaUpdates to use utility functions rather than array wrappers Create all indexes at point of creating table
Modified paths:	/trunk/extensions/MoodBar/MoodBar.hooks.php (modified) (history) /trunk/extensions/MoodBar/sql/MoodBar.sql (modified) (history) /trunk/extensions/MoodBar/sql/moodbar_feedback_response.sql (modified) (history)

Diff [purge]

Index: trunk/extensions/MoodBar/MoodBar.hooks.php
—	—	@@ -15,52 +15,52 @@
16	16
17	17	return true;
18	18	}
19		-
	19	+
20	20	/**
21	21	* Determines if this user has right to mark an feedback response as helpful, only the user who wrote the
22	22	* feedback can mark the response as helpful
23	23	* @param $mahaction string - mark/unmark
24	24	* @param $type string - the object type to be marked
25		~~- * @param $item int - an item of $type to be marked~~
	25	+ * @param $item int - an item of $type to be marked
26	26	* @param $User User Object - the User in current session
27	27	* @param $isAbleToMark bool - determine whether the user is able to mark the item
28	28	* @return bool
29	29	*/
30	30	public static function onMarkItemAsHelpful( $mahaction, $type, $item, $User, &$isAbleToMark ) {
31		-
	31	+
32	32	if ( $User->isAnon() ) {
33	33	$isAbleToMark = false;
34	34	return true;
35	35	}
36		-
	36	+
37	37	if ( $type == 'mbresponse' ) {
38		-
	38	+
39	39	switch ( $mahaction ) {
40		-
	40	+
41	41	case 'mark':
42	42	$dbr = wfGetDB( DB_SLAVE );
43		-
44		~~- $res = $dbr->selectRow( array( 'moodbar_feedback', 'moodbar_feedback_response' ),~~
	43	+
	44	+ $res = $dbr->selectRow( array( 'moodbar_feedback', 'moodbar_feedback_response' ),
45	45	array( 'mbf_id' ),
46		~~- array( 'mbf_id = mbfr_mbf_id',~~
47		~~- 'mbfr_id' => intval( $item ),~~
48		~~- 'mbf_user_id' => $User->getId() ),~~
49		~~- __METHOD__ );~~
50		-
	46	+ array( 'mbf_id = mbfr_mbf_id',
	47	+ 'mbfr_id' => intval( $item ),
	48	+ 'mbf_user_id' => $User->getId() ),
	49	+ __METHOD__ );
	50	+
51	51	if ( $res === false ) {
52	52	$isAbleToMark = false;
53	53	}
54		~~- break;~~
55		-
	54	+ break;
	55	+
56	56	case 'unmark':
57	57	default:
58	58	//We will leve the MarkAsHelpFul extension to check if the user has unmark right
59		~~- break;~~
	59	+ break;
60	60	}
61	61	}
62		-
	62	+
63	63	return true;
64		-
	64	+
65	65	}
66	66
67	67	/**
—	—	@@ -127,45 +127,31 @@
128	128	* @param $updater DatabasEUpdater
129	129	*/
130	130	public static function onLoadExtensionSchemaUpdates( $updater = null ) {
131		~~- $updater->addExtensionUpdate( array( 'addTable', 'moodbar_feedback',~~
132		~~- dirname(__FILE__).'/sql/MoodBar.sql', true ) );~~
	131	+ $dir = dirname(__FILE__) . '/sql/';
	132	+ $updater->addExtensionTable( 'moodbar_feedback', $dir . 'MoodBar.sql' );
133	133
134		~~- $updater->addExtensionUpdate( array( 'addField', 'moodbar_feedback',~~
135		~~- 'mbf_user_editcount', dirname(__FILE__).'/sql/mbf_user_editcount.sql', true )~~
136		~~- );~~
	134	+ $updater->addExtensionField( 'moodbar_feedback', 'mbf_user_editcount', $dir . 'mbf_user_editcount.sql' );
137	135
138		~~- $db = $updater->getDB();~~
139		~~- if ( $db->tableExists( 'moodbar_feedback' ) &&~~
140		~~- $db->indexExists( 'moodbar_feedback', 'type_timestamp', __METHOD__ ) )~~
141		~~- {~~
142		~~- $updater->addExtensionUpdate( array( 'addIndex', 'moodbar_feedback',~~
143		~~- 'mbf_type_timestamp_id', dirname( __FILE__ ) . '/sql/AddIDToIndexes.sql', true )~~
144		~~- );~~
145		~~- }~~
	136	+ $updater->addExtensionIndex( 'moodbar_feedback', 'mbf_type_timestamp_id', $dir . 'AddIDToIndexes.sql' );
	137	+
146	138	$updater->addExtensionUpdate( array( 'dropIndex', 'moodbar_feedback',
147		~~- 'mbf_timestamp', dirname( __FILE__ ) . '/sql/AddIDToIndexes2.sql', true )~~
	139	+ 'mbf_userid_ip_timestamp', $dir . 'AddIDToIndexes2.sql', true )
148	140	);
149	141
150		~~- $updater->addExtensionUpdate( array( 'addIndex', 'moodbar_feedback',~~
151		~~- 'mbf_timestamp_id', dirname( __FILE__ ) . '/sql/mbf_timestamp_id.sql', true )~~
152		~~- );~~
	142	+ $updater->addExtensionIndex( 'moodbar_feedback', 'mbfr_timestamp_id', $dir . 'mbf_timestamp_id.sql' );
153	143
154		~~- $updater->addExtensionUpdate( array( 'addField', 'moodbar_feedback',~~
155		~~- 'mbf_hidden_state', dirname(__FILE__).'/sql/mbf_hidden_state.sql', true ) );~~
156		-
157		~~- $updater->addExtensionUpdate( array( 'addTable', 'moodbar_feedback_response',~~
158		~~- dirname(__FILE__).'/sql/moodbar_feedback_response.sql', true ) );~~
	144	+ $updater->addExtensionField( 'moodbar_feedback', 'mbf_hidden_state', $dir . 'mbf_hidden_state.sql' );
159	145
160		~~- $updater->addExtensionUpdate( array( 'addIndex', 'moodbar_feedback_response',~~
161		~~- 'mbfr_timestamp_id', dirname( __FILE__ ) . '/sql/mbfr_timestamp_id_index.sql', true )~~
162		~~- );~~
163		-
	146	+ $updater->addExtensionTable( 'moodbar_feedback_response', $dir . 'moodbar_feedback_response.sql' );
	147	+
	148	+ $updater->addExtensionIndex( 'moodbar_feedback_response', 'mbfr_timestamp_id', $dir . 'mbfr_timestamp_id_index.sql' );
	149	+
164	150	return true;
165	151	}
166	152
167	153	/**
168	154	* Gets the MoodBar testing bucket that a user is in.
169		~~- * @param $user The user to check~~
	155	+ * @param $user User The user to check
170	156	* @return array of bucket names
171	157	*/
172	158	public static function getUserBuckets( $user ) {
Index: trunk/extensions/MoodBar/sql/MoodBar.sql
—	—	@@ -1,20 +1,20 @@
2	2	CREATE TABLE /_/moodbar_feedback (
3	3	mbf_id int unsigned NOT NULL PRIMARY KEY auto_increment, -- Primary key
4	4	mbf_type varchar(32) binary NOT NULL, -- Type of feedback
5		-
	5	+
6	6	-- User who provided the feedback
7	7	mbf_user_id int unsigned NOT NULL, -- User ID, or zero
8	8	mbf_user_ip varchar(255) binary NULL, -- If anonymous, user's IP address
9	9	mbf_user_editcount int unsigned NOT NULL, -- Edit count of the user
10		-
	10	+
11	11	-- Page where the feedback was received
12	12	-- Nullable.
13	13	mbf_namespace int,
14	14	mbf_title varchar(255) binary,
15		-
	15	+
16	16	-- The feedback itself
17	17	mbf_comment varchar(255) binary,
18		-
	18	+
19	19	-- Options and context
20	20	-- Whether or not the feedback item is hidden
21	21	-- 0 = No; 255 = Yes (other values reserved for partial hiding)
Index: trunk/extensions/MoodBar/sql/moodbar_feedback_response.sql
—	—	@@ -1,18 +1,18 @@
2	2	CREATE TABLE /_/moodbar_feedback_response (
3	3	mbfr_id int unsigned NOT NULL PRIMARY KEY auto_increment, -- Primary key
4		-
	4	+
5	5	mbfr_mbf_id int unsigned NOT NULL, -- Primary key of moodbar_feedback table
6		-
	6	+
7	7	-- User who provided the response
8	8	mbfr_user_id int unsigned NOT NULL, -- User ID, or zero
9	9	mbfr_user_ip varbinary(40) NULL, -- If anonymous, user's IP address
10		-
	10	+
11	11	mbfr_commenter_editcount int unsigned NOT NULL, -- number of edit for the user who writes the feedback
12	12	mbfr_user_editcount int unsigned NOT NULL, -- number of edit for the responder
13		-
	13	+
14	14	-- The response itself
15	15	mbfr_response_text text NOT NULL,
16		-
	16	+
17	17	-- Options and context
18	18	mbfr_timestamp varchar(14) binary NOT NULL, -- When response was received
19	19	mbfr_anonymous tinyint unsigned NOT NULL DEFAULT 0, -- Anonymity
—	—	@@ -23,3 +23,4 @@
24	24	) /$wgDBTableOptions/;
25	25
26	26	CREATE INDEX /i/mbfr_mbf_id ON /_/moodbar_feedback_response (mbfr_mbf_id);
	27	+CREATE INDEX /i/mbfr_timestamp_id ON /_/moodbar_feedback_response (mbfr_mbf_id, mbfr_timestamp, mbfr_id);

Follow-up revisions

Revision	Commit summary	Author	Date
r107566	Follow-up r107522: Moar whitespace and indentation updates.	siebrand	12:59, 29 December 2011
r107611	Fix for r107522: incorrect index name in update hook	tstarling	01:10, 30 December 2011

Comments

#Comment by Nikerabbit (talk | contribs) 06:27, 29 December 2011

+								array( 'mbf_id = mbfr_mbf_id',
+										'mbfr_id' => intval( $item ),
+										'mbf_user_id' => $User->getId() ),

The previous indentation was better.

"$dir/Fooname.sql" is shorter and maybe easier to read.

#Comment by Reedy (talk | contribs) 11:43, 29 December 2011

It'd be more consistent with other extensions too

Status & tagging log

01:12, 30 December 2011 Tim Starling (talk | contribs) changed the status of r107522 [removed: new added: resolved]