From: Timo Tijhof Date: Wed, 23 Dec 2015 01:18:15 +0000 (-0800) Subject: mediawiki.messagePoster: Minor code and docs clean up X-Git-Tag: 1.31.0-rc.0~8434^2 X-Git-Url: http://git.cyclocoop.org/%22.%24h.%22?a=commitdiff_plain;h=144bbb321c33c2d44e34eda9722e23b25087718e;p=lhc%2Fweb%2Fwiklou.git 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 69655a6c8f..68fb2aae0e 100644 --- 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 288b527b64..b3c4beeb4e 100644 --- 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' ); } );