r90787 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r90786‎ \| r90787 \| r90788 >
Date:	19:27, 25 June 2011
Author:	ashley
Status:	deferred
Tags:
Comment:	SocialProfile: gift ID must be an int. Also did some coding style tweaks and fixed some code in GiftManagerLogo::saveUploadedFile(); $type is now defined as zero and if it's still zero at the end of the function, we will display an error message to the user. As a result of this, removed the ancient, commented-out code from that function. Made URL building in GiftManagerLogo::showSuccess() more robust by using the proper Title functions instead of hacky DIY stuff.
Modified paths:	/trunk/extensions/SocialProfile/UserGifts/SpecialGiftManagerLogo.php (modified) (history) /trunk/extensions/SocialProfile/UserGifts/SpecialRemoveGift.php (modified) (history)

Diff [purge]

Index: trunk/extensions/SocialProfile/UserGifts/SpecialGiftManagerLogo.php
—	—	@@ -24,7 +24,7 @@
25	25	*/
26	26	public function execute( $par ) {
27	27	global $wgRequest;
28		~~- $this->gift_id = $wgRequest->getVal( 'gift_id' );~~
	28	+ $this->gift_id = $wgRequest->getInt( 'gift_id' );
29	29	$this->initLogo( $wgRequest );
30	30	$this->executeLogo();
31	31	}
—	—	@@ -51,7 +51,7 @@
52	52	# GET requests just give the main form; no data except wpDestfile.
53	53	return;
54	54	}
55		~~- $this->gift_id = $request->getVal( 'gift_id' );~~
	55	+ $this->gift_id = $request->getInt( 'gift_id' );
56	56	$this->mIgnoreWarning = $request->getCheck( 'wpIgnoreWarning' );
57	57	$this->mReUpload = $request->getCheck( 'wpReUpload' );
58	58	$this->mUpload = $request->getCheck( 'wpUpload' );
—	—	@@ -296,6 +296,7 @@
297	297	$this->createThumbnail( $tempName, $ext, $this->gift_id . '_m', 30 );
298	298	$this->createThumbnail( $tempName, $ext, $this->gift_id . '_s', 16 );
299	299
	300	+ $type = 0;
300	301	if ( $ext == 'JPG' && is_file( $this->avatarUploadDirectory . '/' . $this->gift_id . '_l.jpg' ) ) {
301	302	$type = 2;
302	303	}
—	—	@@ -349,13 +350,10 @@
350	351	}
351	352	}
352	353
353		~~- if ( $type > 0 ) {~~
354		~~- // $dbw = wfGetDB( DB_MASTER );~~
355		~~- // $sql = "UPDATE user set user_avatar = " . $type . " WHERE user_id = " . $wgUser->mId;~~
356		~~- // $res = $dbw->query($sql);~~
357		~~- } else {~~
	354	+ if ( $type === 0 ) {
358	355	throw new FatalError( wfMsg( 'filecopyerror', $tempName, $stash ) ); # FIXME: undefined variable $stash
359	356	}
	357	+
360	358	return $type;
361	359	}
362	360
—	—	@@ -430,7 +428,7 @@
431	429	* @access private
432	430	*/
433	431	function showSuccess( $status ) {
434		~~- global $wgOut, $wgUploadPath, $wgScriptPath, $wgLang;~~
	432	+ global $wgOut, $wgUploadPath, $wgLang;
435	433	$ext = 'jpg';
436	434
437	435	$output = '<h2>' . wfMsg( 'g-uploadsuccess' ) . '</h2>';
—	—	@@ -456,9 +454,12 @@
457	455	<td><img src="' . $wgUploadPath . '/awards/' . $this->gift_id . '_s.' . $ext . '?ts' . rand() . '"></td></tr>';
458	456	$output .= '<tr><td><input type="button" onclick="javascript:history.go(-1)" value="' . wfMsg( 'g-go-back' ) . '"></td></tr>';
459	457
	458	+ $giftManager = SpecialPage::getTitleFor( 'GiftManager' );
460	459	$output .= $wgLang->pipeList( array(
461		~~- '<tr><td><a href="' . $wgScriptPath . '/index.php?title=Special:GiftManager">' . wfMsg( 'g-back-gift-list' ) . '</a> ',~~
462		~~- ' <a href="' . $wgScriptPath . '/index.php?title=Special:GiftManager&id=' . $this->gift_id . '">' . wfMsg( 'g-back-edit-gift' ) . '</a></td></tr>'~~
	460	+ '<tr><td><a href="' . $giftManager->escapeFullURL() . '">' .
	461	+ wfMsg( 'g-back-gift-list' ) . '</a> ',
	462	+ ' <a href="' . $giftManager->escapeFullURL( 'id=' . $this->gift_id ) .
	463	+ '">' . wfMsg( 'g-back-edit-gift' ) . '</a></td></tr>'
463	464	) );
464	465	$output .= '</table>';
465	466	$wgOut->addHTML( $output );
—	—	@@ -598,8 +599,11 @@
599	600	global $wgUploadPath;
600	601	$gift_image = Gifts::getGiftImage( $this->gift_id, 'l' );
601	602	if ( $gift_image != '' ) {
602		~~- $output = '<table><tr><td style="color:#666666;font-weight:800">' . wfMsg( 'g-current-image' ) . '</td></tr>';~~
603		~~- $output .= '<tr><td><img src="' . $wgUploadPath . '/images/awards/' . $gift_image . '" border="0" alt="' . wfMsg( 'g-gift' ) . '" /></td></tr></table><br />';~~
	603	+ $output = '<table><tr><td style="color:#666666;font-weight:800">' .
	604	+ wfMsg( 'g-current-image' ) . '</td></tr>';
	605	+ $output .= '<tr><td><img src="' . $wgUploadPath .
	606	+ '/images/awards/' . $gift_image . '" border="0" alt="' .
	607	+ wfMsg( 'g-gift' ) . '" /></td></tr></table><br />';
604	608	}
605	609	$wgOut->addHTML( $output );
606	610
Index: trunk/extensions/SocialProfile/UserGifts/SpecialRemoveGift.php
—	—	@@ -19,7 +19,7 @@
20	20
21	21	$wgOut->addExtensionStyle( $wgUserGiftsScripts . '/UserGifts.css' );
22	22
23		~~- $this->gift_id = $wgRequest->getVal( 'gift_id' );~~
	23	+ $this->gift_id = $wgRequest->getInt( 'gift_id' );
24	24	$rel = new UserGifts( $wgUser->getName() );
25	25
26	26	if ( !$this->gift_id \|\| !is_numeric( $this->gift_id ) ) {
—	—	@@ -27,6 +27,7 @@
28	28	$wgOut->addHTML( wfMsg( 'g-error-message-invalid-link' ) );
29	29	return false;
30	30	}
	31	+
31	32	if ( $rel->doesUserOwnGift( $wgUser->getID(), $this->gift_id ) == false ) {
32	33	$wgOut->setPageTitle( wfMsg( 'g-error-title' ) );
33	34	$wgOut->addHTML( wfMsg( 'g-error-do-not-own' ) );
—	—	@@ -44,15 +45,18 @@
45	46	$rel->deleteGift( $this->gift_id );
46	47	}
47	48
48		~~- $gift_image = '<img src="' . $wgUploadPath . '/awards/' . Gifts::getGiftImage( $gift['gift_id'], 'l' ) . '" border="0" alt="" />';~~
	49	+ $gift_image = '<img src="' . $wgUploadPath . '/awards/' .
	50	+ Gifts::getGiftImage( $gift['gift_id'], 'l' ) .
	51	+ '" border="0" alt="" />';
49	52
50	53	$wgOut->setPageTitle( wfMsg( 'g-remove-success-title', $gift['name'] ) );
51	54
52	55	$out = '<div class="back-links">
53		~~- <a href="' . $wgUser->getUserPage()->escapeFullURL() . '">' . wfMsg( 'g-back-link', $gift['user_name_to'] ) . '</a>~~
	56	+ <a href="' . $wgUser->getUserPage()->escapeFullURL() . '">' .
	57	+ wfMsg( 'g-back-link', $gift['user_name_to'] ) . '</a>
54	58	</div>
55		~~- <div class="g-container">'~~
56		~~- . $gift_image . wfMsg( 'g-remove-success-message', $gift['name'] ) .~~
	59	+ <div class="g-container">' .
	60	+ $gift_image . wfMsg( 'g-remove-success-message', $gift['name'] ) .
57	61	'<div class="cleared"></div>
58	62	</div>
59	63	<div class="g-buttons">
—	—	@@ -77,22 +81,32 @@
78	82	$rel = new UserGifts( $wgUser->getName() );
79	83	$gift = $rel->getUserGift( $this->gift_id );
80	84	$user = Title::makeTitle( NS_USER, $gift['user_name_from'] );
81		~~- $gift_image = '<img src="' . $wgUploadPath . '/awards/' . Gifts::getGiftImage( $gift['gift_id'], 'l' ) . '" border="0" alt="gift" />';~~
	85	+ $gift_image = '<img src="' . $wgUploadPath . '/awards/' .
	86	+ Gifts::getGiftImage( $gift['gift_id'], 'l' ) .
	87	+ '" border="0" alt="gift" />';
82	88
83		~~- $output = $wgOut->setPageTitle( wfMsg( 'g-remove-title', $gift['name'] ) );~~
84		~~- $output .= '<div class="back-links">~~
85		~~- <a href="' . $wgUser->getUserPage()->escapeFullURL() . '">' . wfMsg( 'g-back-link', $gift['user_name_to'] ) . '</a>~~
	89	+ $wgOut->setPageTitle( wfMsg( 'g-remove-title', $gift['name'] ) );
	90	+
	91	+ $output = '<div class="back-links">
	92	+ <a href="' . $wgUser->getUserPage()->escapeFullURL() . '">' .
	93	+ wfMsg( 'g-back-link', $gift['user_name_to'] ) . '</a>
86	94	</div>
87	95	<form action="" method="post" enctype="multipart/form-data" name="form1">
88		~~- <div class="g-remove-message">'~~
89		~~- . wfMsg( 'g-remove-message', $gift['name'] ) .~~
	96	+ <div class="g-remove-message">' .
	97	+ wfMsg( 'g-remove-message', $gift['name'] ) .
90	98	'</div>
91		~~- <div class="g-container">'~~
92		~~- . $gift_image .~~
	99	+ <div class="g-container">' .
	100	+ $gift_image .
93	101	'<div class="g-name">' . $gift['name'] . '</div>
94		~~- <div class="g-from">' . wfMsg( 'g-from', $user->escapeFullURL(), $gift['user_name_from'] ) . '</div>';~~
	102	+ <div class="g-from">' .
	103	+ wfMsg(
	104	+ 'g-from',
	105	+ $user->escapeFullURL(),
	106	+ $gift['user_name_from']
	107	+ ) . '</div>';
95	108	if ( $gift['message'] ) {
96		~~- $output .= '<div class="g-user-message">' . $gift['message'] . '</div>';~~
	109	+ $output .= '<div class="g-user-message">' .
	110	+ $gift['message'] . '</div>';
97	111	}
98	112	$output .= '</div>
99	113	<div class="cleared"></div>

Status & tagging log

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