r104646 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r104645‎ | r104646 | r104647 >
Date:01:57, 30 November 2011
Author:rsterbin
Status:deferred
Tags:
Comment:
Buckets 2 and 3 are now being stored
* NB: Select rollups are still broken.
- sql/ArticleFeedbackv5.sql:
- Bug fix for aft_article_field.afi_data_type: ENUM should have
"option_id" rather than "select"
- Added aft_article_field_option.afo_field_id
- Inserts for buckets 2 and 3 fields, and options for bucket 2
- Added temporary file sql/alter.sql, to make tomorrow's push to Prototype
a little easier
- modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js:
- Removed localValidation() from the buckets that aren't using it --
all of them, except bucket 5
- Removed setSuccessState() from all the buckets, as it's no longer
usable, and associated references to $.articleFeedbackv5.successTimeout
- Filled in getFormData() for buckets 2 and 3
- ApiArticleFeedbackv5:
- Updated validateParam() to validate options:
- Now takes a reference to the value, so that the option text can
be replaced with an id
- The field id is passed in
- Returns false if there's no match in the options list
- ApiArticleFeedbackv5Utils:
- New method getOptions()
Modified paths:
  • /trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5Utils.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/sql/ArticleFeedbackv5.sql (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/sql/alter.sql (added) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedbackv5/sql/ArticleFeedbackv5.sql
——@@ -47,7 +47,7 @@
4848 CREATE TABLE IF NOT EXISTS /*_*/aft_article_field (
4949 afi_id integer unsigned NOT NULL PRIMARY KEY AUTO_INCREMENT,
5050 afi_name varchar(255) NOT NULL,
51 - afi_data_type ENUM('text', 'boolean', 'rating', 'select'),
 51+ afi_data_type ENUM('text', 'boolean', 'rating', 'option_id'),
5252 -- FKey to article_field_groups.group_id
5353 afi_group_id integer unsigned NULL,
5454 afi_bucket_id integer unsigned NOT NULL
——@@ -55,6 +55,7 @@
5656
5757 CREATE TABLE IF NOT EXISTS /*_*/aft_article_field_option (
5858 afo_option_id integer unsigned NOT NULL PRIMARY KEY AUTO_INCREMENT,
 59+ afo_field_id integer unsigned NOT NULL,
5960 afo_name varchar(255) NOT NULL
6061 ) /*$wgDBTableOptions*/;
6162
——@@ -120,6 +121,10 @@
121122
122123 INSERT INTO aft_article_field(afi_name, afi_data_type, afi_bucket_id) VALUES ('found', 'boolean', 1);
123124 INSERT INTO aft_article_field(afi_name, afi_data_type, afi_bucket_id) VALUES ('comment', 'text', 1);
 125+INSERT INTO aft_article_field(afi_name, afi_data_type, afi_bucket_id) VALUES ('tag', 'option_id', 2);
 126+INSERT INTO aft_article_field(afi_name, afi_data_type, afi_bucket_id) VALUES ('comment', 'text', 2);
 127+INSERT INTO aft_article_field(afi_name, afi_data_type, afi_bucket_id) VALUES ('rating', 'rating', 3);
 128+INSERT INTO aft_article_field(afi_name, afi_data_type, afi_bucket_id) VALUES ('comment', 'text', 3);
124129 INSERT INTO aft_article_field(afi_name, afi_data_type, afi_bucket_id) VALUES ('trustworthy', 'rating', 5);
125130 INSERT INTO aft_article_field(afi_name, afi_data_type, afi_bucket_id) VALUES ('objective', 'rating', 5);
126131 INSERT INTO aft_article_field(afi_name, afi_data_type, afi_bucket_id) VALUES ('complete', 'rating', 5);
——@@ -130,3 +135,12 @@
131136 INSERT INTO aft_article_field(afi_name, afi_data_type, afi_bucket_id) VALUES ('expertise-hobby', 'boolean', 5);
132137 INSERT INTO aft_article_field(afi_name, afi_data_type, afi_bucket_id) VALUES ('expertise-other', 'boolean', 5);
133138
 139+INSERT INTO aft_article_field_option (afo_field_id, afo_name)
 140+ SELECT afi_id, 'suggestion' FROM aft_article_field WHERE afi_name = 'tag' AND afi_bucket_id = 2;
 141+INSERT INTO aft_article_field_option (afo_field_id, afo_name)
 142+ SELECT afi_id, 'question' FROM aft_article_field WHERE afi_name = 'tag' AND afi_bucket_id = 2;
 143+INSERT INTO aft_article_field_option (afo_field_id, afo_name)
 144+ SELECT afi_id, 'problem' FROM aft_article_field WHERE afi_name = 'tag' AND afi_bucket_id = 2;
 145+INSERT INTO aft_article_field_option (afo_field_id, afo_name)
 146+ SELECT afi_id, 'praise' FROM aft_article_field WHERE afi_name = 'tag' AND afi_bucket_id = 2;
 147+
Index: trunk/extensions/ArticleFeedbackv5/sql/alter.sql
——@@ -0,0 +1,23 @@
 2+--
 3+-- This is a temporary file containing the changes necessary to bring prototype
 4+-- up to speed; it will be removed when the upgrade process is built.
 5+--
 6+
 7+ALTER TABLE aft_article_field CHANGE afi_data_type afi_data_type ENUM('text', 'boolean', 'rating', 'option_id');
 8+ALTER TABLE aft_article_field_option ADD COLUMN afo_field_id integer unsigned NOT NULL;
 9+
 10+INSERT INTO aft_article_field(afi_name, afi_data_type, afi_bucket_id) VALUES ('tag', 'option_id', 2);
 11+INSERT INTO aft_article_field(afi_name, afi_data_type, afi_bucket_id) VALUES ('comment', 'text', 2);
 12+INSERT INTO aft_article_field(afi_name, afi_data_type, afi_bucket_id) VALUES ('rating', 'rating', 3);
 13+INSERT INTO aft_article_field(afi_name, afi_data_type, afi_bucket_id) VALUES ('comment', 'text', 3);
 14+
 15+INSERT INTO aft_article_field_option (afo_field_id, afo_name)
 16+ SELECT afi_id, 'suggestion' FROM aft_article_field WHERE afi_name = 'tag' AND afi_bucket_id = 2;
 17+INSERT INTO aft_article_field_option (afo_field_id, afo_name)
 18+ SELECT afi_id, 'question' FROM aft_article_field WHERE afi_name = 'tag' AND afi_bucket_id = 2;
 19+INSERT INTO aft_article_field_option (afo_field_id, afo_name)
 20+ SELECT afi_id, 'problem' FROM aft_article_field WHERE afi_name = 'tag' AND afi_bucket_id = 2;
 21+INSERT INTO aft_article_field_option (afo_field_id, afo_name)
 22+ SELECT afi_id, 'praise' FROM aft_article_field WHERE afi_name = 'tag' AND afi_bucket_id = 2;
 23+
 24+
Property changes on: trunk/extensions/ArticleFeedbackv5/sql/alter.sql
___________________________________________________________________
Added: svn:eol-style
125 + native
Added: svn:keywords
226 + Author Date Id HeadURL Revision
Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js
——@@ -355,25 +355,6 @@
356356 },
357357
358358 // }}}
359 - // {{{ localValidation
360 -
361 - /**
362 - * Performs any local validation
363 - *
364 - * @param object formdata the form data
365 - * @return mixed if ok, false; otherwise, an object as { 'field name' : 'message' }
366 - */
367 - localValidation: function ( formdata ) {
368 - var error = {};
369 - var ok = true;
370 - if ( $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-bucket1-toggle input[checked]' ).length < 1 ) {
371 - error.toggle = 'Please select an option';
372 - ok = false;
373 - }
374 - return ok ? false : error;
375 - },
376 -
377 - // }}}
378359 // {{{ markFormError
379360
380361 /**
——@@ -389,21 +370,6 @@
390371 mw.log( 'Validation error' );
391372 }
392373 console.log( error );
393 - },
394 -
395 - // }}}
396 - // {{{ setSuccessState
397 -
398 - /**
399 - * Sets the success state
400 - */
401 - setSuccessState: function () {
402 - var $h = $.articleFeedbackv5.$holder;
403 - $h.find( '.articleFeedbackv5-success span' ).fadeIn( 'fast' );
404 - $.articleFeedbackv5.successTimeout = setTimeout( function () {
405 - $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-success span' )
406 - .fadeOut( 'slow' );
407 - }, 5000 );
408374 }
409375
410376 // }}}
——@@ -652,7 +618,6 @@
653619 for ( t in $.articleFeedbackv5.currentBucket().commentDefault ) {
654620 if ( $c.val() == $.articleFeedbackv5.currentBucket().commentDefault[t] ) {
655621 empty = true;
656 - break;
657622 }
658623 }
659624 }
——@@ -678,29 +643,17 @@
679644 */
680645 getFormData: function () {
681646 var data = {};
 647+ data.tag = $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-tags input[checked]' ).val();
 648+ data.comment = $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-comment textarea' ).val();
 649+ for ( t in $.articleFeedbackv5.currentBucket().commentDefault ) {
 650+ if ( data.comment == $.articleFeedbackv5.currentBucket().commentDefault[t] ) {
 651+ data.comment = '';
 652+ }
 653+ }
682654 return data;
683655 },
684656
685657 // }}}
686 - // {{{ localValidation
687 -
688 - /**
689 - * Performs any local validation
690 - *
691 - * @param object formdata the form data
692 - * @return mixed if ok, false; otherwise, an object as { 'field name' : 'message' }
693 - */
694 - localValidation: function ( formdata ) {
695 - var error = {};
696 - var ok = true;
697 - if ( $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-bucket2-toggle input[checked]' ).length < 1 ) {
698 - error.toggle = 'Please select an option';
699 - ok = false;
700 - }
701 - return ok ? false : error;
702 - },
703 -
704 - // }}}
705658 // {{{ markFormError
706659
707660 /**
——@@ -716,21 +669,6 @@
717670 mw.log( 'Validation error' );
718671 }
719672 console.log( error );
720 - },
721 -
722 - // }}}
723 - // {{{ setSuccessState
724 -
725 - /**
726 - * Sets the success state
727 - */
728 - setSuccessState: function () {
729 - var $h = $.articleFeedbackv5.$holder;
730 - $h.find( '.articleFeedbackv5-success span' ).fadeIn( 'fast' );
731 - $.articleFeedbackv5.successTimeout = setTimeout( function () {
732 - $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-success span' )
733 - .fadeOut( 'slow' );
734 - }, 5000 );
735673 }
736674
737675 // }}}
——@@ -990,29 +928,15 @@
991929 */
992930 getFormData: function () {
993931 var data = {};
 932+ data.rating = $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-rating input:hidden' ).val();
 933+ data.comment = $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-comment textarea' ).val();
 934+ if ( data.comment == mw.msg( 'articlefeedbackv5-bucket3-comment-default' ) ) {
 935+ data.comment = '';
 936+ }
994937 return data;
995938 },
996939
997940 // }}}
998 - // {{{ localValidation
999 -
1000 - /**
1001 - * Performs any local validation
1002 - *
1003 - * @param object formdata the form data
1004 - * @return mixed if ok, false; otherwise, an object as { 'field name' : 'message' }
1005 - */
1006 - localValidation: function ( formdata ) {
1007 - var error = {};
1008 - var ok = true;
1009 - if ( $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-bucket3-toggle input[checked]' ).length < 1 ) {
1010 - error.toggle = 'Please select an option';
1011 - ok = false;
1012 - }
1013 - return ok ? false : error;
1014 - },
1015 -
1016 - // }}}
1017941 // {{{ markFormError
1018942
1019943 /**
——@@ -1028,21 +952,6 @@
1029953 mw.log( 'Validation error' );
1030954 }
1031955 console.log( error );
1032 - },
1033 -
1034 - // }}}
1035 - // {{{ setSuccessState
1036 -
1037 - /**
1038 - * Sets the success state
1039 - */
1040 - setSuccessState: function () {
1041 - var $h = $.articleFeedbackv5.$holder;
1042 - $h.find( '.articleFeedbackv5-success span' ).fadeIn( 'fast' );
1043 - $.articleFeedbackv5.successTimeout = setTimeout( function () {
1044 - $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-success span' )
1045 - .fadeOut( 'slow' );
1046 - }, 5000 );
1047956 }
1048957
1049958 // }}}
——@@ -1226,7 +1135,6 @@
12271136 <div style="clear:both;"></div>\
12281137 </div>\
12291138 <button class="articleFeedbackv5-submit articleFeedbackv5-visibleWith-form" type="submit" disabled="disabled"><html:msg key="bucket5-form-panel-submit" /></button>\
1230 - <div class="articleFeedbackv5-success articleFeedbackv5-visibleWith-form"><span><html:msg key="bucket5-form-panel-success" /></span></div>\
12311139 <div class="articleFeedbackv5-pending articleFeedbackv5-visibleWith-form"><span><html:msg key="bucket5-form-panel-pending" /></span></div>\
12321140 <div style="clear:both;"></div>\
12331141 <div class="articleFeedbackv5-notices articleFeedbackv5-visibleWith-form">\
——@@ -1546,7 +1454,6 @@
15471455 enableSubmission: function ( state ) {
15481456 var $h = $.articleFeedbackv5.$holder;
15491457 if ( state ) {
1550 - $h.find( '.articleFeedbackv5-success span' ).hide();
15511458 $h.find( '.articleFeedbackv5-pending span' ).fadeIn( 'fast' );
15521459 } else {
15531460 $h.find( '.articleFeedbackv5-pending span' ).hide();
——@@ -1753,21 +1660,6 @@
17541661 },
17551662
17561663 // }}}
1757 - // {{{ setSuccessState
1758 -
1759 - /**
1760 - * Sets the success state
1761 - */
1762 - setSuccessState: function () {
1763 - var $h = $.articleFeedbackv5.$holder;
1764 - $h.find( '.articleFeedbackv5-success span' ).fadeIn( 'fast' );
1765 - $.articleFeedbackv5.successTimeout = setTimeout( function () {
1766 - $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-success span' )
1767 - .fadeOut( 'slow' );
1768 - }, 5000 );
1769 - },
1770 -
1771 - // }}}
17721664 // {{{ onSubmit
17731665
17741666 /**
——@@ -2020,9 +1912,6 @@
20211913 $.articleFeedbackv5.enableSubmission = function ( state ) {
20221914 var $h = $.articleFeedbackv5.$holder;
20231915 if ( state ) {
2024 - if ($.articleFeedbackv5.successTimeout) {
2025 - clearTimeout( $.articleFeedbackv5.successTimeout );
2026 - }
20271916 $h.find( '.articleFeedbackv5-submit' ).button( { 'disabled': false } );
20281917 } else {
20291918 $h.find( '.articleFeedbackv5-submit' ).button( { 'disabled': true } );
Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.php
——@@ -50,7 +50,7 @@
5151 // The rating categories for bucket 5 -- these MUST match the field names in the database.
5252 $wgArticleFeedbackv5Bucket5RatingCategories = array( 'trustworthy', 'objective', 'complete', 'wellwritten' );
5353
54 -// The tag names for bucket 5.
 54+// The tag names and values for bucket 2 -- these MUST match the option names in the database.
5555 $wgArticleFeedbackv5Bucket2TagNames = array( 'suggestion', 'question', 'problem', 'praise' );
5656
5757 // Bucket settings for display options
Index: trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5.php
——@@ -57,7 +57,7 @@
5858 if ( $value === '' ) {
5959 continue;
6060 }
61 - if ( $this->validateParam( $value, $type ) ) {
 61+ if ( $this->validateParam( $value, $type, $field->afi_id ) ) {
6262 $data = array(
6363 'aa_feedback_id' => $feedbackId,
6464 'aa_field_id' => $field->afi_id,
——@@ -105,15 +105,17 @@
106106 /**
107107 * Validates a value against a field type
108108 *
109 - * @param $value mixed the value
110 - * @param $type string the field type
111 - * @return bool whether this is okay
 109+ * @param $value mixed the value (reference, as option_id switches out
 110+ * text for the id)
 111+ * @param $type string the field type
 112+ * @param $field_id int the field id
 113+ * @return bool whether this is okay
112114 */
113 - protected function validateParam( $value, $type ) {
 115+ protected function validateParam( &$value, $type, $field_id ) {
114116 # rating: int between 1 and 5 (inclusive)
115117 # boolean: 1 or 0
116 - # option: option exists
117 - # text: none (maybe xss encode)
 118+ # option_id: option exists
 119+ # text: none (maybe xss encode)
118120 switch ( $type ) {
119121 case 'rating':
120122 if ( preg_match( '/^(1|2|3|4|5)$/', $value ) ) {
——@@ -125,8 +127,16 @@
126128 return true;
127129 }
128130 break;
129 - case 'option':
130 - # TODO: check for option existance.
 131+ case 'option_id':
 132+ $options = ApiArticleFeedbackv5Utils::getOptions();
 133+ if ( !isset( $options[$field_id] ) ) {
 134+ return false;
 135+ }
 136+ $flip = array_flip( $options[$field_id] );
 137+ if ( isset( $flip[$value] ) ) {
 138+ $value = $flip[$value];
 139+ return true;
 140+ }
131141 break;
132142 case 'text':
133143 return true;
——@@ -189,10 +199,10 @@
190200 }
191201
192202 $rows = $dbr->select(
193 - array(
194 - 'aft_article_answer',
195 - 'aft_article_feedback',
196 - 'aft_article_field'
 203+ array(
 204+ 'aft_article_answer',
 205+ 'aft_article_feedback',
 206+ 'aft_article_field'
197207 ),
198208 $select,
199209 array(
——@@ -242,7 +252,7 @@
243253 $rev_prefix . 'revision_id' => $revId,
244254 $rev_prefix . 'total' => $row->earned,
245255 $rev_prefix . 'count' => $count,
246 - $rev_prefix . $field => $value
 256+ $rev_prefix . $field => $value
247257 );
248258
249259 $page_data[$key][$page_prefix . 'total'] += $row->earned;
Index: trunk/extensions/ArticleFeedbackv5/api/ApiArticleFeedbackv5Utils.php
——@@ -126,5 +126,34 @@
127127 return $rv;
128128 }
129129
 130+ /**
 131+ * Gets the known feedback options
 132+ *
 133+ * Pulls all the rows in the aft_article_field_option table, then arranges
 134+ * them like so:
 135+ * {field id} => array(
 136+ * {option id} => {option name},
 137+ * ),
 138+ *
 139+ * TODO: use memcache
 140+ *
 141+ * @return array the rows in the aft_article_field_option table
 142+ */
 143+ public static function getOptions() {
 144+ $dbr = wfGetDB( DB_SLAVE );
 145+ $rows = $dbr->select(
 146+ 'aft_article_field_option',
 147+ array( 'afo_option_id', 'afo_field_id', 'afo_name' )
 148+ );
 149+ $rv = array();
 150+ foreach ( $rows as $row ) {
 151+ if ( !isset( $rv[$row->afo_field_id] ) ) {
 152+ $rv[$row->afo_field_id] = array();
 153+ }
 154+ $rv[$row->afo_field_id][$row->afo_option_id] = $row->afo_name;
 155+ }
 156+ return $rv;
 157+ }
 158+
130159 }
131160

Status & tagging log