r85503 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r85502‎ \| r85503 \| r85504 >
Date:	01:25, 6 April 2011
Author:	ashley
Status:	deferred
Tags:
Comment:	SocialProfile: in SpecialGiveGift.php: documentation tweaks renamed badly_named_variables to betterNamedVariables more $wgRequest->getVal() to $wgRequest->getInt() swaps broke some long lines fixed indentation marked some batshit insane code as such for further review
Modified paths:	/trunk/extensions/SocialProfile/UserGifts/SpecialGiveGift.php (modified) (history)

Diff [purge]

Index: trunk/extensions/SocialProfile/UserGifts/SpecialGiveGift.php
—	—	@@ -1,4 +1,10 @@
2	2	<?php
	3	+/**
	4	+ * Special:GiveGift -- a special page for sending out user-to-user gifts
	5	+ *
	6	+ * @file
	7	+ * @ingroup Extensions
	8	+ */
3	9
4	10	class GiveGift extends SpecialPage {
5	11
—	—	@@ -9,7 +15,6 @@
10	16	parent::__construct( 'GiveGift' );
11	17	}
12	18
13		-
14	19	/**
15	20	* Show the special page
16	21	*
—	—	@@ -23,16 +28,16 @@
24	29	$wgOut->addScriptFile( $wgUserGiftsScripts . '/UserGifts.js' );
25	30	$wgOut->addExtensionStyle( $wgUserGiftsScripts . '/UserGifts.css' );
26	31
27		~~- $usertitle = Title::newFromDBkey( $wgRequest->getVal( 'user' ) );~~
28		~~- if ( !$usertitle ) {~~
	32	+ $userTitle = Title::newFromDBkey( $wgRequest->getVal( 'user' ) );
	33	+ if ( !$userTitle ) {
29	34	$wgOut->addHTML( $this->displayFormNoUser() );
30	35	return false;
31	36	}
32	37
33	38	$user_title = Title::makeTitle( NS_USER, $wgRequest->getVal( 'user' ) );
34		~~- $this->user_name_to = $usertitle->getText();~~
	39	+ $this->user_name_to = $userTitle->getText();
35	40	$this->user_id_to = User::idFromName( $this->user_name_to );
36		~~- $gift_id = $wgRequest->getVal( 'gift_id' );~~
	41	+ $giftId = $wgRequest->getInt( 'gift_id' );
37	42	$out = '';
38	43
39	44	if ( $wgUser->getID() === $this->user_id_to ) {
—	—	@@ -51,16 +56,14 @@
52	57	$out .= wfMsg( 'g-error-message-login' );
53	58	$wgOut->addHTML( $out );
54	59	} else {
55		-
56	60	$gift = new UserGifts( $wgUser->getName() );
57	61
58	62	if ( $wgRequest->wasPosted() && $_SESSION['alreadysubmitted'] == false ) {
59		-
60	63	$_SESSION['alreadysubmitted'] = true;
61	64
62	65	$ug_gift_id = $gift->sendGift(
63	66	$this->user_name_to,
64		~~- $wgRequest->getVal( 'gift_id' ),~~
	67	+ $wgRequest->getInt( 'gift_id' ),
65	68	0,
66	69	$wgRequest->getVal( 'message' )
67	70	);
—	—	@@ -72,12 +75,12 @@
73	76	$data = $wgMemc->get( $key );
74	77
75	78	// check to see if this type of gift is in the unique list
76		~~- $last_unique_gifts = $data;~~
	79	+ $lastUniqueGifts = $data;
77	80	$found = 1;
78	81
79		~~- if ( is_array( $last_unique_gifts ) ) {~~
80		~~- foreach ( $last_unique_gifts as $last_unique_gift ) {~~
81		~~- if ( $wgRequest->getVal( 'gift_id' ) == $last_unique_gift['gift_id'] ) {~~
	82	+ if ( is_array( $lastUniqueGifts ) ) {
	83	+ foreach ( $lastUniqueGifts as $lastUniqueGift ) {
	84	+ if ( $wgRequest->getInt( 'gift_id' ) == $lastUniqueGift['gift_id'] ) {
82	85	$found = 0;
83	86	}
84	87	}
—	—	@@ -85,38 +88,44 @@
86	89
87	90	if ( $found ) {
88	91	// add new unique to array
89		~~- $last_unique_gifts[] = array(~~
	92	+ $lastUniqueGifts[] = array(
90	93	'id' => $ug_gift_id,
91		~~- 'gift_id' => $wgRequest->getVal( 'gift_id' )~~
	94	+ 'gift_id' => $wgRequest->getInt( 'gift_id' )
92	95	);
93	96
94	97	// remove oldest value
95		~~- if ( count( $last_unique_gifts ) > 4 ) {~~
96		~~- array_shift( $last_unique_gifts );~~
	98	+ if ( count( $lastUniqueGifts ) > 4 ) {
	99	+ array_shift( $lastUniqueGifts );
97	100	}
98	101
99	102	// reset the cache
100		~~- $wgMemc->set( $key, $last_unique_gifts );~~
	103	+ $wgMemc->set( $key, $lastUniqueGifts );
101	104	}
102	105
103	106	$sent_gift = UserGifts::getUserGift( $ug_gift_id );
104		~~- $gift_image = '<img src="' . $wgUploadPath . '/awards/' . Gifts::getGiftImage( $sent_gift['gift_id'], 'l' ) . '" border="0" alt="" />';~~
	107	+ $gift_image = '<img src="' . $wgUploadPath . '/awards/' .
	108	+ Gifts::getGiftImage( $sent_gift['gift_id'], 'l' ) .
	109	+ '" border="0" alt="" />';
105	110
106	111	$output .= $wgOut->setPageTitle( wfMsg( 'g-sent-title', $this->user_name_to ) );
107	112
108	113	$output .= '<div class="back-links">
109		~~- <a href="' . $user_title->escapeFullURL() . '">' . wfMsg( 'g-back-link', $this->user_name_to ) . '</a>~~
	114	+ <a href="' . $user_title->escapeFullURL() . '">' .
	115	+ wfMsg( 'g-back-link', $this->user_name_to ) .
	116	+ '</a>
110	117	</div>
111		~~- <div class="g-message">'~~
112		~~- . wfMsg( 'g-sent-message', $this->user_name_to ) .~~
	118	+ <div class="g-message">' .
	119	+ wfMsg( 'g-sent-message', $this->user_name_to ) .
113	120	'</div>
114		~~- <div class="g-container">'~~
115		~~- . $gift_image .~~
	121	+ <div class="g-container">' .
	122	+ $gift_image .
116	123	'<div class="g-title">' . $sent_gift['name'] . '</div>';
117		~~- if ( $sent_gift['message'] ) {~~
118		~~- $output .= '<div class="g-user-message">' . $sent_gift['message'] . '</div>';~~
119		~~- }~~
120		~~- $output .= '</div>~~
	124	+ if ( $sent_gift['message'] ) {
	125	+ $output .= '<div class="g-user-message">' .
	126	+ $sent_gift['message'] .
	127	+ '</div>';
	128	+ }
	129	+ $output .= '</div>
121	130	<div class="cleared"></div>
122	131	<div class="g-buttons">
123	132	<input type="button" class="site-button" value="' . wfMsg( 'g-main-page' ) . '" size="20" onclick="window.location=\'index.php?title=' . wfMsgForContent( 'mainpage' ) . '\'" />
—	—	@@ -127,7 +136,7 @@
128	137	} else {
129	138	$_SESSION['alreadysubmitted'] = false;
130	139
131		~~- if ( $gift_id ) {~~
	140	+ if ( $giftId ) {
132	141	$wgOut->addHTML( $this->displayFormSingle() );
133	142	} else {
134	143	$wgOut->addHTML( $this->displayFormAll() );
—	—	@@ -136,18 +145,24 @@
137	146	}
138	147	}
139	148
	149	+ /**
	150	+ * Display the form for sending out a single gift.
	151	+ * Relies on the gift_id URL parameter and bails out if it's not there.
	152	+ *
	153	+ * @return String: HTML
	154	+ */
140	155	function displayFormSingle() {
141	156	global $wgUser, $wgOut, $wgRequest, $wgUploadPath;
142	157
143		~~- $gift_id = $wgRequest->getVal( 'gift_id' );~~
	158	+ $giftId = $wgRequest->getInt( 'gift_id' );
144	159
145		~~- if ( !$gift_id \|\| !is_numeric( $gift_id ) ) {~~
	160	+ if ( !$giftId \|\| !is_numeric( $giftId ) ) {
146	161	$wgOut->setPageTitle( wfMsg( 'g-error-title' ) );
147	162	$wgOut->addHTML( wfMsg( 'g-error-message-invalid-link' ) );
148	163	return false;
149	164	}
150	165
151		~~- $gift = Gifts::getGift( $gift_id );~~
	166	+ $gift = Gifts::getGift( $giftId );
152	167
153	168	if ( empty( $gift ) ) {
154	169	return false;
—	—	@@ -159,28 +174,35 @@
160	175
161	176	// Safe titles
162	177	$user = Title::makeTitle( NS_USER, $this->user_name_to );
163		~~- $give_gift_link = SpecialPage::getTitleFor( 'GiveGift' );~~
	178	+ $giveGiftLink = SpecialPage::getTitleFor( 'GiveGift' );
164	179
165	180	$wgOut->setPageTitle( wfMsg( 'g-give-to-user-title', $gift['gift_name'], $this->user_name_to ) );
166	181
167		~~- $gift_image = "<img id=\"gift_image_{$gift['gift_id']}\" src=\"{$wgUploadPath}/awards/" . Gifts::getGiftImage( $gift['gift_id'], 'l' ) . '" border="0" alt="" />';~~
	182	+ $gift_image = "<img id=\"gift_image_{$gift['gift_id']}\" src=\"{$wgUploadPath}/awards/" .
	183	+ Gifts::getGiftImage( $gift['gift_id'], 'l' ) .
	184	+ '" border="0" alt="" />';
168	185
169	186	$output = '<form action="" method="post" enctype="multipart/form-data" name="gift">
170		~~- <div class="g-message">'~~
171		~~- . wfMsg( 'g-give-to-user-message', $this->user_name_to, $give_gift_link->escapeFullURL( 'user=' . $this->user_name_to ) ) .~~
172		~~- "</div>~~
	187	+ <div class="g-message">' .
	188	+ wfMsg(
	189	+ 'g-give-to-user-message',
	190	+ $this->user_name_to,
	191	+ $giveGiftLink->escapeFullURL( 'user=' . $this->user_name_to )
	192	+ ) . "</div>
173	193	<div id=\"give_gift_{$gift['gift_id']}\" class=\"g-container\">
174	194	{$gift_image}
175	195	<div class=\"g-title\">{$gift['gift_name']}</div>";
176		~~- if ( $gift['gift_description'] ) {~~
177		~~- $output .= '<div class="g-describe">' . $gift['gift_description'] . '</div>';~~
178		~~- }~~
179		~~- $output .= '</div>~~
	196	+ if ( $gift['gift_description'] ) {
	197	+ $output .= '<div class="g-describe">' .
	198	+ $gift['gift_description'] .
	199	+ '</div>';
	200	+ }
	201	+ $output .= '</div>
180	202	<div class="cleared"></div>
181	203	<div class="g-add-message">' . wfMsg( 'g-add-message' ) . '</div>
182	204	<textarea name="message" id="message" rows="4" cols="50"></textarea>
183	205	<div class="g-buttons">
184		~~- <input type="hidden" name="gift_id" value="' . $gift_id . '" />~~
	206	+ <input type="hidden" name="gift_id" value="' . $giftId . '" />
185	207	<input type="hidden" name="user_name" value="' . addslashes( $this->user_name_to ) . '" />
186	208	<input type="button" class="site-button" value="' . wfMsg( 'g-send-gift' ) . '" size="20" onclick="document.gift.submit()" />
187	209	<input type="button" class="site-button" value="' . wfMsg( 'g-cancel' ) . '" size="20" onclick="history.go(-1)" />
—	—	@@ -190,35 +212,61 @@
191	213	return $output;
192	214	}
193	215
	216	+ /**
	217	+ * Display the form for giving out a gift to a user when there was no user
	218	+ * parameter in the URL.
	219	+ *
	220	+ * @return String: HTML
	221	+ */
194	222	function displayFormNoUser() {
195	223	global $wgUser, $wgOut, $wgRequest, $wgFriendingEnabled;
196	224
197	225	$output = $wgOut->setPageTitle( wfMsg( 'g-give-no-user-title' ) );
198	226
	227	+ // @todo FIXME: $wgRequest->getVal()...seriously?
	228	+ // Seems that this should use the proper MW function (maybe
	229	+ // $this->getTitle()->getFullURL() or something) instead.
	230	+ // Maybe that's the reason why I (and other people) frequently have
	231	+ // problems with this special page.
	232	+ // --Jack Phoenix <jack@countervandalism.net>, 6 April 2011
199	233	$output .= '<form action="" method="get" enctype="multipart/form-data" name="gift">
200	234	<input type="hidden" name="title" value="' . $wgRequest->getVal( 'title' ) . '" />
201		~~- <div class="g-message">' . wfMsg( 'g-give-no-user-message' ) . '</div>~~
	235	+ <div class="g-message">' .
	236	+ wfMsg( 'g-give-no-user-message' ) .
	237	+ '</div>
202	238	<div class="g-give-container">';
203	239
	240	+ // If friending is enabled, build a dropdown menu of the user's
	241	+ // friends
204	242	if ( $wgFriendingEnabled ) {
205	243	$rel = new UserRelationship( $wgUser->getName() );
206	244	$friends = $rel->getRelationshipList( 1 );
207	245
208	246	if ( $friends ) {
209		~~- $output .= '<div class="g-give-title">' . wfMsg( 'g-give-list-friends-title' ) . '</div>~~
	247	+ $output .= '<div class="g-give-title">' .
	248	+ wfMsg( 'g-give-list-friends-title' ) .
	249	+ '</div>
210	250	<div class="g-gift-select">
211	251	<select onchange="javascript:chooseFriend(this.value)">
212		~~- <option value="#" selected="selected">' . wfMsg( 'g-select-a-friend' ) . '</option>';~~
	252	+ <option value="#" selected="selected">' .
	253	+ wfMsg( 'g-select-a-friend' ) .
	254	+ '</option>';
213	255	foreach ( $friends as $friend ) {
214		~~- $output .= '<option value="' . urlencode( $friend['user_name'] ) . '">' . $friend['user_name'] . '</option>';~~
	256	+ $output .= '<option value="' . urlencode( $friend['user_name'] ) . '">' .
	257	+ $friend['user_name'] .
	258	+ '</option>';
215	259	}
216	260	$output .= '</select>
217	261	</div>
218		~~- <div class="g-give-separator">' . wfMsg( 'g-give-separator' ) . '</div>';~~
	262	+ <div class="g-give-separator">' .
	263	+ wfMsg( 'g-give-separator' ) .
	264	+ '</div>';
219	265	}
220	266	}
221	267
222		~~- $output .= '<div class="g-give-title">' . wfMsg( 'g-give-enter-friend-title' ) . '</div>~~
	268	+ $output .= '<div class="g-give-title">' .
	269	+ wfMsg( 'g-give-enter-friend-title' ) .
	270	+ '</div>
223	271	<div class="g-give-textbox">
224	272	<input type="text" width="85" name="user" value="" />
225	273	<input class="site-button" type="button" value="' . wfMsg( 'g-give-gift' ) . '" onclick="document.gift.submit()" />
—	—	@@ -233,7 +281,7 @@
234	282	global $wgUser, $wgOut, $wgRequest, $wgGiveGiftPerRow, $wgUploadPath;
235	283	$user = Title::makeTitle( NS_USER, $this->user_name_to );
236	284
237		~~- $page = $wgRequest->getVal( 'page' );~~
	285	+ $page = $wgRequest->getInt( 'page' );
238	286	if ( !$page \|\| !is_numeric( $page ) ) {
239	287	$page = 1;
240	288	}
—	—	@@ -252,25 +300,29 @@
253	301	$wgOut->setPageTitle( wfMsg( 'g-give-all-title', $this->user_name_to ) );
254	302
255	303	$output .= '<div class="back-links">
256		~~- <a href="' . $user->escapeFullURL() . '">' . wfMsg( 'g-back-link', $this->user_name_to ) . '</a>~~
	304	+ <a href="' . $user->escapeFullURL() . '">' .
	305	+ wfMsg( 'g-back-link', $this->user_name_to ) .
	306	+ '</a>
257	307	</div>
258		~~- <div class="g-message">~~
259		~~- ' . wfMsg( 'g-give-all', $this->user_name_to ) . '~~
260		~~- </div>~~
	308	+ <div class="g-message">' .
	309	+ wfMsg( 'g-give-all', $this->user_name_to ) .
	310	+ '</div>
261	311	<form action="" method="post" enctype="multipart/form-data" name="gift">';
262	312
263	313	$x = 1;
264	314
265	315	foreach ( $gifts as $gift ) {
266		~~- $gift_image = "<img id=\"gift_image_{$gift['id']}\" src=\"{$wgUploadPath}/awards/" . Gifts::getGiftImage( $gift['id'], 'l' ) . '" border="0" alt="" />';~~
	316	+ $gift_image = "<img id=\"gift_image_{$gift['id']}\" src=\"{$wgUploadPath}/awards/" .
	317	+ Gifts::getGiftImage( $gift['id'], 'l' ) .
	318	+ '" border="0" alt="" />';
267	319
268	320	$output .= "<div onclick=\"selectGift({$gift['id']})\" onmouseover=\"highlightGift({$gift['id']})\" onmouseout=\"unHighlightGift({$gift['id']})\" id=\"give_gift_{$gift['id']}\" class=\"g-give-all\">
269	321	{$gift_image}
270	322	<div class=\"g-title g-blue\">{$gift['gift_name']}</div>";
271		~~- if ( $gift['gift_description'] ) {~~
272		~~- $output .= "<div class=\"g-describe\">{$gift['gift_description']}</div>";~~
273		~~- }~~
274		~~- $output .= '<div class="cleared"></div>~~
	323	+ if ( $gift['gift_description'] ) {
	324	+ $output .= "<div class=\"g-describe\">{$gift['gift_description']}</div>";
	325	+ }
	326	+ $output .= '<div class="cleared"></div>
275	327	</div>';
276	328	if ( $x == count( $gifts ) \|\| $x != 1 && $x % $per_row == 0 ) {
277	329	$output .= '<div class="cleared"></div>';
—	—	@@ -281,14 +333,14 @@
282	334	/**
283	335	* Build next/prev nav
284	336	*/
285		~~- $give_gift_link = SpecialPage::getTitleFor( 'GiveGift' );~~
	337	+ $giveGiftLink = SpecialPage::getTitleFor( 'GiveGift' );
286	338
287	339	$numofpages = $total / $per_page;
288	340	$user_safe = urlencode( $user->getText() );
289	341	if ( $numofpages > 1 ) {
290	342	$output .= '<div class="page-nav">';
291	343	if ( $page > 1 ) {
292		~~- $output .= '<a href="' . $give_gift_link->escapeFullURL( 'user=' . $user_safe . '&page=' . ( $page - 1 ) ) . '">' . wfMsg( 'g-previous' ) . '</a> ';~~
	344	+ $output .= '<a href="' . $giveGiftLink->escapeFullURL( 'user=' . $user_safe . '&page=' . ( $page - 1 ) ) . '">' . wfMsg( 'g-previous' ) . '</a> ';
293	345	}
294	346
295	347	if ( ( $total % $per_page ) != 0 ) {
—	—	@@ -301,12 +353,12 @@
302	354	if ( $i == $page ) {
303	355	$output .= ( $i . ' ' );
304	356	} else {
305		~~- $output .= '<a href="' . $give_gift_link->escapeFullURL( 'user=' . $user_safe . '&page=' . $i ) . "\">$i</a> ";~~
	357	+ $output .= '<a href="' . $giveGiftLink->escapeFullURL( 'user=' . $user_safe . '&page=' . $i ) . "\">$i</a> ";
306	358	}
307	359	}
308	360
309	361	if ( ( $total - ( $per_page * $page ) ) > 0 ) {
310		~~- $output .= ' <a href="' . $give_gift_link->escapeFullURL( 'user=' . $user_safe . '&page=' . ( $page + 1 ) ) . '">' . wfMsg( 'g-next' ) . '</a>';~~
	362	+ $output .= ' <a href="' . $giveGiftLink->escapeFullURL( 'user=' . $user_safe . '&page=' . ( $page + 1 ) ) . '">' . wfMsg( 'g-next' ) . '</a>';
311	363	}
312	364	$output .= '</div>';
313	365	}
—	—	@@ -314,7 +366,9 @@
315	367	/**
316	368	* Build next/prev nav
317	369	*/
318		~~- $output .= '<div class="g-give-all-message-title">' . wfMsg( 'g-give-all-message-title' ) . '</div>~~
	370	+ $output .= '<div class="g-give-all-message-title">' .
	371	+ wfMsg( 'g-give-all-message-title' ) .
	372	+ '</div>
319	373	<textarea name="message" id="message" rows="4" cols="50"></textarea>
320	374	<div class="g-buttons">
321	375	<input type="hidden" name="gift_id" value="0" />

Status & tagging log

13:46, 15 June 2011 Reedy (talk | contribs) changed the status of r85503 [removed: new added: deferred]
20:25, 26 May 2011 Jack Phoenix (talk | contribs) changed the status of r85503 [removed: deferred added: new]
17:48, 26 May 2011 Reedy (talk | contribs) changed the status of r85503 [removed: new added: deferred]
18:02, 23 May 2011 Jack Phoenix (talk | contribs) changed the status of r85503 [removed: deferred added: new]
13:38, 23 May 2011 Reedy (talk | contribs) changed the status of r85503 [removed: new added: deferred]
12:47, 16 April 2011 Jack Phoenix (talk | contribs) changed the status of r85503 [removed: deferred added: new]
22:59, 15 April 2011 Reedy (talk | contribs) changed the status of r85503 [removed: new added: deferred]
01:25, 6 April 2011 Jack Phoenix (talk | contribs) changed the status of r85503 [removed: deferred added: new]