r53909 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r53908‎ | r53909 | r53910 >
Date:22:24, 28 July 2009
Author:catrope
Status:deferred
Tags:
Comment:
OptIn: Fixes per Brion's review
* Make browser/OS detection JS less fragile by using text keys for questions and answers
* Make ois_question a varchar(16) to accomodate this
* Fix the issue where the title on the "Welcome to Beta" page would change to "Leave Beta" after refreshing or after logging in on an already opted-in account
* Remove obsolete PrefStats file
Modified paths:
  • /trunk/extensions/UsabilityInitiative/OptIn/OptIn.hooks.php (modified) (history)
  • /trunk/extensions/UsabilityInitiative/OptIn/OptIn.js (modified) (history)
  • /trunk/extensions/UsabilityInitiative/OptIn/OptIn.php (modified) (history)
  • /trunk/extensions/UsabilityInitiative/OptIn/OptIn.sql (modified) (history)
  • /trunk/extensions/UsabilityInitiative/OptIn/SpecialOptIn.php (modified) (history)
  • /trunk/extensions/UsabilityInitiative/OptIn/patch-ois_question.sql (added) (history)
  • /trunk/extensions/UsabilityInitiative/PrefStats/getstats.sql (deleted) (history)

Diff [purge]

Index: trunk/extensions/UsabilityInitiative/OptIn/OptIn.php
——@@ -28,7 +28,7 @@
2929 $wgOptInAlwaysShowPersonalLink = false;
3030 $wgOptInNeverShowPersonalLink = false;
3131
32 -$wgOptInStyleVersion = 6;
 32+$wgOptInStyleVersion = 7;
3333
3434 // Preferences to set when users opt in
3535 // array( prefname => value )
——@@ -36,75 +36,85 @@
3737
3838 // Survey questions to ask when users opt out
3939 $wgOptInSurvey = array(
40 - array( 'question' => 'optin-survey-question-likedislike',
 40+ 'likedislike' => array(
 41+ 'question' => 'optin-survey-question-likedislike',
4142 'type' => 'textarea' ),
42 - array( 'question' => 'optin-survey-question-whyoptout',
 43+ 'whyoptout' => array(
 44+ 'question' => 'optin-survey-question-whyoptout',
4345 'type' => 'checkboxes',
4446 'answers' => array(
45 - 'optin-survey-answer-whyoptout-hard',
46 - 'optin-survey-answer-whyoptout-didntwork',
47 - 'optin-survey-answer-whyoptout-notpredictable',
48 - 'optin-survey-answer-whyoptout-didntlike-look',
49 - 'optin-survey-answer-whyoptout-didntlike-layout',
50 - 'optin-survey-answer-whyoptout-didntlike-toolbar' ),
 47+ 'hard' => 'optin-survey-answer-whyoptout-hard',
 48+ 'didntwork' => 'optin-survey-answer-whyoptout-didntwork',
 49+ 'notpredictable' => 'optin-survey-answer-whyoptout-notpredictable',
 50+ 'look' => 'optin-survey-answer-whyoptout-didntlike-look',
 51+ 'layout' => 'optin-survey-answer-whyoptout-didntlike-layout',
 52+ 'toolbar' => 'optin-survey-answer-whyoptout-didntlike-toolbar' ),
5153 'other' => 'optin-survey-answer-whyoptout-other' ),
52 - array( 'question' => 'optin-survey-question-explain',
 54+ 'explainwhyoptout' => array(
 55+ 'question' => 'optin-survey-question-explain',
5356 'type' => 'textarea' ),
54 - array( 'question' => 'optin-survey-question-techfail',
 57+ 'techfail' => array(
 58+ 'question' => 'optin-survey-question-techfail',
5559 'type' => 'yesno',
5660 'ifyes' => 'optin-survey-question-techfail-ifyes' ),
57 - array( 'question' => 'optin-survey-question-usedtoolbar',
 61+ 'usedtoolbar' => array(
 62+ 'question' => 'optin-survey-question-usedtoolbar',
5863 'type' => 'yesno',
5964 'ifyes' => 'optin-survey-question-usedtoolbar-ifyes' ),
60 - array( 'question' => 'optin-survey-question-different',
 65+ 'different' => array(
 66+ 'question' => 'optin-survey-question-different',
6167 'type' => 'textarea' ),
62 - array( 'question' => 'optin-survey-question-feedback',
 68+ 'feedback' => array(
 69+ 'question' => 'optin-survey-question-feedback',
6370 'type' => 'textarea' ),
64 - array( 'question' => 'optin-survey-question-browser',
 71+ 'browser' => array(
 72+ 'question' => 'optin-survey-question-browser',
6573 'type' => 'dropdown',
6674 'answers' => array(
67 - 'optin-survey-answer-browser-ie5',
68 - 'optin-survey-answer-browser-ie6',
69 - 'optin-survey-answer-browser-ie7',
70 - 'optin-survey-answer-browser-ie8',
71 - 'optin-survey-answer-browser-ff1',
72 - 'optin-survey-answer-browser-ff2',
73 - 'optin-survey-answer-browser-ff3',
74 - 'optin-survey-answer-browser-cb',
75 - 'optin-survey-answer-browser-c1',
76 - 'optin-survey-answer-browser-c2',
77 - 'optin-survey-answer-browser-s3',
78 - 'optin-survey-answer-browser-s4',
79 - 'optin-survey-answer-browser-o9',
80 - 'optin-survey-answer-browser-o9.5',
81 - 'optin-survey-answer-browser-o10' ),
 75+ 'ie5' => 'optin-survey-answer-browser-ie5',
 76+ 'ie6' => 'optin-survey-answer-browser-ie6',
 77+ 'ie7' => 'optin-survey-answer-browser-ie7',
 78+ 'ie8' => 'optin-survey-answer-browser-ie8',
 79+ 'ff1' => 'optin-survey-answer-browser-ff1',
 80+ 'ff2' => 'optin-survey-answer-browser-ff2',
 81+ 'ff3'=> 'optin-survey-answer-browser-ff3',
 82+ 'cb' => 'optin-survey-answer-browser-cb',
 83+ 'c1' => 'optin-survey-answer-browser-c1',
 84+ 'c2' => 'optin-survey-answer-browser-c2',
 85+ 's3' => 'optin-survey-answer-browser-s3',
 86+ 's4' => 'optin-survey-answer-browser-s4',
 87+ 'o9' => 'optin-survey-answer-browser-o9',
 88+ 'o9.5' => 'optin-survey-answer-browser-o9.5',
 89+ 'o10' => 'optin-survey-answer-browser-o10' ),
8290 'other' => 'optin-survey-answer-browser-other' ),
83 - array( 'question' => 'optin-survey-question-os',
 91+ 'os' => array(
 92+ 'question' => 'optin-survey-question-os',
8493 'type' => 'dropdown',
8594 'answers' => array(
86 - 'optin-survey-answer-os-windows',
87 - 'optin-survey-answer-os-macos',
88 - 'optin-survey-answer-os-linux' ),
 95+ 'windows' => 'optin-survey-answer-os-windows',
 96+ 'macos' => 'optin-survey-answer-os-macos',
 97+ 'linux' => 'optin-survey-answer-os-linux' ),
8998 'other' => 'optin-survey-answer-os-other' ),
90 - array( 'question' => 'optin-survey-question-res',
 99+ 'res' => array(
 100+ 'question' => 'optin-survey-question-res',
91101 'type' => 'resolution' ),
92102 );
93103
94104 $wgOptInFeedBackSurvey = $wgOptInSurvey;
95 -unset( $wgOptInFeedBackSurvey[1] );
96 -unset( $wgOptInFeedBackSurvey[2] );
97 -$wgOptInFeedBackSurvey[4]['ifno'] = 'optin-survey-question-usedtoolbar-ifno';
98 -$wgOptInFeedBackSurvey[5] = array(
 105+unset( $wgOptInFeedBackSurvey['whyoptout'] );
 106+unset( $wgOptInFeedBackSurvey['explainwhyoptout'] );
 107+$wgOptInFeedBackSurvey['usedtoolbar']['ifno'] = 'optin-survey-question-usedtoolbar-ifno';
 108+$wgOptInFeedBackSurvey['changes'] = array(
99109 'question' => 'optin-survey-question-changes',
100110 'type' => 'checkboxes',
101111 'answers' => array(
102 - 'optin-survey-answer-changes-nav',
103 - 'optin-survey-answer-changes-edittools',
104 - 'optin-survey-answer-changes-upload',
105 - 'optin-survey-answer-changes-richtext',
106 - 'optin-survey-answer-changes-lookfeel',
107 - 'optin-survey-answer-changes-predictability',
108 - 'optin-survey-answer-changes-custom' ),
 112+ 'nav' => 'optin-survey-answer-changes-nav',
 113+ 'edittools' => 'optin-survey-answer-changes-edittools',
 114+ 'upload' => 'optin-survey-answer-changes-upload',
 115+ 'richtext' => 'optin-survey-answer-changes-richtext',
 116+ 'lookfeel' => 'optin-survey-answer-changes-lookfeel',
 117+ 'predictability' => 'optin-survey-answer-changes-predictability',
 118+ 'custom' => 'optin-survey-answer-changes-custom' ),
109119 'other' => 'optin-survey-answer-changes-other'
110120 );
111121
——@@ -141,4 +151,3 @@
142152 $wgHooks['LoadExtensionSchemaUpdates'][] = 'OptInHooks::schema';
143153
144154 $wgHooks['PersonalUrls'][] = 'OptInHooks::personalUrls';
145 -
Index: trunk/extensions/UsabilityInitiative/OptIn/SpecialOptIn.php
——@@ -81,6 +81,9 @@
8282 else if ( $wgRequest->getVal( 'opt' ) == 'feedback' )
8383 // Giving feedback
8484 $wgOut->setPageTitle( wfMsg( 'optin-title-feedback' ) );
 85+ else if ( $wgRequest->getVal( 'opt' ) == 'in' )
 86+ // Just opted in and reloaded... or something
 87+ $wgOut->setPagetitle( wfMsg( 'optin-title-justoptedin' ) );
8588 else
8689 // About to opt out
8790 $wgOut->setPageTitle( wfMsg( 'optin-title-optedin' ) );
Index: trunk/extensions/UsabilityInitiative/OptIn/OptIn.hooks.php
——@@ -21,6 +21,11 @@
2222 'ois_type',
2323 dirname( __FILE__ ) . '/OptIn.patch.ois_type.sql'
2424 );
 25+ $wgExtNewFields[] = array(
 26+ 'optin_survey',
 27+ 'ois_question',
 28+ dirname( __FILE__ ) . '/patch-ois_question.sql'
 29+ );
2530
2631 return true;
2732 }
——@@ -73,4 +78,4 @@
7479 $personal_urls = array_merge( $addLinks, $personal_urls );
7580 return true;
7681 }
77 -}
\ No newline at end of file
 82+}
Index: trunk/extensions/UsabilityInitiative/OptIn/OptIn.js
——@@ -36,51 +36,44 @@
3737 }
3838 // Detect browser and version
3939 // BEWARE: this depends on the order of browsers in $wgOptInSurvey
40 - var browserIndex = -1;
 40+ var browserIndex = 'other';
4141 switch ( $.browser.name ) {
4242 case 'msie':
43 - browserIndex = $.browser.versionNumber - 5;
 43+ browserIndex = 'ie'+ parseInt( $.browser.versionNumber );
4444 break;
4545 case 'firefox':
46 - browserIndex = $.browser.versionNumber + 3;
 46+ browserIndex = 'ff' + parseInt( $.browser.versionNumber );
4747 break;
4848 case 'chrome':
49 - browserIndex = $.browser.versionNumber + 7;
 49+ browserIndex = 'c' + parseInt( $.browser.versionNumber ); // FIXME: Chrome Beta?
5050 break;
5151 case 'safari':
52 - browserIndex = $.browser.versionNumber + 7;
 52+ browserIndex = 's' + parseInt( $.browser.versionNumber );
5353 break;
5454 case 'opera':
5555 if ( parseInt( $.browser.versionNumber ) == 9 ) {
5656 if ( $.browser.version.substr( 0, 3 ) == '9.5' )
57 - browserIndex = 13;
 57+ browserIndex = 'o9.5';
5858 else
59 - browserIndex = 12;
 59+ browserIndex = 'o9';
6060 } else if ( parseInt( $.browser.versionNumber ) == 10 )
61 - browserIndex = 14;
 61+ browserIndex = 'o10';
6262 break;
6363 }
6464
65 - var osIndex = -1;
 65+ var osIndex = 'other';
6666 switch ( $.os.name ) {
6767 case 'win':
68 - osIndex = 0;
 68+ osIndex = 'windows';
6969 break;
7070 case 'mac':
71 - osIndex = 1;
 71+ osIndex = 'macos';
7272 break;
7373 case 'linux':
74 - osIndex = 2;
 74+ osIndex = 'linux';
7575 break;
7676 }
7777
78 - if ( browserIndex == -1 )
79 - $( '#survey-7' ).val( 'other' );
80 - else
81 - $( '#survey-7' ).val( parseInt( browserIndex ) );
82 - if ( osIndex == -1 )
83 - $( '#survey-8' ).val( 'other' );
84 - else
85 - $( '#survey-8' ).val( osIndex );
86 -
 78+ $( '#survey-browser' ).val( browserIndex );
 79+ $( '#survey-os' ).val( osIndex );
8780 });
Index: trunk/extensions/UsabilityInitiative/OptIn/OptIn.sql
——@@ -13,7 +13,7 @@
1414 ois_type varchar(16) NOT NULL,
1515
1616 -- Question ID (key in $wgOptInSurvey)
17 - ois_question int unsigned NOT NULL,
 17+ ois_question varchar(16) NOT NULL,
1818
1919 -- Answer ID (key in $wgOptInSurvey[ois_question]['answers'])
2020 ois_answer text NULL,
Index: trunk/extensions/UsabilityInitiative/OptIn/patch-ois_question.sql
——@@ -0,0 +1,5 @@
 2+--
 3+-- Schema patch for OptIn
 4+--
 5+
 6+ALTER TABLE /*_*/optin_survey modify ois_type varchar(16) NOT NULL;
\ No newline at end of file
Index: trunk/extensions/UsabilityInitiative/PrefStats/getstats.sql
——@@ -1,42 +0,0 @@
2 -
3 -
4 -UPDATE prefstats
5 -SET ps_end = IF(ISNULL(
6 - (SELECT up_user
7 - FROM enwiki_p.user_properties
8 - WHERE up_user=ps_user AND up_property='skin' AND up_value='vector'
9 - )), NOW(), NULL),
10 -ps_duration = TIMESTAMPDIFF(DAY, NOW(), ps_start)
11 -WHERE ps_pref='vector' AND ps_end IS NULL;
12 -
13 -INSERT IGNORE INTO prefstats (ps_user, ps_pref, ps_start, ps_end, ps_duration)
14 -SELECT up_user, up_value, NOW(), NULL, 0
15 -FROM enwiki_p.user_properties
16 -WHERE up_property='skin' AND up_value='vector';
17 -
18 -
19 -UPDATE prefstats
20 -SET ps_end = IF(ISNULL(
21 - (SELECT up_user
22 - FROM enwiki_p.user_properties
23 - WHERE up_user=ps_user AND up_property='usebetatoolbar' AND up_value='1'
24 - )), NOW(), NULL),
25 -ps_duration = TIMESTAMPDIFF(DAY, NOW(), ps_start)
26 -WHERE ps_pref='vector' AND ps_end IS NULL;
27 -
28 -INSERT IGNORE INTO prefstats (ps_user, ps_pref, ps_start, ps_end, ps_duration)
29 -SELECT up_user, up_value, NOW(), NULL, 0
30 -FROM enwiki_p.user_properties
31 -WHERE up_property='usebetatoolbar' AND up_value='1';

Status & tagging log