Matthias Mullie has uploaded a new change for review.
https://gerrit.wikimedia.org/r/206826
Change subject: Get rid over queryMap overrides
......................................................................
Get rid over queryMap overrides
This override system was way too complex & passing code around
from method to method for no great reason.
There was only 1 slightly more complex prehandler: watch/unwatch.
That one has a conditional depending on an existing data in
queryMap.
Instead of waiting until the request is about to happen to
generate the queryMap, I'm doing that first. This now allows all
preHandlers to access that data - no more need for the overrides-
callback, or even passing around overrides at all: we can now
just pass the queryMap the preHandlers return.
This is a pre-req for a followup patch where those preHandlers'
returns will be chained into promise handlers, each adding
their own data to the mix. Without this, that was impossible.
Change-Id: Ie117b2f7ca380e88b1998d0d154675a5fcb5d9fa
---
M modules/engine/components/board/base/flow-board-api-events.js
M modules/engine/components/board/features/flow-board-loadmore.js
M modules/engine/components/board/features/flow-board-toc.js
M modules/engine/components/common/flow-component-events.js
M modules/engine/misc/flow-api.js
5 files changed, 83 insertions(+), 122 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow
refs/changes/26/206826/1
diff --git a/modules/engine/components/board/base/flow-board-api-events.js
b/modules/engine/components/board/base/flow-board-api-events.js
index e0de540..3a7b272 100644
--- a/modules/engine/components/board/base/flow-board-api-events.js
+++ b/modules/engine/components/board/base/flow-board-api-events.js
@@ -34,9 +34,11 @@
* real editor can be anything, depending on the type of editor)
*
* @param {Event} event
+ * @param {object} info
+ * @param {object} queryMap
* @return {Object}
*/
-
FlowBoardComponentApiEventsMixin.UI.events.globalApiPreHandlers.prepareEditor =
function ( event ) {
+
FlowBoardComponentApiEventsMixin.UI.events.globalApiPreHandlers.prepareEditor =
function ( event, info, queryMap ) {
var $textareas = $( this ).closest( 'form' ).find( 'textarea' ),
override = {};
@@ -59,7 +61,7 @@
// add its own nodes, which may be picked up by
serializeArray()
} );
- return override;
+ return $.extend( queryMap, override );
};
/**
@@ -68,14 +70,16 @@
* This will prepare the data-to-be-submitted so that the override is
* submitted against the most current revision ID.
* @param {Event} event
+ * @param {object} info
+ * @param {object} queryMap
* @return {Object}
*/
-
FlowBoardComponentApiEventsMixin.UI.events.globalApiPreHandlers.prepareEditConflict
= function ( event ) {
+
FlowBoardComponentApiEventsMixin.UI.events.globalApiPreHandlers.prepareEditConflict
= function ( event, info, queryMap ) {
var $form = $( this ).closest( 'form' ),
prevRevisionId = $form.data( 'flow-prev-revision' );
if ( !prevRevisionId ) {
- return {};
+ return queryMap;
}
// Get rid of the temp-saved new revision ID
@@ -87,81 +91,89 @@
* be properly applied for the respective API call; e.g.
* epprev_revision (for edit post)
*/
- return {
+ return $.extend( queryMap, {
flow_prev_revision: prevRevisionId
- };
+ } );
};
/**
* Before activating header, sends an overrideObject to the API to
modify the request params.
* @param {Event} event
+ * @param {object} info
+ * @param {object} queryMap
* @return {Object}
*/
-
FlowBoardComponentApiEventsMixin.UI.events.apiPreHandlers.activateEditHeader =
function () {
- return {
+
FlowBoardComponentApiEventsMixin.UI.events.apiPreHandlers.activateEditHeader =
function ( event, info, queryMap ) {
+ return $.extend( queryMap, {
submodule: 'view-header', // href submodule is
edit-header
vhformat: mw.flow.editor.getFormat() // href does not
have this param
- };
+ } );
};
/**
* Before activating topic, sends an overrideObject to the API to
modify the request params.
*
* @param {Event} event
+ * @param {object} info
+ * @param {object} queryMap
* @return {Object}
*/
-
FlowBoardComponentApiEventsMixin.UI.events.apiPreHandlers.activateEditTitle =
function ( event ) {
+
FlowBoardComponentApiEventsMixin.UI.events.apiPreHandlers.activateEditTitle =
function ( event, info, queryMap ) {
// Use view-post API for topic as well; we only want this on
// particular (title) post revision, not the full topic
- return {
+ return $.extend( queryMap, {
submodule: "view-post",
vppostId: $( this ).closest( '.flow-topic' ).data(
'flow-id' ),
vpformat: mw.flow.editor.getFormat()
- };
+ } );
};
/**
* Before activating post, sends an overrideObject to the API to modify
the request params.
* @param {Event} event
+ * @param {object} info
+ * @param {object} queryMap
* @return {Object}
*/
-
FlowBoardComponentApiEventsMixin.UI.events.apiPreHandlers.activateEditPost =
function ( event ) {
- return {
+
FlowBoardComponentApiEventsMixin.UI.events.apiPreHandlers.activateEditPost =
function ( event, info, queryMap ) {
+ return $.extend( queryMap, {
submodule: 'view-post',
vppostId: $( this ).closest( '.flow-post' ).data(
'flow-id' ),
vpformat: mw.flow.editor.getFormat()
- };
+ } );
};
/**
* Adjusts query params to use global watch action, and specifies it
should use a watch token.
* @param {Event} event
+ * @param {object} info
+ * @param {object} queryMap
* @returns {Function}
*/
- FlowBoardComponentApiEventsMixin.UI.events.apiPreHandlers.watchItem =
function ( event ) {
- return function ( queryMap ) {
- var params = {
- action: 'watch',
- titles: queryMap.page,
- _internal: {
- tokenType: 'watch'
- }
- };
- if ( queryMap.submodule === 'unwatch' ) {
- params.unwatch = 1;
+ FlowBoardComponentApiEventsMixin.UI.events.apiPreHandlers.watchItem =
function ( event, info, queryMap ) {
+ var params = {
+ action: 'watch',
+ titles: queryMap.page,
+ _internal: {
+ tokenType: 'watch'
}
- return params;
};
+ if ( queryMap.submodule === 'unwatch' ) {
+ params.unwatch = 1;
+ }
+
+ return params;
};
/**
* Before activating summarize topic, sends an overrideObject to the
* API to modify the request params.
* @param {Event} event
- * @param {Object} info
+ * @param {object} info
+ * @param {object} queryMap
* @return {Object}
*/
-
FlowBoardComponentApiEventsMixin.UI.events.apiPreHandlers.activateSummarizeTopic
= function ( event, info ) {
+
FlowBoardComponentApiEventsMixin.UI.events.apiPreHandlers.activateSummarizeTopic
= function ( event, info, queryMap ) {
if ( info.$target.find( 'form' ).length ) {
// Form already open; cancel the old form
var flowBoard = mw.flow.getPrototypeMethod( 'board',
'getInstanceByElement' )( $( this ) );
@@ -169,29 +181,31 @@
return false;
}
- return {
+ return $.extend( queryMap, {
// href submodule is edit-topic-summary
submodule: 'view-topic-summary',
// href does not have this param
vtsformat: mw.flow.editor.getFormat()
- };
+ } );
};
/**
* Before activating lock/unlock edit form, sends an overrideObject
* to the API to modify the request params.
* @param {Event} event
+ * @param {object} info
+ * @param {object} queryMap
* @return {Object}
*/
-
FlowBoardComponentApiEventsMixin.UI.events.apiPreHandlers.activateLockTopic =
function ( event ) {
- return {
+
FlowBoardComponentApiEventsMixin.UI.events.apiPreHandlers.activateLockTopic =
function ( event, info, queryMap ) {
+ return $.extend( queryMap, {
// href submodule is lock-topic
submodule: 'view-post',
// href does not have this param
vpformat: 'wikitext',
// request just the data for this topic
vppostId: $( this ).data( 'flow-id' )
- };
+ } );
};
//
diff --git a/modules/engine/components/board/features/flow-board-loadmore.js
b/modules/engine/components/board/features/flow-board-loadmore.js
index c7024a5..c08c0f8 100644
--- a/modules/engine/components/board/features/flow-board-loadmore.js
+++ b/modules/engine/components/board/features/flow-board-loadmore.js
@@ -159,9 +159,10 @@
* @param {Event} event
* @param {Object} info
* @param {jQuery} info.$target
+ * @param {object} queryMap
* @param {FlowBoardComponent} info.component
*/
- function flowBoardComponentLoadMoreFeatureBoardApiPreHandler( event,
info ) {
+ function flowBoardComponentLoadMoreFeatureBoardApiPreHandler( event,
info, queryMap ) {
// Backup the topic data
info.component.renderedTopicsBackup =
info.component.renderedTopics;
info.component.topicTitlesByIdBackup =
info.component.topicTitlesById;
diff --git a/modules/engine/components/board/features/flow-board-toc.js
b/modules/engine/components/board/features/flow-board-toc.js
index 4d64730..d9843c5 100644
--- a/modules/engine/components/board/features/flow-board-toc.js
+++ b/modules/engine/components/board/features/flow-board-toc.js
@@ -38,9 +38,10 @@
* @param {Event} event
* @param {Object} info
* @param {jQuery} info.$target
+ * @param {object} queryMap
* @param {FlowBoardComponent} info.component
*/
- function flowBoardComponentTocFeatureMixinBoardApiPreHandler( event,
info ) {
+ function flowBoardComponentTocFeatureMixinBoardApiPreHandler( event,
info, queryMap ) {
info.component.topicIdsInTocBackup =
info.component.topicIdsInToc;
info.component.lastTopicIdInTocBackup =
info.component.lastTopicIdInToc;
@@ -60,9 +61,10 @@
* @param {Event} event
* @param {Object} info
* @param {jQuery} info.$target
+ * @param {object} queryMap
* @param {FlowBoardComponent} info.component
*/
- function flowBoardComponentTocFeatureMixinTopicListApiPreHandler(
event, info, extraParameters ) {
+ function flowBoardComponentTocFeatureMixinTopicListApiPreHandler(
event, info, queryMap, extraParameters ) {
var $this = $( this ),
isLoadMoreButton = $this.data( 'flow-load-handler' )
=== 'loadMore',
overrides;
@@ -99,7 +101,7 @@
delete overrides.topiclist_sortby;
}
- return overrides;
+ return $.extend( queryMap, overrides );
}
FlowBoardComponentTocFeatureMixin.UI.events.apiPreHandlers.topicList =
flowBoardComponentTocFeatureMixinTopicListApiPreHandler;
diff --git a/modules/engine/components/common/flow-component-events.js
b/modules/engine/components/common/flow-component-events.js
index 8a20709..226fbd4 100644
--- a/modules/engine/components/common/flow-component-events.js
+++ b/modules/engine/components/common/flow-component-events.js
@@ -265,13 +265,13 @@
flowComponent = mw.flow.getPrototypeMethod(
'component', 'getInstanceByElement' )( $this ),
dataParams = $this.data(),
handlerName = dataParams.flowApiHandler,
- preHandlerReturns = [],
info = {
$target: null,
status: null,
component: flowComponent
},
- args = Array.prototype.slice.call( arguments, 0 );
+ args = Array.prototype.slice.call( arguments, 0 ),
+ queryMap = flowComponent.Api.getQueryMap( self.href ||
self );
event.preventDefault();
@@ -285,8 +285,10 @@
$target = $this;
}
+ // insert queryMap & info into args for prehandler
info.$target = $target;
- args.splice( 1, 0, info ); // insert info into args for
prehandler
+ args.splice( 1, 0, info );
+ args.splice( 2, 0, queryMap );
// Make sure an API call is not already in progress for this
target
if ( $target.closest( '.flow-api-inprogress' ).length ) {
@@ -301,7 +303,7 @@
// Let generic pre-handler take care of edit conflicts
$.each( flowComponent.UI.events.globalApiPreHandlers, function(
key, callbackArray ) {
$.each( callbackArray, function ( i, callbackFn ) {
- preHandlerReturns.push( callbackFn.apply( self,
args ) );
+ queryMap = callbackFn.apply( self, args );
} );
} );
@@ -313,15 +315,18 @@
if ( flowComponent.UI.events.apiPreHandlers[ handlerName ] ) {
// apiPreHandlers can return FALSE to prevent
processing,
// nothing at all to proceed,
- // or OBJECT to add param overrides to the API
- // or FUNCTION to modify API params
+ // or new queryMap (= API params)
$.each( flowComponent.UI.events.apiPreHandlers[
handlerName ], function ( i, callbackFn ) {
+ // make sure queryMap is up to date (may have
been altered by
+ // previous preHandler)
+ args.splice( 2, 1, queryMap );
preHandlerReturn = callbackFn.apply( self, args
);
- preHandlerReturns.push( preHandlerReturn );
if ( preHandlerReturn === false ) {
// Callback returned false; break out
of this loop
return false;
+ } else {
+ queryMap = preHandlerReturn;
}
} );
@@ -330,7 +335,7 @@
flowComponent.debug( false, 'apiPreHandler
returned false', handlerName, args );
// Abort any old request in flight; this is
normally done automatically by requestFromNode
- flowComponent.Api.abortOldRequestFromNode(
self, null, null, preHandlerReturns );
+ flowComponent.Api.abortOldRequestFromNode(
self, queryMap, null );
// @todo support for multiple indicators on
same target
$target.removeClass( 'flow-api-inprogress' );
@@ -341,7 +346,7 @@
}
// Make the request
- $deferred = flowComponent.Api.requestFromNode( self,
preHandlerReturns );
+ $deferred = flowComponent.Api.requestFromNode( self, queryMap );
if ( !$deferred ) {
mw.flow.debug( '[FlowApi] [interactiveHandlers]
apiRequest element is not anchor or form element' );
$deferred = $.Deferred();
diff --git a/modules/engine/misc/flow-api.js b/modules/engine/misc/flow-api.js
index d01c6ee..1949950 100644
--- a/modules/engine/misc/flow-api.js
+++ b/modules/engine/misc/flow-api.js
@@ -155,39 +155,12 @@
FlowApi.prototype.setDefaultSubmodule = flowApiSetDefaultSubmodule;
/**
- * Overrides (values of) queryMap with a provided override, which can
come
- * in the form of an object (which the queryMap will be extended with)
or as
- * a function (whose return value will replace queryMap)
- *
- * @param {Object} [queryMap]
- * @param {Function|Object} [override]
- * @returns {Object}
- */
- function flowOverrideQueryMap( queryMap, override ) {
- if ( override ) {
- switch ( typeof override ) {
- // If given an override object, extend our
queryMap with it
- case 'object':
- $.extend( queryMap, override );
- break;
- // If given an override function, call it and
make it return the new queryMap
- case 'function':
- queryMap = override( queryMap );
- break;
- }
- }
-
- return queryMap;
- }
-
- /**
* With a url (a://b.c/d?e=f&g#h) will return an object of key-value
pairs ({e:'f', g:''}).
* @param {String|Element} url
* @param {Object} [queryMap]
- * @param {Array<(Function|Object)>} [overrides]
* @returns {Object}
*/
- function flowApiGetQueryMap( url, queryMap, overrides ) {
+ function flowApiGetQueryMap( url, queryMap ) {
var uri,
queryKey,
queryValue,
@@ -195,7 +168,6 @@
$node, $form, formData;
queryMap = queryMap || {};
- overrides = overrides || [];
// If URL is an Element...
if ( typeof url !== 'string' ) {
@@ -261,11 +233,6 @@
// Default action is flow
queryMap.action = queryMap.action || 'flow';
- // Override the automatically generated queryMap
- for ( i = 0; i < overrides.length; i++ ) {
- queryMap = flowOverrideQueryMap( queryMap, overrides[i]
);
- }
-
// Use the API map to transform this data if necessary, eg.
queryMap = flowApiTransformMap( queryMap );
@@ -278,24 +245,13 @@
* Using a given form, parses its action, serializes the data, and
sends it as GET or POST depending on form method.
* With button, its name=value is serialized in. If button is an Event,
it will attempt to find the clicked button.
* Additional params can be set with data-flow-api-params on both the
clicked button or the form.
- * @param {Event|Element} [button]
- * @param {Array<(Function|Object)>} [overrides]
+ * @param {Event|Element} button
+ * @param {Object} queryMap
* @return {$.Deferred}
*/
- function flowApiRequestFromForm( button, overrides ) {
- var $deferred = $.Deferred(),
- $button = $( button ),
- method = $button.closest( 'form' ).attr( 'method' ) ||
'GET',
- queryMap;
-
- // Parse the form action to get the rest of the queryMap
- if ( !( queryMap = this.getQueryMap( button, null, overrides )
) ) {
- return $deferred.rejectWith( { error: 'Invalid form
action' } );
- }
-
- if ( !( queryMap.action ) ) {
- return $deferred.rejectWith( { error: 'Unknown action
for form' } );
- }
+ function flowApiRequestFromForm( button, queryMap ) {
+ var $button = $( button ),
+ method = $button.closest( 'form' ).attr( 'method' ) ||
'GET';
// Cancel any old form request, and also trigger a new one
return this.abortOldRequestFromNode( $button, queryMap, method
);
@@ -307,20 +263,12 @@
* Using a given anchor, parses its URL and sends it as a GET (default)
or POST depending on data-flow-api-method.
* Additional params can be set with data-flow-api-params.
* @param {Element} anchor
- * @param {Array<(Function|Object)>} [overrides]
+ * @param {Object} queryMap
* @return {$.Deferred}
*/
- function flowApiRequestFromAnchor( anchor, overrides ) {
+ function flowApiRequestFromAnchor( anchor, queryMap ) {
var $anchor = $( anchor ),
- $deferred = $.Deferred(),
- queryMap,
method = $anchor.data( 'flow-api-method' ) || 'GET';
-
- // Build the query map from this anchor's HREF
- if ( !( queryMap = this.getQueryMap( anchor.href, null,
overrides ) ) ) {
- mw.flow.debug( '[FlowApi] requestFromAnchor error:
invalid href', arguments );
- return $deferred.rejectWith( { error: 'Invalid href' }
);
- }
// Abort any old requests, and have it issue a new one via GET
or POST
return this.abortOldRequestFromNode( $anchor, queryMap, method
);
@@ -331,10 +279,10 @@
/**
* Automatically calls requestFromAnchor or requestFromForm depending
on the type of node given.
* @param {Element} node
- * @param {Array<(Function|Object)>} [overrides]
- * @return {$.Deferred|bool}
+ * @param {Object} queryMap
+ * @return {$.Deferred}
*/
- function flowApiRequestFromNode( node, overrides ) {
+ function flowApiRequestFromNode( node, queryMap ) {
var $node = $( node );
if ( $node.is( 'a' ) ) {
@@ -342,7 +290,7 @@
} else if ( $node.is( 'form, input, button, textarea, select,
option' ) ) {
return this.requestFromForm.apply( this, arguments );
} else {
- return false;
+ return $.Deferred().reject();
}
}
@@ -352,21 +300,12 @@
* Handles aborting an old in-flight API request.
* If startNewMethod is given, this method also STARTS a new API call
and stores it for later abortion if needed.
* @param {jQuery|Element} $node
- * @param {Object} [queryMap]
+ * @param {Object} queryMap
* @param {String} [startNewMethod] If given: starts, stores, and
returns a new API call
- * @param {Array<(Function|Object)>} [overrides]
* @return {undefined|$.Deferred}
*/
- function flowApiAbortOldRequestFromNode( $node, queryMap,
startNewMethod, overrides ) {
+ function flowApiAbortOldRequestFromNode( $node, queryMap,
startNewMethod ) {
$node = $( $node );
-
- if ( !queryMap ) {
- // Get the queryMap automatically if one wasn't given
- if ( !( queryMap = this.getQueryMap( $node, null,
overrides ) ) ) {
- mw.flow.debug( '[FlowApi]
abortOldRequestFromNode failed to find a queryMap', arguments );
- return;
- }
- }
// If this anchor already has a request in flight, abort it
var str = 'flow-api-query-temp-' + queryMap.action + '-' +
queryMap.submodule,
--
To view, visit https://gerrit.wikimedia.org/r/206826
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie117b2f7ca380e88b1998d0d154675a5fcb5d9fa
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Matthias Mullie <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
