r104504 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r104503‎ | r104504 | r104505 >
Date:23:12, 28 November 2011
Author:rsterbin
Status:deferred (Comments)
Tags:
Comment:
Added one pathway for edit tracking: add info to query string and pass through
as hidden variables on the edit page:
- articleFeedbackv5 jquery plugin:
- New property feedbackId
- Updated ctas[1].build() to add the feedback id, cta id, and bucket id
to the query string for edits
- Updated submitForm() to grab the feedback and cta ids from the
returned data on success
- Fixed showCTA(); no longer needs to accept a cta id
- Updated ApiArticleFeedbackv5 to return the feedback and cta ids
- ArticleFeedbackv5Hooks:
- New methods pushTrackingFieldsToEdit() and trackEdit()
* NB: only prints info to the log; method of storage is still not in
the spec.
- ArticleFeedbackv5.php:
- Added ArticleFeedbackv5Hooks::pushTrackingFieldsToEdit() to the
"EditPage::showEditForm:fields" hook
- Added ArticleFeedbackv5Hooks::trackEdit() to the
"ArticleSaveComplete" hook

Removed old pitch handling from ext.articleFeedbackv5.js (can use version 4 as a
reference; no need to keep it around)
Modified paths:
  • /trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.hooks.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/ext.articleFeedbackv5/ext.articleFeedbackv5.js (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js
@@ -104,6 +104,11 @@
105105 */
106106 $.articleFeedbackv5.revisionId = mw.config.get( 'wgCurRevisionId' );
107107
 108+ /**
 109+ * The feedback ID (collected on submit, for use in tracking edits)
 110+ */
 111+ $.articleFeedbackv5.feedbackId = 0;
 112+
108113 // }}}
109114 // {{{ Bucket UI objects
110115
@@ -1768,7 +1773,10 @@
17691774 'href',
17701775 mw.config.get( 'wgScript' ) + '?' + $.param( {
17711776 'title': mw.config.get( 'wgPageName' ),
1772 - 'action': 'edit'
 1777+ 'action': 'edit',
 1778+ 'articleFeedbackv5_feedback_id': $.articleFeedbackv5.feedbackId,
 1779+ 'articleFeedbackv5_cta_id': $.articleFeedbackv5.ctaId,
 1780+ 'articleFeedbackv5_bucket_id': $.articleFeedbackv5.bucketId
17731781 } )
17741782 );
17751783
@@ -2008,19 +2016,29 @@
20092017 'bucket': $.articleFeedbackv5.bucketId
20102018 } ),
20112019 'success': function( data ) {
2012 - if ( 'error' in data ) {
2013 - if ( 'markFormError' in bucket ) {
2014 - bucket.markFormError( { _api : data.error } );
2015 - } else {
2016 - alert( 'ArticleFeedback: Form submission error : ' + data.error );
2017 - }
 2020+ if ( 'articlefeedbackv5' in data
 2021+ && 'feedback_id' in data.articlefeedbackv5
 2022+ && 'cta_id' in data.articlefeedbackv5 ) {
 2023+ $.articleFeedbackv5.feedbackId = data.articlefeedbackv5.feedback_id;
 2024+ $.articleFeedbackv5.ctaId = data.articlefeedbackv5.cta_id;
20182025 $.articleFeedbackv5.unlockForm();
2019 - } else {
2020 - $.articleFeedbackv5.unlockForm();
20212026 if ( 'onSuccess' in bucket ) {
20222027 bucket.onSuccess( formdata );
20232028 }
20242029 $.articleFeedbackv5.showCTA();
 2030+ } else {
 2031+ var msg;
 2032+ if ( 'error' in data ) {
 2033+ msg = data.error;
 2034+ } else {
 2035+ msg = 'Unknown error';
 2036+ }
 2037+ if ( 'markFormError' in bucket ) {
 2038+ bucket.markFormError( { _api : msg } );
 2039+ } else {
 2040+ alert( 'ArticleFeedback: Form submission error : ' + msg );
 2041+ }
 2042+ $.articleFeedbackv5.unlockForm();
20252043 }
20262044 },
20272045 'error': function () {
@@ -2089,11 +2107,8 @@
20902108
20912109 /**
20922110 * Shows a CTA
2093 - *
2094 - * @param cta_name string the name of the CTA to display
20952111 */
2096 - $.articleFeedbackv5.showCTA = function ( ctaId ) {
2097 - $.articleFeedbackv5.ctaId = 1; // For now, just use the edit CTA
 2112+ $.articleFeedbackv5.showCTA = function () {
20982113 var cta = $.articleFeedbackv5.currentCTA();
20992114 if ( !( 'build' in cta ) ) {
21002115 return;
Index: trunk/extensions/ArticleFeedbackv5/modules/ext.articleFeedbackv5/ext.articleFeedbackv5.js
@@ -8,312 +8,15 @@
99 'ext.articleFeedbackv5-tracking', mw.config.get( 'wgArticleFeedbackv5Tracking' )
1010 );
1111
12 -/**
13 - * Prefixes a key for cookies or events, with extension and version information
14 - *
15 - * @param event String: Name of event to prefix
16 - * @return String: Prefixed event name
17 - */
18 -function prefix( key ) {
19 - var version = mw.config.get( 'wgArticleFeedbackv5Tracking' ).version || 0;
20 - return 'ext.articleFeedbackv5@' + version + '-' + key;
21 -}
22 -
23 -/**
24 - * Checks if a pitch is currently muted
25 - *
26 - * @param pitch String: Name of pitch to check
27 - * @return Boolean: Whether the pitch is muted
28 - */
29 -function isPitchVisible( pitch ) {
30 - return $.cookie( prefix( 'pitches-' + pitch ) ) != 'hide';
31 -}
32 -
33 -/**
34 - * Ensures a pitch will be muted for a given duration of time
35 - *
36 - * @param pitch String: Name of pitch to mute
37 - * @param durration Integer: Number of days to mute the pitch for
38 - */
39 -function mutePitch( pitch, duration ) {
40 - $.cookie( prefix( 'pitches-' + pitch ), 'hide', { 'expires': duration, 'path': '/' } );
41 -}
42 -
4312 function trackClick( id ) {
4413 // Track the click so we can figure out how useful this is
4514 if ( tracked && $.isFunction( $.trackActionWithInfo ) ) {
46 - $.trackActionWithInfo( prefix( id ), mw.config.get( 'wgTitle' ) );
 15+ $.trackActionWithInfo( $.articleFeedbackv5.prefix( id ), mw.config.get( 'wgTitle' ) );
4716 }
4817 }
4918
50 -function trackClickURL( url, id ) {
51 - if ( tracked && $.isFunction( $.trackActionURL ) ) {
52 - return $.trackActionURL( url, prefix( id ) );
53 - } else {
54 - return url;
55 - }
56 -}
 19+var config = { };
5720
58 -/**
59 - * Survey object
60 - *
61 - * This object makes use of Special:SimpleSurvey, and uses some nasty hacks - this needs to be
62 - * replaced with something much better in the future.
63 - */
64 -var survey = new ( function() {
65 -
66 - /* Private Members */
67 -
68 - var that = this;
69 - var $dialog;
70 - var $form = $( [] );
71 - var $message = $( [] );
72 - // The form is rendered by loading the raw results of a special page into a div, this is the
73 - // URL of that special page
74 - var formSource = mw.config.get( 'wgScript' ) + '?' + $.param( {
75 - 'title': 'Special:SimpleSurvey',
76 - 'survey': 'articlerating',
77 - 'raw': 1
78 - } );
79 -
80 - /* Public Methods */
81 -
82 - this.load = function() {
83 - // Try to select existing dialog
84 - $dialog = $( '#articleFeedbackv5-dialog' );
85 - // Fall-back on creating one
86 - if ( $dialog.length === 0 ) {
87 - // Create initially in loading state
88 - $dialog = $( '<div id="articleFeedbackv5-dialog" class="loading" />' )
89 - .dialog( {
90 - 'width': 600,
91 - 'height': 400,
92 - 'bgiframe': true,
93 - 'autoOpen': true,
94 - 'modal': true,
95 - 'title': mw.msg( 'articlefeedbackv5-survey-title' ),
96 - 'close': function() {
97 - // Click tracking
98 - trackClick( 'survey-cancel' );
99 - // Return the survey to default state
100 - $dialog.dialog( 'option', 'height', 400 );
101 - $form.show();
102 - $message.remove();
103 - }
104 - } )
105 - .load( formSource, function() {
106 - $form = $dialog.find( 'form' );
107 - // Bypass normal form processing
108 - $form.submit( function() { return that.submit(); } );
109 - // Dirty hack - we want a fully styled button, and we can't get that from an
110 - // input[type=submit] control, so we just swap it out
111 - var $input = $form.find( 'input[type=submit]' );
112 - var $button = $( '<button type="submit"></button>' )
113 - .text( $(this).find( 'input[type=submit]' ).val() )
114 - .button()
115 - .insertAfter( $input );
116 - $input.remove();
117 - $form.find( '#prefswitch-survey-origin' ).text( mw.config.get( 'wgTitle' ) );
118 -
119 - // Insert disclaimer message
120 - $button.before(
121 - $( '<div>' )
122 - .addClass( 'articleFeedbackv5-survey-disclaimer' )
123 - // Can't use .text() with mw.message(, /* $1 */ link).toString(),
124 - // because 'link' should not be re-escaped (which would happen if done by mw.message)
125 - .html( function() {
126 - var link = mw.html.element(
127 - 'a', {
128 - href: mw.msg( 'articlefeedbackv5-privacyurl' )
129 - }, mw.msg( 'articlefeedbackv5-survey-disclaimerlink' )
130 - );
131 - return mw.html.escape( mw.msg( 'articlefeedbackv5-survey-disclaimer' ) )
132 - .replace( /\$1/, link );
133 - })
134 - );
135 -
136 - // Take dialog out of loading state
137 - $dialog.removeClass( 'loading' );
138 - } );
139 - }
140 - $dialog.dialog( 'open' );
141 - };
142 - this.submit = function() {
143 - $dialog = $( '#articleFeedbackv5-dialog' );
144 - // Put dialog into "loading" state
145 - $dialog
146 - .addClass( 'loading' )
147 - .find( 'form' )
148 - .hide();
149 - // Setup request to send information directly to a special page
150 - var data = {
151 - 'title': 'Special:SimpleSurvey'
152 - };
153 - // Build request from form data
154 - $dialog
155 - .find( [
156 - 'input[type=text]',
157 - 'input[type=radio]:checked',
158 - 'input[type=checkbox]:checked',
159 - 'input[type=hidden]',
160 - 'textarea'
161 - ].join( ',' ) )
162 - .each( function() {
163 - var name = $(this).attr( 'name' );
164 - if ( name !== '' ) {
165 - if ( name.substr( -2 ) == '[]' ) {
166 - var trimmedName = name.substr( 0, name.length - 2 );
167 - if ( typeof data[trimmedName] == 'undefined' ) {
168 - data[trimmedName] = [];
169 - }
170 - data[trimmedName].push( $(this).val() );
171 - } else {
172 - data[name] = $(this).val();
173 - }
174 - }
175 - } );
176 - // Click tracking
177 - trackClick( 'survey-submit-attempt' );
178 - // XXX: Not only are we submitting to a special page instead of an API request, but we are
179 - // screen-scraping the result - this is evil and needs to be addressed later
180 - $.ajax( {
181 - 'url': wgScript,
182 - 'type': 'POST',
183 - 'data': data,
184 - 'dataType': 'html',
185 - 'success': function( data ) {
186 - // Take the dialog out of "loading" state
187 - $dialog.removeClass( 'loading' );
188 - // Screen-scrape to determine success or error
189 - var success = $( data ).find( '.simplesurvey-success' ).size() > 0;
190 - // Display success/error message
191 - that.alert( success ? 'success' : 'error' );
192 - // Mute for 30 days
193 - mutePitch( 'survey', 30 );
194 - // Click tracking
195 - trackClick( 'survey-submit-complete' );
196 - },
197 - 'error': function( XMLHttpRequest, textStatus, errorThrown ) {
198 - // Take the dialog out of "loading" state
199 - $dialog.removeClass( 'loading' );
200 - // Display error message
201 - that.alert( 'error' );
202 - }
203 - } );
204 - // Do not continue with normal form processing
205 - return false;
206 - };
207 - this.alert = function( message ) {
208 - $message = $( '<div />' )
209 - .addClass( 'articleFeedbackv5-survey-message-' + message )
210 - .text( mw.msg( 'articlefeedbackv5-survey-message-' + message ) )
211 - .appendTo( $dialog );
212 - $dialog.dialog( 'option', 'height', $message.height() + 100 );
213 - };
214 -} )();
215 -
216 -var config = {
217 - 'pitches': {
218 - 'survey': {
219 - 'weight': 1,
220 - 'condition': function() {
221 - return isPitchVisible( 'survey' );
222 - },
223 - 'action': function() {
224 - survey.load();
225 - // Click tracking
226 - trackClick( 'pitch-survey-accept' );
227 - // Hide the pitch immediately
228 - return true;
229 - },
230 - 'title': 'articlefeedbackv5-pitch-thanks',
231 - 'message': 'articlefeedbackv5-pitch-survey-message',
232 - 'body': 'articlefeedbackv5-pitch-survey-body',
233 - 'accept': 'articlefeedbackv5-pitch-survey-accept',
234 - 'reject': 'articlefeedbackv5-pitch-reject'
235 - },
236 - 'join': {
237 - 'weight': 1,
238 - 'condition': function() {
239 - return isPitchVisible( 'join' ) && mw.user.anonymous();
240 - },
241 - 'action': function() {
242 - // Mute for 1 day
243 - mutePitch( 'join', 1 );
244 - // Go to account creation page
245 - // Track the click through an API redirect
246 - window.location = trackClickURL(
247 - mw.config.get( 'wgScript' ) + '?' + $.param( {
248 - 'title': 'Special:UserLogin',
249 - 'type': 'signup',
250 - 'returnto': mw.config.get( 'wgPageName' )
251 - } ), 'pitch-join-accept-signup' );
252 - return false;
253 - },
254 - 'title': 'articlefeedbackv5-pitch-thanks',
255 - 'message': 'articlefeedbackv5-pitch-join-message',
256 - 'body': 'articlefeedbackv5-pitch-join-body',
257 - 'accept': 'articlefeedbackv5-pitch-join-accept',
258 - 'reject': 'articlefeedbackv5-pitch-reject',
259 - // Special alternative action for going to login page
260 - 'altAccept': 'articlefeedbackv5-pitch-join-login',
261 - 'altAction': function() {
262 - // Mute for 1 day
263 - mutePitch( 'join', 1 );
264 - // Go to login page
265 - // Track the click through an API redirect
266 - window.location = trackClickURL(
267 - mw.config.get( 'wgScript' ) + '?' + $.param( {
268 - 'title': 'Special:UserLogin',
269 - 'returnto': mw.config.get( 'wgPageName' )
270 - } ), 'pitch-join-accept-login' );
271 - return false;
272 - }
273 - },
274 - 'edit': {
275 - 'weight': 2,
276 - 'condition': function() {
277 - // An empty restrictions array means anyone can edit
278 - var restrictions = mw.config.get( 'wgRestrictionEdit', [] );
279 - if ( restrictions.length ) {
280 - var groups = mw.config.get( 'wgUserGroups' );
281 - // Verify that each restriction exists in the user's groups
282 - for ( var i = 0; i < restrictions.length; i++ ) {
283 - if ( $.inArray( restrictions[i], groups ) < 0 ) {
284 - return false;
285 - }
286 - }
287 - }
288 - return isPitchVisible( 'edit' );
289 - },
290 - 'action': function() {
291 - // Mute for 7 days
292 - mutePitch( 'edit', 7 );
293 - // Setup edit page link
294 - var params = {
295 - 'title': mw.config.get( 'wgPageName' ),
296 - 'action': 'edit'
297 - };
298 - if ( tracked ) {
299 - // Keep track of tracked users' edits
300 - params.clicktrackingsession = $.cookie( 'clicktracking-session' );
301 - params.clicktrackingevent = prefix( 'pitch-edit-save' );
302 - }
303 - // Track the click through an API redirect (automatically bypasses if !tracked)
304 - window.location = trackClickURL(
305 - mw.config.get( 'wgScript' ) + '?' + $.param( params ), 'pitch-edit-accept'
306 - );
307 - return false;
308 - },
309 - 'title': 'articlefeedbackv5-pitch-thanks',
310 - 'message': 'articlefeedbackv5-pitch-edit-message',
311 - 'body': 'articlefeedbackv5-pitch-edit-body',
312 - 'accept': 'articlefeedbackv5-pitch-edit-accept',
313 - 'reject': 'articlefeedbackv5-pitch-reject'
314 - }
315 - }
316 -};
317 -
31821 /* Load at the bottom of the article */
31922 var $aftDiv = $( '<div id="mw-articlefeedbackv5"></div>' ).articleFeedbackv5( config );
32023
Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.php
@@ -203,6 +203,8 @@
204204 $wgHooks['ResourceLoaderGetConfigVars'][] = 'ArticleFeedbackv5Hooks::resourceLoaderGetConfigVars';
205205 $wgHooks['GetPreferences'][] = 'ArticleFeedbackv5Hooks::getPreferences';
206206 $wgHooks['SkinTemplateNavigation'][] = 'ArticleFeedbackv5Hooks::addFeedbackLink';
 207+$wgHooks['EditPage::showEditForm:fields'][] = 'ArticleFeedbackv5Hooks::pushTrackingFieldsToEdit';
 208+$wgHooks['ArticleSaveComplete'][] = 'ArticleFeedbackv5Hooks::trackEdit';
207209
208210 // API Registration
209211 $wgAPIListModules['articlefeedbackv5-view-ratings'] = 'ApiViewRatingsArticleFeedbackv5';
Index: trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5.php
@@ -94,7 +94,11 @@
9595 $this->getResult()->addValue(
9696 null,
9797 $this->getModuleName(),
98 - array( 'result' => 'Success' )
 98+ array(
 99+ 'result' => 'Success',
 100+ 'feedback_id' => $feedbackId,
 101+ 'cta_id' => $ctaId,
 102+ )
99103 );
100104 }
101105
Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.hooks.php
@@ -243,4 +243,48 @@
244244 );
245245 return true;
246246 }
 247+
 248+ /**
 249+ * Pushes the tracking fields into the edit page
 250+ *
 251+ * @see http://www.mediawiki.org/wiki/Manual:Hooks/EditPage::showEditForm:fields
 252+ */
 253+ public static function pushTrackingFieldsToEdit( $editPage, $output ) {
 254+ global $wgRequest;
 255+
 256+ $feedbackId = $wgRequest->getVal( 'articleFeedbackv5_feedback_id' );
 257+ $ctaId = $wgRequest->getVal( 'articleFeedbackv5_cta_id' );
 258+ $bucketId = $wgRequest->getVal( 'articleFeedbackv5_bucket_id' );
 259+
 260+ $editPage->editFormTextAfterContent .= Html::hidden( 'articleFeedbackv5_feedback_id', $feedbackId );
 261+ $editPage->editFormTextAfterContent .= Html::hidden( 'articleFeedbackv5_cta_id', $ctaId );
 262+ $editPage->editFormTextAfterContent .= Html::hidden( 'articleFeedbackv5_bucket_id', $bucketId );
 263+
 264+ return true;
 265+ }
 266+
 267+ /**
 268+ * Tracks edits
 269+ *
 270+ * @see http://www.mediawiki.org/wiki/Manual:Hooks/ArticleSaveComplete
 271+ */
 272+ public static function trackEdit( $article, $user, $text, $summary, $minoredit,
 273+ $watchthis, $sectionanchor, $flags, $revision, $baseRevId ) {
 274+ global $wgRequest;
 275+
 276+ $feedbackId = $wgRequest->getVal( 'articleFeedbackv5_feedback_id' );
 277+ $ctaId = $wgRequest->getVal( 'articleFeedbackv5_cta_id' );
 278+ $bucketId = $wgRequest->getVal( 'articleFeedbackv5_bucket_id' );
 279+
 280+error_log('Tracking!');
 281+error_log(var_export($feedbackId, true));
 282+error_log(var_export($ctaId, true));
 283+error_log(var_export($bucketId, true));
 284+error_log(var_export($article->getTitle()->getText(), true));
 285+
 286+ return true;
 287+ }
 288+
 289+
247290 }
 291+

Comments

#Comment by Johnduhart (talk | contribs)   13:56, 2 December 2011
+						msg = 'Unknown error';

Shouldn't this be localized?

#Comment by Rsterbin (talk | contribs)   04:01, 3 December 2011

Fixed in r105042

Status & tagging log