mediawiki.messagePoster: Minor code and docs clean up

* Improve and simpify various descriptions.

* Consistently use the imperative mood for the first line of
  method descriptions.

* create(): Refactor to be more stable and resilient.
  - Move variable declarations to inside the Deferred handler to avoid
    fragile code where data is transferred between different promise callbacks.
  - Remove check for 'result.query.pageids'. If it doesn't exist then neither
    does 'result.query' and it would throw either way.
  - Simplify check by asserting [0] directly instead of computing the length.
    Matches the actual code below.
  - Rename local variable 'errorCode' to match documentation of second param
    being 'error'. 'errorCode' is the first parameter passed to reject().
  - Outdent body and change to pattern of error-first returning.
  - Rename unexposed classname to be plain without MW prefix.

Change-Id: If642e94942abcbf7e5aa373fbd83a30d9202f24a

diff --git a/resources/src/mediawiki.messagePoster/mediawiki.messagePoster.factory.js b/resources/src/mediawiki.messagePoster/mediawiki.messagePoster.factory.js
index 69655a6..68fb2aa 100644 (file)
--- a/resources/src/mediawiki.messagePoster/mediawiki.messagePoster.factory.js
+++ b/resources/src/mediawiki.messagePoster/mediawiki.messagePoster.factory.js
@@ -1,52 +1,54 @@
-/*global OO*/
+/*global OO */
 ( function ( mw, $ ) {
        /**
-        * This is a factory for MessagePoster objects, which allows a pluggable to way to script leaving a
-        * talk page message.
+        * Factory for MessagePoster objects. This provides a pluggable to way to script the action
+        * of adding a message to someone's talk page.
         *
         * @class mw.messagePoster.factory
         * @singleton
         */
-       function MwMessagePosterFactory() {
+       function MessagePosterFactory() {
                this.contentModelToClass = {};
        }
 
-       OO.initClass( MwMessagePosterFactory );
+       OO.initClass( MessagePosterFactory );
 
        // Note: This registration scheme is currently not compatible with LQT, since that doesn't
-       // have its own content model, just islqttalkpage.  LQT pages will be passed to the wikitext
+       // have its own content model, just islqttalkpage. LQT pages will be passed to the wikitext
        // MessagePoster.
        /**
-        * Registers a MessagePoster subclass for a given content model.
+        * Register a MessagePoster subclass for a given content model.
         *
         * @param {string} contentModel Content model of pages this MessagePoster can post to
-        * @param {Function} messagePosterConstructor Constructor for MessagePoster
+        * @param {Function} constructor Constructor of a MessagePoster subclass
         */
-       MwMessagePosterFactory.prototype.register = function ( contentModel, messagePosterConstructor ) {
+       MessagePosterFactory.prototype.register = function ( contentModel, constructor ) {
                if ( this.contentModelToClass[ contentModel ] !== undefined ) {
-                       throw new Error( 'The content model \'' + contentModel + '\' is already registered.' );
+                       throw new Error( 'Content model "' + contentModel + '" is already registered' );
                }
 
-               this.contentModelToClass[ contentModel ] = messagePosterConstructor;
+               this.contentModelToClass[ contentModel ] = constructor;
        };
 
        /**
-        * Unregisters a given content model
-        * This is exposed for testing and should not normally be needed.
+        * Unregister a given content model.
+        * This is exposed for testing and should not normally be used.
         *
         * @param {string} contentModel Content model to unregister
         */
-       MwMessagePosterFactory.prototype.unregister = function ( contentModel ) {
+       MessagePosterFactory.prototype.unregister = function ( contentModel ) {
                delete this.contentModelToClass[ contentModel ];
        };
 
        /**
-        * Creates a MessagePoster, given a title.  A promise for this is returned.
-        * This works by determining the content model, then loading the corresponding
-        * module (which will register the MessagePoster class), and finally constructing it.
+        * Create a MessagePoster for given a title.
         *
-        * This does not require the message and should be called as soon as possible, so it does the
-        * API and ResourceLoader requests in the background.
+        * A promise for this is returned. It works by determining the content model, then loading
+        * the corresponding module (which registers the MessagePoster class), and finally constructing
+        * an object for the given title.
+        *
+        * This does not require the message and should be called as soon as possible, so that the
+        * API and ResourceLoader requests run in the background.
         *
         * @param {mw.Title} title Title that will be posted to
         * @param {string} [apiUrl] api.php URL if the title is on another wiki
@@ -57,61 +59,54 @@
         *   - error Error explanation
         *   - details Further error details
         */
-       MwMessagePosterFactory.prototype.create = function ( title, apiUrl ) {
-               var pageId, page, contentModel, moduleName, api,
-                       factory = this;
-
-               if ( apiUrl ) {
-                       api = new mw.ForeignApi( apiUrl );
-               } else {
-                       api = new mw.Api();
-               }
+       MessagePosterFactory.prototype.create = function ( title, apiUrl ) {
+               var factory = this,
+                       api = apiUrl ? new mw.ForeignApi( apiUrl ) : new mw.Api();
 
                return api.get( {
                        action: 'query',
                        prop: 'info',
                        indexpageids: true,
                        titles: title.getPrefixedDb()
-               } ).then( function ( result ) {
-                       if ( result.query.pageids && result.query.pageids.length > 0 ) {
-                               pageId = result.query.pageids[ 0 ];
-                               page = result.query.pages[ pageId ];
-
-                               contentModel = page.contentmodel;
-                               moduleName = 'mediawiki.messagePoster.' + contentModel;
-                               return mw.loader.using( moduleName ).then( function () {
-                                       return factory.createForContentModel(
-                                               contentModel,
-                                               title,
-                                               api
-                                       );
-                               }, function () {
-                                       return $.Deferred().reject( 'failed-to-load-module', 'Failed to load the \'' + moduleName + '\' module' );
-                               } );
-                       } else {
+               } ).then( function ( data ) {
+                       var pageId, page, contentModel, moduleName;
+                       if ( !data.query.pageids[ 0 ] ) {
                                return $.Deferred().reject( 'unexpected-response', 'Unexpected API response' );
                        }
-               }, function ( errorCode, details ) {
-                       return $.Deferred().reject( 'content-model-query-failed', errorCode, details );
-               } ).promise();
+                       pageId = data.query.pageids[ 0 ];
+                       page = data.query.pages[ pageId ];
+
+                       contentModel = page.contentmodel;
+                       moduleName = 'mediawiki.messagePoster.' + contentModel;
+                       return mw.loader.using( moduleName ).then( function () {
+                               return factory.createForContentModel(
+                                       contentModel,
+                                       title,
+                                       api
+                               );
+                       }, function () {
+                               return $.Deferred().reject( 'failed-to-load-module', 'Failed to load "' + moduleName + '"' );
+                       } );
+               }, function ( error, details ) {
+                       return $.Deferred().reject( 'content-model-query-failed', error, details );
+               } );
        };
 
        /**
         * Creates a MessagePoster instance, given a title and content model
         *
         * @private
-        *
         * @param {string} contentModel Content model of title
         * @param {mw.Title} title Title being posted to
         * @param {mw.Api} api mw.Api instance that the instance should use
         * @return {mw.messagePoster.MessagePoster}
         *
         */
-       MwMessagePosterFactory.prototype.createForContentModel = function ( contentModel, title, api ) {
+       MessagePosterFactory.prototype.createForContentModel = function ( contentModel, title, api ) {
                return new this.contentModelToClass[ contentModel ]( title, api );
        };
 
        mw.messagePoster = {
-               factory: new MwMessagePosterFactory()
+               factory: new MessagePosterFactory()
        };
 }( mediaWiki, jQuery ) );

diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.messagePoster.factory.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.messagePoster.factory.test.js
index 288b527..b3c4bee 100644 (file)
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.messagePoster.factory.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.messagePoster.factory.test.js
@@ -21,7 +21,7 @@
                        function () {
                                mw.messagePoster.factory.register( TEST_MODEL, testMessagePosterConstructor );
                        },
-                       new RegExp( 'The content model \'' + TEST_MODEL + '\' is already registered.' ),
+                       new RegExp( 'Content model "' + TEST_MODEL + '" is already registered' ),
                        'Throws exception is same model is registered a second time'
                );
        } );