r101940 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r101939‎ | r101940 | r101941 >
Date:01:25, 4 November 2011
Author:jeroendedauw
Status:deferred (Comments)
Tags:
Comment:
added filter options for the contestant list
Modified paths:
  • /trunk/extensions/Contest/Contest.i18n.php (modified) (history)
  • /trunk/extensions/Contest/RELEASE-NOTES (modified) (history)
  • /trunk/extensions/Contest/includes/ContestantPager.php (modified) (history)
  • /trunk/extensions/Contest/specials/SpecialContest.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Contest/Contest.i18n.php
——@@ -153,6 +153,18 @@
154154 'contest-contest-reminder-mail' => 'Reminder e-mail',
155155 'contest-contest-reminder-page' => 'The content for the reminder e-mail comes from [[$1|this page]].',
156156 'contest-contest-send-reminder' => 'Send reminder',
 157+ 'contest-contest-go' => 'Go',
 158+ 'contest-contest-showonly' => 'Filter contestants by',
 159+ 'contest-contest-yes' => 'Yes',
 160+ 'contest-contest-no' => 'No',
 161+ 'contest-contest-none' => 'None',
 162+ 'contest-contest-some' => 'Some',
 163+ 'contest-contest-filter-challenge' => 'Challenge',
 164+ 'contest-contest-filter-volunteer' => 'Volunteer',
 165+ 'contest-contest-filter-wmf' => 'WMF',
 166+ 'contest-contest-filter-comments' => 'Comments',
 167+ 'contest-contest-filter-rating_count' => 'Votes',
 168+ 'contest-contest-filter-submission' => 'Submission',
157169
158170 // Special:Contest, reminder email JS
159171 'contest-contest-reminder-preview' => 'Preview of the reminder e-mail:',
——@@ -276,6 +288,8 @@
277289 'contest-contest-reminder-mail' => 'Reminder e-mail',
278290 'contest-contest-reminder-page' => 'Text explaining the e-mail content is pulled from a page, $1 is the page name.',
279291 'contest-contest-send-reminder' => 'Send reminder button text',
 292+ 'contest-contest-go' => 'Submission button text for filter form',
 293+ 'contest-contest-showonly' => 'Header text for filter form',
280294 'contest-contest-reminder-preview' => 'Text indicating that the following content is the preview for the reminder e-mail.',
281295 'contest-contest-reminder-title' => 'Dialog title',
282296 'contest-contest-reminder-send' => 'Send button text',
Index: trunk/extensions/Contest/specials/SpecialContest.php
——@@ -61,6 +61,10 @@
6262 $this->showMailFunctionality( $contest );
6363 }
6464
 65+ $out->addHTML( Html::element( 'h3', array(), wfMsg( 'contest-contest-contestants' ) ) );
 66+
 67+ $this->addFilterOptionsToSession();
 68+ $this->showFilterControl( $contest, $challengeTitle );
6569 $this->showContestants( $contest, $challengeTitle );
6670
6771 $out->addModules( 'contest.special.contest' );
——@@ -175,23 +179,14 @@
176180 protected function showContestants( Contest $contest, $challengeTitle ) {
177181 $out = $this->getOutput();
178182
179 - $out->addHTML( Html::element( 'h3', array(), wfMsg( 'contest-contest-contestants' ) ) );
 183+ $out->addWikiMsg( 'contest-contest-contestants-text' );
180184
181185 $conds = array(
182186 'contestant_contest_id' => $contest->getId()
183187 );
184 -
185 - if ( $challengeTitle !== false ) {
186 - $challenge = ContestChallenge::s()->selectRow( 'id', array( 'title' => $challengeTitle ) );
187 -
188 - if ( $challenge !== false ) {
189 - $conds['contestant_challenge_id'] = $challenge->getField( 'id' );
190 - unset( $conds['contestant_contest_id'] ); // Not needed because the challenge implies the context
191 - }
192 - }
193 -
194 - $out->addWikiMsg( 'contest-contest-contestants-text' );
195 -
 188+
 189+ $this->addRequestConditions( $conds );
 190+
196191 $pager = new ContestantPager( $this, $conds );
197192
198193 if ( $pager->getNumRows() ) {
——@@ -205,5 +200,178 @@
206201 $out->addWikiMsg( 'contest-contest-no-results' );
207202 }
208203 }
 204+
 205+ /**
 206+ * Add the filter options to the session, so they get retained
 207+ * when the user does navigation such as going to the next
 208+ * set of results using the pager.
 209+ *
 210+ * @since 0.2
 211+ */
 212+ protected function addFilterOptionsToSession() {
 213+ $fields = array(
 214+ 'volunteer',
 215+ 'wmf',
 216+ 'comments',
 217+ 'rating_count',
 218+ 'challenge',
 219+ 'submission'
 220+ );
 221+
 222+ $req = $this->getRequest();
 223+
 224+ foreach ( $fields as $field ) {
 225+ if ( $req->getCheck( $field ) ) {
 226+ $req->setSessionData( 'contestant-' . $field, $req->getVal( $field ) );
 227+ }
 228+ }
 229+ }
 230+
 231+ /**
 232+ * Add the needed conditions to the provided array depending
 233+ * on the filter options set.
 234+ *
 235+ * @since 0.2
 236+ *
 237+ * @param array $conds
 238+ */
 239+ protected function addRequestConditions( &$conds ) {
 240+ $req = $this->getRequest();
 241+
 242+ foreach ( array( 'volunteer', 'wmf' ) as $field ) {
 243+ if ( in_array( $req->getSessionData( 'contestant-' . $field ), array( 'yes', 'no' ) ) ) {
 244+ $conds['contestant_' . $field] = $req->getSessionData( 'contestant-' . $field ) == 'yes' ? 1 : 0;
 245+ }
 246+ }
209247
 248+ foreach ( array( 'comments', 'rating_count' ) as $field ) {
 249+ if ( in_array( $req->getSessionData( 'contestant-' . $field ), array( 'some', 'none' ) ) ) {
 250+ if ( $req->getSessionData( 'contestant-' . $field ) == 'none' ) {
 251+ $conds['contestant_' . $field] = 0;
 252+ }
 253+ else {
 254+ $conds[] = 'contestant_' . $field . ' > 0';
 255+ }
 256+ }
 257+ }
 258+
 259+ if ( $req->getSessionData( 'contestant-challenge' ) ) {
 260+ $challenge = ContestChallenge::s()->selectRow( 'id', array( 'title' => $req->getSessionData( 'contestant-' . $field ) ) );
 261+
 262+ if ( $challenge !== false ) {
 263+ $conds['contestant_challenge_id'] = $challenge->getField( 'id' );
 264+ unset( $conds['contestant_contest_id'] ); // Not needed because the challenge implies the context
 265+ }
 266+ }
 267+
 268+ if ( in_array( $req->getSessionData( 'contestant-submission' ), array( 'some', 'none' ) ) ) {
 269+ if ( $req->getSessionData( 'contestant-submission' ) == 'none' ) {
 270+ $conds['contestant_submission'] = '';
 271+ }
 272+ else {
 273+ $conds[] = 'contestant_submission <> ""';
 274+ }
 275+ }
 276+ }
 277+
 278+ /**
 279+ * Create the filter control and add it to the output.
 280+ *
 281+ * @since 0.2
 282+ *
 283+ * @param Contest $contest
 284+ */
 285+ protected function showFilterControl( Contest $contest ) {
 286+ $req = $this->getRequest();
 287+ $challenges = array();
 288+
 289+ foreach ( $contest->getChallenges() as /* ContestChallenge */ $challenge ) {
 290+ $challenges[$challenge->getField( 'title' )] = $challenge->getField( 'title' );
 291+ }
 292+
 293+ $yesNo = array(
 294+ 'yes' => wfMsg( 'contest-contest-yes' ),
 295+ 'no' => wfMsg( 'contest-contest-no' )
 296+ );
 297+
 298+ $noneSome = array(
 299+ 'none' => wfMsg( 'contest-contest-none' ),
 300+ 'some' => wfMsg( 'contest-contest-some' ),
 301+ );
 302+
 303+ $title = $this->getTitle( $this->subPage )->getFullText();
 304+
 305+ $this->getOutput()->addHTML(
 306+ '<fieldset>' .
 307+ '<legend>' . wfMsgHtml( 'contest-contest-showonly' ) . '</legend>' .
 308+ '<form method="post" action="' . $GLOBALS['wgScript'] . '?title=' . $title . '">' .
 309+ Html::hidden( 'title', $title ) .
 310+ $this->getDropdownHTML(
 311+ 'challenge',
 312+ $challenges
 313+ ) .
 314+ $this->getDropdownHTML(
 315+ 'volunteer',
 316+ $yesNo
 317+ ) .
 318+ $this->getDropdownHTML(
 319+ 'wmf',
 320+ $yesNo
 321+ ) .
 322+ $this->getDropdownHTML(
 323+ 'comments',
 324+ $noneSome
 325+ ) .
 326+ $this->getDropdownHTML(
 327+ 'rating_count',
 328+ $noneSome
 329+ ) .
 330+ $this->getDropdownHTML(
 331+ 'submission',
 332+ $noneSome
 333+ ) .
 334+ '<input type="submit" value="' . wfMsgHtml( 'contest-contest-go' ) . '">' .
 335+ '</form>' .
 336+ '</fieldset>'
 337+ );
 338+ }
 339+
 340+ /**
 341+ * Get the HTML for a filter option dropdown menu.
 342+ *
 343+ * @since 0.2
 344+ *
 345+ * @param string $name
 346+ * @param array $options
 347+ * @param string|null $message
 348+ * @param mixed $value
 349+ *
 350+ * @return string
 351+ */
 352+ protected function getDropdownHTML( $name, array $options, $message = null, $value = null ) {
 353+ $opts = array();
 354+ $options = array_merge( array( '' => ' ' ), $options );
 355+
 356+ if ( is_null( $value ) ) {
 357+ $value = $this->getRequest()->getSessionData( 'contestant-' . $name );
 358+ }
 359+
 360+ if ( is_null( $message ) ) {
 361+ $message = 'contest-contest-filter-' . $name;
 362+ }
 363+
 364+ foreach ( $options as $val => $label ) {
 365+ $attribs = array( 'value' => $val );
 366+
 367+ if ( $val == $value || ( $val === ' ' && !array_key_exists( $val, $options ) ) ) {
 368+ $attribs['selected'] = 'selected';
 369+ }
 370+
 371+ $opts[] = Html::element( 'option', $attribs, $label );
 372+ }
 373+
 374+ return Html::element( 'label', array( 'for' => $name ), wfMsg( $message ) ) . '&#160;' .
 375+ Html::rawElement( 'select', array( 'name' => $name, 'id' => $name ), implode( "\n", $opts ) ) . '&#160;';
 376+ }
 377+
210378 }
Index: trunk/extensions/Contest/RELEASE-NOTES
——@@ -4,6 +4,10 @@
55 Latest version of the release notes: http://svn.wikimedia.org/viewvc/mediawiki/trunk/extensions/Contest/RELEASE-NOTES?view=co
66
77
 8+=== Version 0.2 ===
 9+
 10+* Added filter options for the contestant pager on Special:Contest.
 11+
812 === Version 0.1 ===
913 2011-10-20
1014
Index: trunk/extensions/Contest/includes/ContestantPager.php
——@@ -198,7 +198,7 @@
199199 );
200200 break;
201201 case 'contestant_challenge_id':
202 - $value = Html::element(
 202+ $value = /*Html::element(
203203 'a',
204204 array(
205205 'href' =>
——@@ -207,8 +207,8 @@
208208 $this->page->subPage . '/' . $this->getChallengeTitle( $value )
209209 )->getLocalURL()
210210 ),
211 - $this->getChallengeTitle( $value )
212 - );
 211+ */$this->getChallengeTitle( $value );
 212+ //);
213213 break;
214214 case 'contestant_volunteer': case 'contestant_wmf':
215215 // contest-contestant-yes, contest-contestant-no

Follow-up revisions

RevisionCommit summaryAuthorDate
r101943follow up to r101940 - added more message docsjeroendedauw01:30, 4 November 2011
r102992MFT r101940reedy17:16, 14 November 2011
r106452Follow up to r101940; fix escape failjeroendedauw18:09, 16 December 2011

Comments

#Comment by Catrope (talk | contribs)   16:33, 15 December 2011 
+				'<form method="post" action="' . $GLOBALS['wgScript'] . '?title=' . $title . '">' .

wgScript should be escaped here. This is not an XSS vulnerability, but $title is. I have disabled the extension because of this.

You MUST escape EVERYTHING that goes into HTML, ALWAYS.

OK otherwise.

#Comment by Jeroen De Dauw (talk | contribs)   18:09, 16 December 2011

I know I should do this, not sure how I managed to not to here :/ Anyway, fixed in follow up.

Status & tagging log