jenkins-bot has submitted this change and it was merged. Change subject: Make lockTopic use _flowBoardComponentRefreshTopic ......................................................................
Make lockTopic use _flowBoardComponentRefreshTopic It was already doing exactly what _flowBoardComponentRefreshTopic does now, because it couldn't "just" make the write API respond with what frontend needed. Bug: T95773 Change-Id: I221d2f032c824c309b0ce9f9dcdc83bdbb61587f --- M modules/engine/components/board/base/flow-board-api-events.js M tests/qunit/engine/components/board/test_flow-board.js 2 files changed, 25 insertions(+), 72 deletions(-) Approvals: EBernhardson: Looks good to me, approved jenkins-bot: Verified 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 743d379..a68bc41 100644 --- a/modules/engine/components/board/base/flow-board-api-events.js +++ b/modules/engine/components/board/base/flow-board-api-events.js @@ -368,68 +368,15 @@ * @returns {$.Promise} */ FlowBoardComponentApiEventsMixin.UI.events.apiHandlers.lockTopic = function ( info, data ) { - var $replacement, - $target = info.$target, - $this = $( this ), - $deferred = $.Deferred(), - flowBoard = mw.flow.getPrototypeMethod( 'board', 'getInstanceByElement' )( $this ), - flowId = $this.closest( '.flow-topic' ).data( 'flow-id' ); - if ( info.status !== 'done' ) { // Error will be displayed by default & edit conflict handled, nothing else to wrap up - return $deferred.reject().promise(); + return $.Deferred().reject().promise(); } - // We couldn't make lock-topic to return topic data after a successful - // post submission because lock-topic is used for no-js support as well. - // If we make it return topic data, that means it has to return wikitext format - // for edit form in no-js mode. This is a performance problem for wikitext - // conversion since topic data returns all children data as well. So we need to - // make lock-topic return a single post for topic then fire - // another request to topic data in html format - // - // @todo the html could json encode the parameters including topics, the js - // could then import that and continuously update it with new revisions from - // api calls. Rendering a topic would then just be pointing the template at - // the right part of that data instead of requesting it. - flowBoard.Api.apiCall( { - action: 'flow', - submodule: 'view-topic', - workflow: flowId, - // Flow topic title, in Topic:<topicId> format (2600 is topic namespace id) - page: mw.Title.newFromText( flowId, 2600 ).getPrefixedDb() - // @todo fixme - // - mw.Title.newFromText can return null. If you're not going to check its return - // value, use 'new mw.Title' instead so that you get an exception for 'invalid title' - // instead of an exception for 'property of null'. - // - The second parameter to mw.Title is 'defaultNamespace' not 'namespace'. - // E.g. mw.Title.newFromText( 'User:Example', 6 ) -> 'User:Example', not 'File: - // If you need to prefix/enforce a namespace, use the canonical prefix instead. - } ).done( function( result ) { - // Update view of the full topic - $replacement = $( flowBoard.constructor.static.TemplateEngine.processTemplateGetFragment( - 'flow_topiclist_loop.partial', - result.flow['view-topic'].result.topic - ) ).children(); - - $target.replaceWith( $replacement ); - flowBoard.emitWithReturn( 'makeContentInteractive', $replacement ); - - $deferred.resolve(); - } ).fail( function( code, result ) { - /* - * At this point, the lock/unlock actually worked, but failed - * fetching the new data to be displayed. - */ - flowBoard.emitWithReturn( 'removeError', $target ); - var errorMsg = flowBoard.constructor.static.getApiErrorMessage( code, result ); - errorMsg = mw.msg( 'flow-error-fetch-after-open-lock', errorMsg ); - flowBoard.emitWithReturn( 'showError', $target, errorMsg ); - - $deferred.reject(); - } ); - - return $deferred.promise(); + return _flowBoardComponentRefreshTopic( + info.$target, + $( this ).closest( '.flow-topic' ).data( 'flow-id' ) + ); }; /** diff --git a/tests/qunit/engine/components/board/test_flow-board.js b/tests/qunit/engine/components/board/test_flow-board.js index fd59445..4dc064e 100644 --- a/tests/qunit/engine/components/board/test_flow-board.js +++ b/tests/qunit/engine/components/board/test_flow-board.js @@ -16,7 +16,6 @@ stub.withArgs( { action: 'flow', submodule: 'view-topic', - workflow: 's18cjkj1bs3rkt13', page: 'Topic:S18cjkj1bs3rkt13' } ).returns( $.Deferred().resolve( { @@ -47,7 +46,6 @@ stub.withArgs( { action: 'flow', submodule: 'view-topic', - workflow: 't18cjkj1bs3rkt13', page: 'Topic:T18cjkj1bs3rkt13' } ).returns( $.Deferred().resolve( { @@ -97,29 +95,37 @@ QUnit.test( 'FlowBoardComponent.UI.events.apiHandlers.lockTopic - perform unlock', 2, function( assert ) { var + $el = this.$el, $topic = $( '<div class="flow-topic" data-flow-id="s18cjkj1bs3rkt13">' ). addClass( 'flow-topic-moderatestate-lock flow-topic-moderated' ). - appendTo( this.$el ), + appendTo( $el ), $titleBar = $( '<div class="flow-topic-titlebar">' ).appendTo( $topic ), - info = { status: 'done', $target: $topic }; + info = { status: 'done', $target: $topic }, + returns; - this.triggerEvent( 'apiHandlers', 'lockTopic', $titleBar, info ); - $topic = this.$el.children( '.flow-topic' ); - assert.strictEqual( $topic.hasClass( 'flow-topic-moderated' ), false, 'No longer has the moderated state.' ); - assert.strictEqual( $topic.hasClass( 'flow-topic-moderatestate-lock' ), false, 'No longer has the moderated lock state.' ); + returns = this.triggerEvent( 'apiHandlers', 'lockTopic', $titleBar, info ); + returns[0].done( function() { + $topic = $el.children( '.flow-topic' ); + assert.strictEqual( $topic.hasClass( 'flow-topic-moderated' ), false, 'No longer has the moderated state.' ); + assert.strictEqual( $topic.hasClass( 'flow-topic-moderatestate-lock' ), false, 'No longer has the moderated lock state.' ); + } ); } ); QUnit.test( 'FlowBoardComponent.UI.events.apiHandlers.lockTopic - perform lock', 2, function( assert ) { var + $el = this.$el, $topic = $( '<div class="flow-topic" data-flow-id="t18cjkj1bs3rkt13">' ). - appendTo( this.$el ), + appendTo( $el ), $titleBar = $( '<div class="flow-topic-titlebar">' ).appendTo( $topic ), - info = { status: 'done', $target: $topic }; + info = { status: 'done', $target: $topic }, + returns; - this.triggerEvent( 'apiHandlers', 'lockTopic', $titleBar, info ); - $topic = this.$el.children( '.flow-topic' ); - assert.strictEqual( $topic.hasClass( 'flow-topic-moderated' ), true, 'Has the moderated state.' ); - assert.strictEqual( $topic.hasClass( 'flow-topic-moderatestate-lock' ), true, 'Has the moderated lock state.' ); + returns = this.triggerEvent( 'apiHandlers', 'lockTopic', $titleBar, info ); + returns[0].done( function() { + $topic = $el.children( '.flow-topic' ); + assert.strictEqual( $topic.hasClass( 'flow-topic-moderated' ), true, 'Has the moderated state.' ); + assert.strictEqual( $topic.hasClass( 'flow-topic-moderatestate-lock' ), true, 'Has the moderated lock state.' ); + } ); } ); QUnit.test( 'FlowBoardComponent.UI.events.apiHandlers.preview', 3, function( assert ) { -- To view, visit https://gerrit.wikimedia.org/r/203822 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I221d2f032c824c309b0ce9f9dcdc83bdbb61587f Gerrit-PatchSet: 4 Gerrit-Project: mediawiki/extensions/Flow Gerrit-Branch: master Gerrit-Owner: Matthias Mullie <mmul...@wikimedia.org> Gerrit-Reviewer: EBernhardson <ebernhard...@wikimedia.org> Gerrit-Reviewer: Matthias Mullie <mmul...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits