r90043 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r90042‎ | r90043 | r90044 >
Date:13:43, 14 June 2011
Author:werdna
Status:deferred (Comments)
Tags:
Comment:
Modified paths:
  • /trunk/extensions/MoodBar (added) (history)
  • /trunk/extensions/MoodBar/ApiMoodBar.php (added) (history)
  • /trunk/extensions/MoodBar/FeedbackItem.php (added) (history)
  • /trunk/extensions/MoodBar/Messages.php (added) (history)
  • /trunk/extensions/MoodBar/MoodBar.php (added) (history)
  • /trunk/extensions/MoodBar/moodbar.sql (added) (history)

Diff [purge]

Index: trunk/extensions/MoodBar/moodbar.sql
@@ -0,0 +1,30 @@
 2+CREATE TABLE /*_*/moodbar_feedback (
 3+ mbf_id int unsigned NOT NULL PRIMARY KEY auto_increment, -- Primary key
 4+ mbf_type varchar(32) binary NOT NULL, -- Type of feedback
 5+
 6+ -- User who provided the feedback
 7+ mbf_user_id int unsigned NOT NULL, -- User ID, or zero
 8+ mbf_user_ip varchar(255) binary NULL, -- If anonymous, user's IP address
 9+
 10+ -- Page where the feedback was received
 11+ -- Nullable.
 12+ mbf_namespace int,
 13+ mbf_title varchar(255) binary,
 14+
 15+ -- The feedback itself
 16+ mbf_comment varchar(255) binary,
 17+
 18+ -- Options and context
 19+ mbf_anonymous tinyint unsigned not null default 0, -- Anonymity
 20+ mbf_timestamp varchar(14) binary not null, -- When feedback was received
 21+ mbf_system_type varchar(64) binary null, -- Operating System
 22+ mbf_user_agent varchar(255) binary null, -- User-Agent header
 23+ mbf_locale varchar(32) binary null, -- The locale of the user's browser
 24+ mbf_editing tinyint unsigned not null, -- Whether or not the user was editing
 25+ mbf_bucket varchar(128) binary null -- Bucket, for A/B testing
 26+) /*$wgDBtableOptions*/;
 27+
 28+-- A little overboard with the indexes perhaps, but we want to be able to dice this data a lot!
 29+CREATE INDEX /*i*/type_timestamp ON /*_*/moodbar_feedback (mbf_type,mbf_timestamp);
 30+CREATE INDEX /*i*/user_timestamp ON /*_*/moodbar_feedback (mbf_user_id,mbf_user_ip,mbf_timestamp);
 31+CREATE INDEX /*i*/title_type ON /*_*/moodbar_feedback (mbf_namespace,mbf_title,mbf_type,mbf_timestamp);
Index: trunk/extensions/MoodBar/ApiMoodBar.php
@@ -0,0 +1,83 @@
 2+<?php
 3+
 4+class ApiMoodBar extends ApiBase {
 5+ public function execute() {
 6+ $params = $this->extractRequestParams();
 7+
 8+ $params['page'] = Title::newFromText( $params['page'] );
 9+
 10+ // Params are deliberately named the same as the properties,
 11+ // just slurp them through.
 12+ $item = MBFeedbackItem::create( $params );
 13+
 14+ $item->save();
 15+
 16+ $result = array( 'result' => 'success' );
 17+ $this->getResult()->addValue( null, $this->getModuleName(),
 18+ }
 19+
 20+ public function needsToken() {
 21+ return true;
 22+ }
 23+
 24+ public function getTokenSalt() {
 25+ return '';
 26+ }
 27+
 28+ public function getAllowedParams() {
 29+ return array(
 30+ 'page' => array(
 31+ ApiBase::PARAM_REQUIRED => true,
 32+ ),
 33+ 'type' => array(
 34+ ApiBase::PARAM_REQUIRED => true,
 35+ ApiBase::PARAM_TYPE => MBFeedbackItem::getValidTypes(),
 36+ ),
 37+ 'comment' => array(
 38+ ApiBase::PARAM_REQUIRED => true,
 39+ ),
 40+ 'anonymize' => array(
 41+ ApiBase::PARAM_TYPE => 'boolean',
 42+ ),
 43+ 'editmode' => array(
 44+ ApiBase::PARAM_TYPE => 'boolean',
 45+ ),
 46+ 'useragent' => null,
 47+ 'system' => null,
 48+ 'locale' => null,
 49+ 'bucket' => null,
 50+ );
 51+ }
 52+
 53+ public function mustBePosted() {
 54+ return true;
 55+ }
 56+
 57+ public function isWriteMode() {
 58+ return true;
 59+ }
 60+
 61+ public function getVersion() {
 62+ return __CLASS__ . ': $Id: ApiThreadAction.php 88574 2011-05-22 13:31:30Z werdna $';
 63+ }
 64+
 65+ public function getPossibleErrors() {
 66+ return array_merge( parent::getPossibleErrors(), array(
 67+
 68+ ) );
 69+ }
 70+
 71+ public function getParamDescription() {
 72+ return array(
 73+ 'page' => 'The page the feedback is on',
 74+ 'type' => 'The type of feedback being provided',
 75+ 'comment' => 'The feedback text',
 76+ 'anonymize' => 'Whether to hide user information',
 77+ 'editmode' => 'Whether or not the feedback context is in edit mode',
 78+ 'bucket' => 'The testing bucket, if any',
 79+ 'useragent' => 'The User-Agent header of the browser',
 80+ 'system' => 'The operating system being used',
 81+ 'locale' => 'The locale in use',
 82+ );
 83+ }
 84+}
Index: trunk/extensions/MoodBar/FeedbackItem.php
@@ -0,0 +1,237 @@
 2+<?php
 3+
 4+/**
 5+ * This class represents a single piece of MoodBar feedback.
 6+ */
 7+class MBFeedbackItem {
 8+ /** Container for the data **/
 9+ protected $data;
 10+ /** Valid data keys **/
 11+ protected $validMembers = array(
 12+ // Feedback essentials
 13+ 'id', // ID in the database
 14+ 'comment', // The feedback itself
 15+ 'page', // The page where it was submitted
 16+ 'type',
 17+
 18+ // Housekeeping
 19+ 'timestamp',
 20+ 'user', // User object who submitted the feedback
 21+ 'anonymize',
 22+
 23+ // Statistics
 24+ 'useragent',
 25+ 'system',
 26+ 'locale',
 27+ 'editmode',
 28+ 'bucket',
 29+ );
 30+
 31+ /** Valid values for the 'type' parameter. **/
 32+ protected static $validTypes = array( 'happy', 'sad', 'confused' );
 33+
 34+ /**
 35+ * Default constructor.
 36+ * Don't use this, use either MBFeedbackItem::newFromRow or MBFeedbackItem::create
 37+ */
 38+ protected function __construct() {
 39+ $this->data = array_fill_keys( $this->validMembers, null );
 40+
 41+ // Non-nullable boolean fields
 42+ $this->setProperty('anonymize', false);
 43+ $this->setProperty('editmode', false);
 44+ }
 45+
 46+ /**
 47+ * Factory function to create a new MBFeedbackItem
 48+ * @param $info Associative array of values
 49+ * Valid keys: type, user, comment, page, flags, timestamp,
 50+ * useragent, system, locale, bucket, anonymize
 51+ * @return MBFeedback object.
 52+ */
 53+ public static function create( $info ) {
 54+ $newObject = new self();
 55+ $newObject->initialiseNew( $info );
 56+ return $newObject;
 57+ }
 58+
 59+ /**
 60+ * Initialiser for new MBFeedbackItems
 61+ * @param $info Associative array of values
 62+ * @see MBFeedbackItem::create
 63+ */
 64+ protected function initialiseNew( $info ) {
 65+ global $wgUser;
 66+
 67+ $template = array(
 68+ 'user' => $wgUser,
 69+ 'timestamp' => wfTimestampNow(),
 70+ );
 71+
 72+ $this->setProperties( $template );
 73+ $this->setProperties( $info );
 74+ }
 75+
 76+ /**
 77+ * Factory function to load an MBFeedbackItem from a DB row.
 78+ * @param $row A row, from DatabaseBase::fetchObject
 79+ * @return MBFeedback object.
 80+ */
 81+ public static function load( $row ) {
 82+ $newObject = new self();
 83+ $newObject->initialiseFromRow( $row );
 84+ return $newObject;
 85+ }
 86+
 87+ /**
 88+ * Initialiser for MBFeedbackItems loaded from the database
 89+ * @param $row A row object from DatabaseBase::fetchObject
 90+ * @see MBFeedbackItem::load
 91+ */
 92+ protected function initialiseFromRow( $row ) {
 93+ $properties = array(
 94+ 'id' => $row->mbf_id,
 95+ 'type' => $row->mbf_type,
 96+ 'comment' => $row->mbf_comment,
 97+ 'timestamp' => $row->mbf_timestamp,
 98+ 'anonymize' => $row->mbf_anonymous,
 99+ 'useragent' => $row->mbf_user_agent,
 100+ 'system' => $row->mbf_system_type,
 101+ 'locale' => $row->mbf_locale,
 102+ 'bucket' => $row->mbf_bucket,
 103+ 'editmode' => $row->mbf_editing,
 104+ );
 105+
 106+ $properties['page'] = Title::makeTitleSafe( $row->mbf_namespace, $row->mbf_title );
 107+
 108+ if ( $row->mbf_user_id > 0 ) {
 109+ $properties['user'] = User::newFromId( $row->mbf_user_id );
 110+ } else {
 111+ $properties['user'] = User::newFromName( $row->mbf_user_ip );
 112+ }
 113+
 114+ $this->setProperties( $properties );
 115+ }
 116+
 117+ /**
 118+ * Set a group of properties. Throws an exception on invalid property.
 119+ * @param $values An associative array of properties to set.
 120+ */
 121+ public function setProperties( $values ) {
 122+ foreach( $values as $key => $value ) {
 123+ if ( ! $this->isValidKey($key) ) {
 124+ throw new MWException( "Attempt to set invalid property $key" );
 125+ }
 126+
 127+ if ( ! $this->validatePropertyValue($key, $value) ) {
 128+ throw new MWException( "Attempt to set invalid value for $key" );
 129+ }
 130+
 131+ $this->data[$key] = $value;
 132+ }
 133+ }
 134+
 135+ /**
 136+ * Set a group of values.
 137+ * @param $key The property to set.
 138+ * @param $value The value to give it.
 139+ */
 140+ public function setProperty( $key, $value ) {
 141+ $this->setProperties( array( $key => $value ) );
 142+ }
 143+
 144+ /**
 145+ * Get a property.
 146+ * @param $key The property to get
 147+ * @return The property value.
 148+ */
 149+ public function getProperty( $key ) {
 150+ if ( ! $this->isValidKey($key) ) {
 151+ throw new MWException( "Attempt to get invalid property $key" );
 152+ }
 153+
 154+ return $this->data[$key];
 155+ }
 156+
 157+ /**
 158+ * Check a property key for validity.
 159+ * If a property key is valid, it will be prefilled to NULL.
 160+ * @param $key The property key to check.
 161+ */
 162+ public function isValidKey( $key ) {
 163+ return in_array( $key, $this->validMembers );
 164+ }
 165+
 166+ /**
 167+ * Check if a property value is valid for that property
 168+ * @param $key The key of the property to check.
 169+ * @param $value The value to check
 170+ * @return Boolean
 171+ */
 172+ public function validatePropertyValue( $key, $value ) {
 173+ if ( $key == 'user' ) {
 174+ return $value instanceof User || $value instanceof StubUser;
 175+ } elseif ( $key == 'page' ) {
 176+ return $value instanceof Title || $value instanceof StubTitle;
 177+ } elseif ( $key == 'type' ) {
 178+ return in_array( $value, self::$validTypes );
 179+ }
 180+
 181+ return true;
 182+ }
 183+
 184+ /**
 185+ * Writes this MBFeedbackItem to the database.
 186+ * Throws an exception if this it is already in the database.
 187+ * @return The MBFeedbackItem's new ID.
 188+ */
 189+ public function save() {
 190+
 191+ if ( $this->getProperty('id') != null ) {
 192+ throw new MWException( "This ".__CLASS__." is already in the database." );
 193+ }
 194+
 195+ $dbw = wfGetDB( DB_MASTER );
 196+
 197+ $row = array(
 198+ 'mbf_id' => $dbw->nextSequenceValue( 'moodbar_feedback_mbf_id' ),
 199+ 'mbf_type' => $this->getProperty('type'),
 200+ 'mbf_comment' => $this->getProperty('comment'),
 201+ 'mbf_timestamp' => $dbw->timestamp($this->getProperty('timestamp')),
 202+ 'mbf_anonymous' => $this->getProperty('anonymize'),
 203+ 'mbf_user_agent' => $this->getProperty('useragent'),
 204+ 'mbf_system_type' => $this->getProperty('system'),
 205+ 'mbf_locale' => $this->getProperty('locale'),
 206+ 'mbf_bucket' => $this->getProperty('bucket'),
 207+ 'mbf_editing' => $this->getProperty('editmode'),
 208+ );
 209+
 210+ $user = $this->getProperty('user');
 211+ if ( $user->isAnon() ) {
 212+ $row['mbf_user_id'] = 0;
 213+ $row['mbf_user_ip'] = $user->getName();
 214+ } else {
 215+ $row['mbf_user_id'] = $user->getId();
 216+ }
 217+
 218+ $page = $this->getProperty('page');
 219+ if ( $page ) {
 220+ $row['mbf_namespace'] = $page->getNamespace();
 221+ $row['mbf_title'] = $page->getDBkey();
 222+ }
 223+
 224+ $dbw->insert( 'moodbar_feedback', $row, __METHOD__ );
 225+
 226+ $this->setProperty( 'id', $dbw->insertId() );
 227+
 228+ return $this->getProperty('id');
 229+ }
 230+
 231+ /**
 232+ * Gets the valid types of a feedback item.
 233+ */
 234+ public static function getValidTypes() {
 235+ return self::$validTypes;
 236+ }
 237+
 238+}
Index: trunk/extensions/MoodBar/MoodBar.php
@@ -0,0 +1,37 @@
 2+<?php
 3+/**
 4+ * MoodBar extension
 5+ * Allows specified users to send their "mood" back to the site operator.
 6+ */
 7+
 8+$wgExtensionCredits['other'][] = array(
 9+ 'author' => array( 'Andrew Garrett', 'Timo Tijhof' ),
 10+ 'descriptionmsg' => 'moodbar-desc',
 11+ 'name' => 'MoodBar',
 12+ 'url' => 'http://www.mediawiki.org/wiki/MoodBar',
 13+ 'version' => '0.1',
 14+ 'path' => __FILE__,
 15+);
 16+
 17+// Object model
 18+$wgAutoloadClasses['MBFeedbackItem'] = dirname(__FILE__).'/FeedbackItem.php';
 19+
 20+// API
 21+$wgAutoloadClasses['ApiMoodBar'] = dirname(__FILE__).'/ApiMoodBar.php';
 22+$wgAPIModules['moodbar'] = 'ApiMoodBar';
 23+
 24+// Internationalisation
 25+$wgExtensionMessagesFiles = dirname(__FILE__).'/Messages.php';
 26+
 27+// Resources
 28+$mbResourceTemplate = array(
 29+ 'localBasePath' => dirname(__FILE__),
 30+ 'remoteExtPath' => 'MoodBar'
 31+);
 32+
 33+$wgResourceModules['ext.moodBar'] = $mbResourceTemplate + array(
 34+ 'styles' => array( ),
 35+ 'scripts' => array( ),
 36+ 'dependencies' => array( ),
 37+ 'messages' => array( ),
 38+);
Index: trunk/extensions/MoodBar/Messages.php
@@ -0,0 +1,5 @@
 2+<?php
 3+
 4+$messages['en'] = array(
 5+ 'moodbar-desc' => 'Allows specified users to send their "mood" back to the site operator.",
 6+);

Follow-up revisions

RevisionCommit summaryAuthorDate
r90120Fix syntax error from r90043 in Api filereedy17:18, 15 June 2011

Comments

#Comment by Siebrand (talk | contribs)   13:48, 14 June 2011

Can you please rename the messages file to MoodBar.i18n.php as this is the current naming convention for messages files?

#Comment by Werdna (talk | contribs)   13:51, 14 June 2011

I prefer to use a different name because it makes autocomplete and scanning the file list easier — is there a convincing reason to stick to MoodBar.i18n.php other than "all the other extensions do it"?

#Comment by Siebrand (talk | contribs)   14:20, 14 June 2011

It takes more configuration to add support for the Translate extension, and all the other couple of hundred extensions do it. We have just taken a few years cleaning everything up. But if it is really important, just add a no rename request in the i18n file. People will understand.

#Comment by Werdna (talk | contribs)   14:22, 14 June 2011

I've renamed it, I didn't realise it caused issues with Translate.

#Comment by Krinkle (talk | contribs)   17:16, 15 June 2011

I couldn't get the update or api script to open. The updater crashed on trying to read a file from /maintenance/archive, the api:

Parse error: syntax error, unexpected '}' in /htdocs/SVN/mediawiki/trunk/extensions/MoodBar/ApiMoodBar.php on line 17

Can you investigate ? Perhaps look at ArticleFeedback as example.

#Comment by Werdna (talk | contribs)   02:16, 16 June 2011

Sorry about that, I have no idea how that bit disappeared — I must have messed something up between testing and committing.

Thanks for fixing this commit up, folks — looks like I was having a bad night. :-)

Status & tagging log