r96855 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r96854‎ | r96855 | r96856 >
Date:15:28, 12 September 2011
Author:jeroendedauw
Status:deferred (Comments)
Tags:
Comment:
added min_pages setting
Modified paths:
  • /trunk/extensions/Survey/Survey.i18n.php (modified) (history)
  • /trunk/extensions/Survey/Survey.settings.php (modified) (history)
  • /trunk/extensions/Survey/Survey.sql (modified) (history)
  • /trunk/extensions/Survey/includes/Survey.class.php (modified) (history)
  • /trunk/extensions/Survey/specials/SpecialSurvey.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Survey/Survey.settings.php
@@ -24,7 +24,8 @@
2525 'defaultUserType' => Survey::$USER_ALL,
2626 'defaultNamespaces' => array(),
2727 'defaultRatio' => 100,
28 - 'defaultExpiry' => 60 * 60 * 24 * 30
 28+ 'defaultExpiry' => 60 * 60 * 24 * 30,
 29+ 'defaultMinPages' => 0
2930 );
3031 }
3132
Index: trunk/extensions/Survey/Survey.sql
@@ -11,10 +11,11 @@
1212 survey_header TEXT NOT NULL, -- Text to display above the survey
1313 survey_footer TEXT NOT NULL, -- Text to display below the survey
1414 survey_thanks TEXT NOT NULL, -- Text to display after survey submission
15 - survey_user_type TINYINT NOT NULL default '0', -- Type of users that can participate in the survey
 15+ survey_user_type TINYINT unsigned NOT NULL default '0', -- Type of users that can participate in the survey
1616 survey_namespaces BLOB NOT NULL, -- Namespaces on which the survey can be displayed
1717 survey_ratio TINYINT unsigned NOT NULL, -- Percentage of users to show the survey to
1818 survey_expiry INT unsigned NOT NULL, -- Coockie expiry time for the survey
 19+ survey_min_pages TINYINT unsigned NOT NULL -- Min amount of pages the user needs to view before getting the survey
1920 ) /*$wgDBTableOptions*/;
2021
2122 -- Questions
@@ -23,7 +24,7 @@
2425 question_survey_id SMALLINT unsigned NOT NULL, -- Foreign key: surveys.survey_id
2526 question_text TEXT NOT NULL,
2627 question_type INT(2) unsigned NOT NULL,
27 - question_required INT(2) unsigned NOT NULL,
 28+ question_required TINYINT NOT NULL,
2829 question_answers BLOB NOT NULL,
2930 question_removed TINYINT NOT NULL default '0'
3031 ) /*$wgDBTableOptions*/;
Index: trunk/extensions/Survey/Survey.i18n.php
@@ -44,6 +44,14 @@
4545 'survey-question-type-textarea' => 'Multi-line text field',
4646 'survey-question-type-check' => 'Checkbox',
4747
 48+ // User types
 49+ 'survey-user-type-all' => 'Everyone',
 50+ 'survey-user-type-loggedin' => 'Logged in users',
 51+ 'survey-user-type-confirmed' => 'Confirmed users',
 52+ 'survey-user-type-editor' => 'Editors',
 53+ 'survey-user-type-anon' => 'Anonymous users',
 54+
 55+ // Navigation
4856 'survey-navigation-edit' => '[[Special:Survey/$1|Edit this survey]]',
4957 'survey-navigation-take' => '[[Special:TakeSurvey/$1|Take this survey]]',
5058 'survey-navigation-list' => '[[Special:Surveys|Surveys list]]',
@@ -66,11 +74,7 @@
6775 'surveys-special-confirm-delete' => 'Are you sure you want to delete this survey?',
6876 'surveys-special-delete-failed' => 'Failed to delete survey.',
6977 'survey-special-label-usertype' => 'Users that should get the survey',
70 - 'survey-user-type-all' => 'Everyone',
71 - 'survey-user-type-loggedin' => 'Logged in users',
72 - 'survey-user-type-confirmed' => 'Confirmed users',
73 - 'survey-user-type-editor' => 'Editors',
74 - 'survey-user-type-anon' => 'Anonymous users',
 78+ 'survey-special-label-minpages' => 'Minimun amount of pages the user needs to visit before getting the survey',
7579
7680 // Special:TakeSurvey
7781 'surveys-takesurvey-loading' => 'Loading survey...',
Index: trunk/extensions/Survey/specials/SpecialSurvey.php
@@ -86,7 +86,7 @@
8787
8888 $survey->setField( 'enabled', $wgRequest->getCheck( 'survey-enabled' ) );
8989
90 - foreach ( array( 'user_type', 'ratio' ) as $field ) {
 90+ foreach ( array( 'user_type', 'ratio', 'min_pages' ) as $field ) {
9191 $survey->setField( $field, $wgRequest->getInt( 'survey-' . $field ) );
9292 }
9393
@@ -169,6 +169,15 @@
170170 );
171171 }
172172
 173+ protected function getNumericalOptions( array $numbers ) {
 174+ $lang = $this->getLang();
 175+
 176+ return array_flip( array_map(
 177+ function( $n ) use( $lang ) { return $lang->formatNum( $n ); },
 178+ array_combine( $numbers, $numbers )
 179+ ) );
 180+ }
 181+
173182 /**
174183 * Show the survey.
175184 *
@@ -231,23 +240,25 @@
232241 ),
233242 );
234243
235 - $nrs = array_merge( array( 0.01, 0.1 ), range( 1, 100 ) );
236 -
237 - $lang = $this->getLang();
238 -
239244 $fields[] = array(
240245 'type' => 'select',
241246 'default' => $survey->getField( 'ratio' ),
242247 'label-message' => 'survey-special-label-ratio',
243248 'id' => 'survey-ratio',
244249 'name' => 'survey-ratio',
245 - 'options' => array_flip( array_map(
246 - function( $n ) use( $lang ) { return $lang->formatNum( $n ); },
247 - array_combine( $nrs, $nrs )
248 - ) ),
 250+ 'options' => $this->getNumericalOptions( array_merge( array( 0.01, 0.1 ), range( 1, 100 ) ) ),
249251 );
250252
251253 $fields[] = array(
 254+ 'type' => 'select',
 255+ 'default' => $survey->getField( 'min_pages' ),
 256+ 'label-message' => 'survey-special-label-minpages',
 257+ 'id' => 'survey-min_pages',
 258+ 'name' => 'survey-min_pages',
 259+ 'options' => $this->getNumericalOptions( range( 0, 250 ) ),
 260+ );
 261+
 262+ $fields[] = array(
252263 'type' => 'text',
253264 'default' => $survey->getField( 'header' ),
254265 'label-message' => 'survey-special-label-header',
Index: trunk/extensions/Survey/includes/Survey.class.php
@@ -46,7 +46,8 @@
4747 'user_type' => 'int',
4848 'namespaces' => 'array',
4949 'ratio' => 'int',
50 - 'expiry' => 'int'
 50+ 'expiry' => 'int',
 51+ 'min_pages' => 'int'
5152 );
5253 }
5354
@@ -70,6 +71,7 @@
7172 'namespaces' => SurveySettings::get( 'defaultNamespaces' ),
7273 'ratio' => SurveySettings::get( 'defaultRatio' ),
7374 'expiry' => SurveySettings::get( 'defaultExpiry' ),
 75+ 'min_pages' => SurveySettings::get( 'defaultMinPages' ),
7476 );
7577 }
7678

Follow-up revisions

RevisionCommit summaryAuthorDate
r96890follow up to r96855; adding js for min_pages settingjeroendedauw19:46, 12 September 2011
r96896Follow up to r96855; i18n docs++jeroendedauw20:20, 12 September 2011

Comments

#Comment by Siebrand (talk | contribs)   16:47, 12 September 2011

Can you please add message documentation in the 'qqq' language for messages that you add? Thanks.

#Comment by Jeroen De Dauw (talk | contribs)   18:05, 12 September 2011

Sure. What would be a helpful message documentation string for the message added here? ie, what's not clear right now, and is needed to make good translations?

#Comment by Siebrand (talk | contribs)   18:51, 12 September 2011

Just point out how/where it is used. Could something like "Group of users for which a survey can be available." for "Everyone", or for some hypothetical case "Field label for ...". Especially parameters in messages should be well documented. If there are space restrictions, which should be avoided, this is also something to mention. Enough to continue for now?

#Comment by Jeroen De Dauw (talk | contribs)   20:21, 12 September 2011

Does the stuff in my follow up rev suffice?

#Comment by Siebrand (talk | contribs)   21:05, 12 September 2011

Very nice, Jeroen. Thank you!

Status & tagging log