r90790 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r90789‎ \| r90790 \| r90791 >
Date:	19:36, 25 June 2011
Author:	ashley
Status:	deferred
Tags:
Comment:	SocialProfile: in UserGifts/SpecialGiftManager.php: coding style tweaks wrap some vars in intval() for sanity's sake add/tweak comments use $this->getTitle() to build the URLs instead of hacky DIY stuff; we require MW 1.16 anyway, so it should be perfectly safe
Modified paths:	/trunk/extensions/SocialProfile/UserGifts/SpecialGiftManager.php (modified) (history)

Diff [purge]

Index: trunk/extensions/SocialProfile/UserGifts/SpecialGiftManager.php
—	—	@@ -15,35 +15,61 @@
16	16	* @param $par Mixed: parameter passed to the page or null
17	17	*/
18	18	public function execute( $par ) {
19		~~- global $wgUser, $wgOut, $wgRequest, $wgScriptPath, $wgUserGiftsScripts;~~
	19	+ global $wgUser, $wgOut, $wgRequest, $wgUserGiftsScripts;
20	20
21	21	$wgOut->setPageTitle( wfMsg( 'giftmanager' ) );
22	22
	23	+ // Make sure that the user is logged in and that they can use this
	24	+ // special page
23	25	if ( $wgUser->isAnon() \|\| !$this->canUserManage() ) {
24	26	throw new ErrorPageError( 'error', 'badaccess' );
25	27	}
26	28
27		~~- $wgOut->addStyle( $wgUserGiftsScripts . '/UserGifts.css' );~~
	29	+ // Add CSS
	30	+ $wgOut->addExtensionStyle( $wgUserGiftsScripts . '/UserGifts.css' );
28	31
29	32	if ( count( $_POST ) ) {
30	33	if ( !( $_POST['id'] ) ) {
31		~~- $gift_id = Gifts::addGift( $_POST['gift_name'], $_POST['gift_description'], $_POST['access'] );~~
32		~~- $wgOut->addHTML( '<span class="view-status">' . wfMsg( 'giftmanager-giftcreated' ) . '</span><br /><br />' );~~
	34	+ $giftId = Gifts::addGift(
	35	+ $_POST['gift_name'],
	36	+ $_POST['gift_description'],
	37	+ intval( $_POST['access'] )
	38	+ );
	39	+ $wgOut->addHTML(
	40	+ '<span class="view-status">' .
	41	+ wfMsg( 'giftmanager-giftcreated' ) .
	42	+ '</span><br /><br />'
	43	+ );
33	44	} else {
34		~~- $gift_id = $_POST['id'];~~
35		~~- Gifts::updateGift( $gift_id, $_POST['gift_name'], $_POST['gift_description'], $_POST['access'] );~~
36		~~- $wgOut->addHTML( '<span class="view-status">' . wfMsg( 'giftmanager-giftsaved' ) . '</span><br /><br />' );~~
	45	+ $giftId = intval( $_POST['id'] );
	46	+ Gifts::updateGift(
	47	+ $giftId,
	48	+ $_POST['gift_name'],
	49	+ $_POST['gift_description'],
	50	+ intval( $_POST['access'] )
	51	+ );
	52	+ $wgOut->addHTML(
	53	+ '<span class="view-status">' .
	54	+ wfMsg( 'giftmanager-giftsaved' ) .
	55	+ '</span><br /><br />'
	56	+ );
37	57	}
38	58
39		~~- $wgOut->addHTML( $this->displayForm( $gift_id ) );~~
	59	+ $wgOut->addHTML( $this->displayForm( $giftId ) );
40	60	} else {
41		~~- $gift_id = $wgRequest->getVal( 'id' );~~
42		~~- if ( $gift_id \|\| $wgRequest->getVal( 'method' ) == 'edit' ) {~~
43		~~- $wgOut->addHTML( $this->displayForm( $gift_id ) );~~
	61	+ $giftId = $wgRequest->getInt( 'id' );
	62	+ if ( $giftId \|\| $wgRequest->getVal( 'method' ) == 'edit' ) {
	63	+ $wgOut->addHTML( $this->displayForm( $giftId ) );
44	64	} else {
	65	+ // If the user is allowed to create new gifts, show the
	66	+ // "add a gift" link to them
45	67	if ( $this->canUserCreateGift() ) {
46		~~- $wgOut->addHTML( '<div><b><a href="' . $wgScriptPath . '/index.php?title=Special:GiftManager&method=edit">'~~
47		~~- . wfMsg( 'giftmanager-addgift' ) . '</a></b></div>' );~~
	68	+ $wgOut->addHTML(
	69	+ '<div><b><a href="' .
	70	+ $this->getTitle()->escapeFullURL( 'method=edit' ) .
	71	+ '">' . wfMsg( 'giftmanager-addgift' ) .
	72	+ '</a></b></div>'
	73	+ );
48	74	}
49	75	$wgOut->addHTML( $this->displayGiftList() );
50	76	}
—	—	@@ -66,7 +92,11 @@
67	93	return true;
68	94	}
69	95
70		~~- if ( $wgUser->isAllowed( 'giftadmin' ) \|\| in_array( 'giftadmin', $wgUser->getGroups() ) ) {~~
	96	+ if (
	97	+ $wgUser->isAllowed( 'giftadmin' ) \|\|
	98	+ in_array( 'giftadmin', $wgUser->getGroups() )
	99	+ )
	100	+ {
71	101	return true;
72	102	}
73	103
—	—	@@ -85,7 +115,11 @@
86	116	return false;
87	117	}
88	118
89		~~- if ( $wgUser->isAllowed( 'giftadmin' ) \|\| in_array( 'giftadmin', $wgUser->getGroups() ) ) {~~
	119	+ if (
	120	+ $wgUser->isAllowed( 'giftadmin' ) \|\|
	121	+ in_array( 'giftadmin', $wgUser->getGroups() )
	122	+ )
	123	+ {
90	124	return true;
91	125	}
92	126
—	—	@@ -105,42 +139,69 @@
106	140	return false;
107	141	}
108	142
109		~~- $created_count = Gifts::getCustomCreatedGiftCount( $wgUser->getID() );~~
110		~~- if ( $wgUser->isAllowed( 'giftadmin' ) \|\| in_array( 'giftadmin', ( $wgUser->getGroups() ) ) \|\| ( $wgMaxCustomUserGiftCount > 0 && $created_count < $wgMaxCustomUserGiftCount ) ) {~~
	143	+ $createdCount = Gifts::getCustomCreatedGiftCount( $wgUser->getID() );
	144	+ if (
	145	+ $wgUser->isAllowed( 'giftadmin' ) \|\|
	146	+ in_array( 'giftadmin', ( $wgUser->getGroups() ) ) \|\|
	147	+ ( $wgMaxCustomUserGiftCount > 0 && $createdCount < $wgMaxCustomUserGiftCount )
	148	+ )
	149	+ {
111	150	return true;
112	151	} else {
113	152	return false;
114	153	}
115	154	}
116	155
	156	+ /**
	157	+ * Display the text list of all existing gifts and a delete link to users
	158	+ * who are allowed to delete gifts.
	159	+ *
	160	+ * @return String: HTML
	161	+ */
117	162	function displayGiftList() {
118		~~- global $wgScriptPath;~~
119	163	$output = ''; // Prevent E_NOTICE
120	164	$page = 0;
121	165	$per_page = 10;
122	166	$gifts = Gifts::getManagedGiftList( $per_page, $page );
123	167	if ( $gifts ) {
124	168	foreach ( $gifts as $gift ) {
	169	+ $deleteLink = '';
	170	+ if ( $this->canUserDelete() ) {
	171	+ $deleteLink = '<a href="' .
	172	+ SpecialPage::getTitleFor( 'RemoveMasterGift' )->escapeFullURL( "gift_id={$gift['id']}" ) .
	173	+ '" style="font-size:10px; color:red;">' .
	174	+ wfMsg( 'delete' ) . '</a>';
	175	+ }
	176	+
125	177	$output .= '<div class="Item">
126		~~- <a href="' . $wgScriptPath . '/index.php?title=Special:GiftManager&id=' . $gift['id'] . '">' . $gift['gift_name'] . '</a> ' .~~
127		- ( ( $this->canUserDelete() ) ? '<a href="' . SpecialPage::getTitleFor( 'RemoveMasterGift' )->escapeFulLURL( "gift_id={$gift["id"]}" ) . '" style="font-size:10px; color:red;">' . wfMsg( 'delete' ) . '</a>' : '' )
128		~~- . "</div>\n";~~
	178	+ <a href="' . $this->getTitle()->escapeFullURL( "id={$gift['id']}" ) . '">' .
	179	+ $gift['gift_name'] . '</a> ' .
	180	+ $deleteLink . "</div>\n";
129	181	}
130	182	}
131	183	return '<div id="views">' . $output . '</div>';
132	184	}
133	185
134	186	function displayForm( $gift_id ) {
135		~~- global $wgUser, $wgOut, $wgScriptPath;~~
	187	+ global $wgUser;
136	188
137	189	if ( !$gift_id && !$this->canUserCreateGift() ) {
138	190	return $this->displayGiftList();
139	191	}
140		~~- $form = '<div><b><a href="' . $wgScriptPath . '/index.php?title=Special:GiftManager">' . wfMsg( 'giftmanager-view' ) . '</a></b></div>';~~
141	192
	193	+ $form = '<div><b><a href="' . $this->getTitle()->escapeFullURL() .
	194	+ '">' . wfMsg( 'giftmanager-view' ) . '</a></b></div>';
	195	+
142	196	if ( $gift_id ) {
143	197	$gift = Gifts::getGift( $gift_id );
144		~~- if ( $wgUser->getID() != $gift['creator_user_id'] && ( !in_array( 'giftadmin', $wgUser->getGroups() ) && !$wgUser->isAllowed( 'delete' ) ) ) {~~
	198	+ if (
	199	+ $wgUser->getID() != $gift['creator_user_id'] &&
	200	+ (
	201	+ !in_array( 'giftadmin', $wgUser->getGroups() ) &&
	202	+ !$wgUser->isAllowed( 'delete' )
	203	+ )
	204	+ )
	205	+ {
145	206	throw new ErrorPageError( 'error', 'badaccess' );
146	207	}
147	208	}
—	—	@@ -149,20 +210,27 @@
150	211	$form .= '<table border="0" cellpadding="5" cellspacing="0" width="500">';
151	212	$form .= '<tr>
152	213	<td width="200" class="view-form">' . wfMsg( 'g-gift-name' ) . '</td>
153		~~- <td width="695"><input type="text" size="45" class="createbox" name="gift_name" value="' . ( isset( $gift['gift_name'] ) ? $gift['gift_name'] : '' ) . '"/></td>~~
	214	+ <td width="695"><input type="text" size="45" class="createbox" name="gift_name" value="' .
	215	+ ( isset( $gift['gift_name'] ) ? $gift['gift_name'] : '' ) . '"/></td>
154	216	</tr>
155	217	<tr>
156	218	<td width="200" class="view-form" valign="top">' . wfMsg( 'giftmanager-description' ) . '</td>
157		~~- <td width="695"><textarea class="createbox" name="gift_description" rows="2" cols="30">' . ( isset( $gift['gift_description'] ) ? $gift['gift_description'] : '' ) . '</textarea></td>~~
	219	+ <td width="695"><textarea class="createbox" name="gift_description" rows="2" cols="30">' .
	220	+ ( isset( $gift['gift_description'] ) ? $gift['gift_description'] : '' ) . '</textarea></td>
158	221	</tr>';
159	222	if ( $gift_id ) {
160	223	$creator = Title::makeTitle( NS_USER, $gift['creator_user_name'] );
161	224	$form .= '<tr>
162		~~- <td class="view-form">' . wfMsgExt( 'g-created-by', 'parsemag', $gift['creator_user_name'] ) . '</td>~~
163		~~- <td><a href="' . $creator->escapeFullURL() . '">' . $gift['creator_user_name'] . '</a></td>~~
	225	+ <td class="view-form">' .
	226	+ wfMsgExt( 'g-created-by', 'parsemag', $gift['creator_user_name'] ) .
	227	+ '</td>
	228	+ <td><a href="' . $creator->escapeFullURL() . '">' .
	229	+ $gift['creator_user_name'] . '</a></td>
164	230	</tr>';
165	231	}
166		~~- global $wgUploadPath;~~
	232	+
	233	+ // If the user isn't in the gift admin group, they can only create
	234	+ // private gifts
167	235	if ( !in_array( 'giftadmin', $wgUser->getGroups() ) ) {
168	236	$form .= '<input type="hidden" name="access" value="1" />';
169	237	} else {
—	—	@@ -171,20 +239,29 @@
172	240	<td class="view-form">' . wfMsg( 'giftmanager-access' ) . '</td>
173	241	<td>
174	242	<select name="access">
175		~~- <option value="0"' . ( ( $gift['access'] == 0 ) ? ' selected="selected"' : '' ) . '>' . wfMsg( 'giftmanager-public' ) . '</option>~~
176		~~- <option value="1"' . ( ( $gift['access'] == 1 ) ? ' selected="selected"' : '' ) . '>' . wfMsg( 'giftmanager-private' ) . '</option>~~
	243	+ <option value="0"' . ( ( $gift['access'] == 0 ) ? ' selected="selected"' : '' ) . '>' .
	244	+ wfMsg( 'giftmanager-public' ) .
	245	+ '</option>
	246	+ <option value="1"' . ( ( $gift['access'] == 1 ) ? ' selected="selected"' : '' ) . '>' .
	247	+ wfMsg( 'giftmanager-private' ) .
	248	+ '</option>
177	249	</select>
178	250	</td>
179	251	</tr>';
180	252	}
181	253
182	254	if ( $gift_id ) {
183		~~- $gift_image = '<img src="' . $wgUploadPath . '/awards/' . Gifts::getGiftImage( $gift_id, 'l' ) . '" border="0" alt="' . wfMsg( 'g-gift' ) . '" />';~~
	255	+ global $wgUploadPath;
	256	+ $gml = SpecialPage::getTitleFor( 'GiftManagerLogo' );
	257	+ $gift_image = '<img src="' . $wgUploadPath . '/awards/' .
	258	+ Gifts::getGiftImage( $gift_id, 'l' ) . '" border="0" alt="' .
	259	+ wfMsg( 'g-gift' ) . '" />';
184	260	$form .= '<tr>
185	261	<td width="200" class="view-form" valign="top">' . wfMsg( 'giftmanager-giftimage' ) . '</td>
186	262	<td width="695">' . $gift_image .
187	263	'<p>
188		~~- <a href="' . $wgScriptPath . '/index.php?title=Special:GiftManagerLogo&gift_id=' . $gift_id . '">' . wfMsg( 'giftmanager-image' ) . '</a>~~
	264	+ <a href="' . $gml->escapeFullURL( 'gift_id=' . $gift_id ) . '">' .
	265	+ wfMsg( 'giftmanager-image' ) . '</a>
189	266	</td>
190	267	</tr>';
191	268	}

Status & tagging log

18:17, 26 June 2011 Reedy (talk | contribs) changed the status of r90790 [removed: new added: deferred]