jenkins-bot has submitted this change and it was merged. Change subject: Publish module refactoring for title collision ......................................................................
Publish module refactoring for title collision Better module restructuring for publishing error handling and title validations Updated tests accordingly. Bug: T85138, T89585 Change-Id: I95b904d9ab2f7ca882c32e91b2441d7cc5e796ab --- M modules/publish/ext.cx.publish.dialog.js M modules/publish/ext.cx.publish.js M tests/qunit/publish/ext.cx.publish.test.js 3 files changed, 216 insertions(+), 166 deletions(-) Approvals: Nikerabbit: Looks good to me, approved jenkins-bot: Verified diff --git a/modules/publish/ext.cx.publish.dialog.js b/modules/publish/ext.cx.publish.dialog.js index 6860adc..77f098f 100644 --- a/modules/publish/ext.cx.publish.dialog.js +++ b/modules/publish/ext.cx.publish.dialog.js @@ -21,22 +21,18 @@ this.$dialog = null; this.$close = null; this.$message = null; + this.$publishButton = null; + this.$keepButton = null; this.init(); } /** * Initializes the publishing dialog instance. - * @param {string} title The title of the existing article */ CXPublishingDialog.prototype.init = function () { var title = $( '.cx-column--translation > h2' ).text(); - if ( this.$dialog ) { - this.setMessage( title ); - } else { - this.render( title ); - } - this.listen(); + this.render( title ); this.position(); this.show(); }; @@ -46,8 +42,7 @@ * @param {string} title The title of the existing article */ CXPublishingDialog.prototype.render = function ( title ) { - var $buttons, $keepButton, $publishAnywayButton, username, namespace, - cxPublishingDialog = this; + var $buttons, username, namespace; username = mw.user.getName(); namespace = mw.config.get( 'wgContentTranslationTargetNamespace' ); @@ -57,49 +52,33 @@ .hide(); this.$close = $( '<div>' ) - .addClass( 'cx-publishing-dialog__close' ) - .on( 'click', function () { - cxPublishingDialog.$dialog.hide(); - } ); + .addClass( 'cx-publishing-dialog__close' ); this.$message = $( '<p>' ) .addClass( 'cx-publishing-dialog__message' ); - $( '.cx-publishing-dialog__message > a' ).prop( 'target', '_blank' ); + this.$dialog.find( '.cx-publishing-dialog__message > a' ).prop( 'target', '_blank' ); $buttons = $( '<div>' ) .addClass( 'cx-publishing-dialog__buttons' ); - $keepButton = $( '<button>' ) + this.$keepButton = $( '<button>' ) .addClass( 'cx-publishing-dialog__buttons-keep mw-ui-button mw-ui-quiet' ); if ( namespace === 'User' ) { - $keepButton.text( mw.msg( 'cx-publishing-dialog-keep-button' ) ); + this.$keepButton.text( mw.msg( 'cx-publishing-dialog-keep-button' ) ); } else { - $keepButton.text( mw.msg( 'cx-publishing-dialog-publish-draft-button' ) ); + this.$keepButton.text( mw.msg( 'cx-publishing-dialog-publish-draft-button' ) ); } - $keepButton.on( 'click', function () { - cxPublishingDialog.$dialog.hide(); - mw.hook( 'mw.cx.publish' ).fire( false ); - } ); - - $publishAnywayButton = $( '<button>' ) + this.$publishButton = $( '<button>' ) .addClass( 'cx-publishing-dialog__buttons-publishanyway mw-ui-button mw-ui-progressive' ) - .text( mw.msg( 'cx-publishing-dialog-publish-anyway-button' ) ) - .on( 'click', function () { - cxPublishingDialog.$dialog.hide(); - mw.hook( 'mw.cx.publish' ).fire( true ); - } ); + .text( mw.msg( 'cx-publishing-dialog-publish-anyway-button' ) ); this.setMessage( title ); - - $buttons.append( $publishAnywayButton, $keepButton ); - + $buttons.append( this.$publishButton, this.$keepButton ); this.$dialog.append( this.$close, this.$message, $buttons ); - $( 'body' ).append( this.$dialog ); - }; /** @@ -129,10 +108,7 @@ buttonPosition = this.$trigger.position(); buttonCenter = buttonPosition.left + ( this.$trigger.outerWidth() / 2 ); dialogLeft = buttonCenter - ( this.$dialog.outerWidth() / 2 ); - dialogTop = $( window ).scrollTop() + - buttonPosition.top + - this.$trigger.height() + 20; - + dialogTop = $( window ).scrollTop() + buttonPosition.top + this.$trigger.height() + 20; this.$dialog.css( { top: dialogTop, left: dialogLeft, @@ -142,11 +118,31 @@ }; /** - * A listener that adjust the positioning when the page is scrolled - * Necessary because the publishing button moves to the left on window scroll + * Listen for events. This dialog is kind of model dialog. + * An action is expected from user while it is shown. The code + * that invoked this dialog will be waiting for this action. + * Hence the method returns a jQuery Promise + * @return {jQuery.Promise} */ CXPublishingDialog.prototype.listen = function () { + var deferred = $.Deferred(), + self = this; + $( window ).on( 'scroll', $.proxy( this.position, this ) ); + this.$keepButton.on( 'click', function () { + deferred.resolve( false ); + self.$dialog.remove(); + } ); + this.$publishButton.on( 'click', function () { + deferred.resolve( true ); + self.$dialog.remove(); + } ); + this.$close.on( 'click', function () { + deferred.reject(); + self.$dialog.remove(); + } ); + + return deferred.promise(); }; /** @@ -161,16 +157,8 @@ */ $.fn.cxPublishingDialog = function () { return this.each( function () { - /*jshint validthis:true */ - var $this = $( this ), - data = $this.data( 'cxPublishingDialog' ); - - if ( !data ) { - $this.data( 'cxPublishingDialog', ( data = new CXPublishingDialog( this ) ) ); - } - - data.init(); - + var $this = $( this ); + $this.data( 'cxPublishingDialog', new CXPublishingDialog( this ) ); } ); }; } )( jQuery, mediaWiki ); diff --git a/modules/publish/ext.cx.publish.js b/modules/publish/ext.cx.publish.js index c6a6c75..f63657a 100644 --- a/modules/publish/ext.cx.publish.js +++ b/modules/publish/ext.cx.publish.js @@ -16,6 +16,7 @@ */ function CXPublish( $trigger ) { this.$trigger = $trigger; + this.targetTitle = null; } /** @@ -26,28 +27,48 @@ var apiParams, self = this; + this.targetTitle = ( params && params.title ) || $( '.cx-column--translation > h2' ).text(); apiParams = $.extend( {}, params, { - action: 'cxpublish' + action: 'cxpublish', + from: mw.cx.sourceLanguage, + to: mw.cx.targetLanguage, + sourcetitle: mw.cx.sourceTitle, + html: self.getContent(), + status: 'published', + sourcerevision: mw.cx.sourceRevision, + categories: this.getCategories().join( '|' ), + progress: JSON.stringify( mw.cx.getProgress() ) } ); - return new mw.Api().postWithToken( 'edit', apiParams, { - // A bigger timeout since publishing after converting html to wikitext - // parsoid is not a fast operation. - timeout: 100 * 1000 // in milliseconds - } ).then( function ( response ) { - if ( response.cxpublish.result === 'success' ) { - return; - } + // Disable the trigger button + this.$trigger.prop( 'disabled', true ).text( mw.msg( 'cx-publish-button-publishing' ) ); - if ( response.cxpublish.edit.captcha ) { - return self.captchaHandler( response.cxpublish.edit.captcha ) - .then( function ( captchaResult ) { - return self.publish( $.extend( params, captchaResult ) ); - } ); - } + return this.checkTargetTitle( this.targetTitle ).then( function ( title ) { - // If it's a different error, log it - mw.log( '[CX] Unexpected error while publishing: ', response.cxpublish ); + apiParams.title = self.targetTitle = title; + + return new mw.Api().postWithToken( 'edit', apiParams, { + // A bigger timeout since publishing after converting html to wikitext + // parsoid is not a fast operation. + timeout: 100 * 1000 // in milliseconds + } ).done( function ( response ) { + if ( response.cxpublish.result === 'success' ) { + self.onSuccess(); + return; + } + if ( response.cxpublish.edit.captcha ) { + return self.captchaHandler( response.cxpublish.edit.captcha ) + .then( function ( captchaResult ) { + return self.publish( $.extend( params, captchaResult ) ); + } ); + } + // Any other failure + self.fail( 'cxpublish', '[CX] Unexpected error while publishing: ' + response.cxpublish ); + } ).fail( function ( code, details ) { + self.onFail( code, details ); + } ).always( function () { + self.$trigger.prop( 'disabled', true ).text( mw.msg( 'cx-publish-button' ) ); + } ); } ); }; @@ -108,10 +129,11 @@ /** * Checks to see if there is already a published article with the title * @param {string} title The title to check + * @return {jQuery.promise} */ - function checkTargetTitle( title ) { + CXPublish.prototype.titleExists = function ( title ) { var api, - $deferred = $.Deferred(); + deferred = $.Deferred(); api = new mw.Api(); @@ -121,18 +143,64 @@ dataType: 'jsonp' } ).done( function ( response ) { if ( response.query.pages[ -1 ] ) { - $deferred.resolve( false ); + deferred.resolve( false ); } else { - $deferred.resolve( true ); + deferred.resolve( true ); } } ); - return $deferred.promise(); + return deferred.promise(); + }; + + /** + * Generate an alternate title in case of title collision + * @param {string} title The title + * @return {string} + */ + function getAlternateTitle( title ) { + var username, mwTitle; + + username = mw.user.getName(); + mwTitle = mw.Title.newFromText( title ); + if ( mwTitle && mwTitle.getNamespaceId() === 2 ) { + return increaseVersion( title ); + } else { + return 'User:' + username + '/' + title; + } } + + /** + * Checks to see if there is already a published article with the title. + * If exists ask the translator a resolution for the conflict. + * @param {string} title The title to check + * @return {jQuery.Promise} + */ + CXPublish.prototype.checkTargetTitle = function ( title ) { + var self = this; + + title = mw.cx.SiteMapper.prototype.getTargetTitle( title ); + return this.titleExists( title ).then( function ( titleExists ) { + var $dialog; + + if ( !titleExists ) { + return title; + } + // Show a dialog to decide what to do now + self.$trigger.cxPublishingDialog(); + $dialog = self.$trigger.data( 'cxPublishingDialog' ); + return $dialog.listen().then( function ( overwrite ) { + if ( overwrite ) { + return title; + } + return getAlternateTitle( title ); + } ); + } ); + }; /** * Increase the version number of a title starting with 1. * @param {string} title The title to increase the version on. + * @return {string} */ function increaseVersion( title ) { var match, version; @@ -148,110 +216,81 @@ } /** - * Publish the translation - * @param {boolean} overwrite Flag to overwrite translation - * @param {string} title [optional], Optional title for the translation + * Get categories for the current translation pair + * @return {string[]} Category titles */ - function publish( overwrite, title ) { - var $publishArea, $publishButton, publisher, - translatedContent, targetCategories, targetTitle, - sortedKeys, i, categoryTitles, categories, publishedTitle; + CXPublish.prototype.getCategories = function () { + var i, sortedKeys, categoryTitles, targetCategories; - $publishArea = $( '.cx-header__publish' ); - $publishButton = $publishArea.find( '.cx-header__publish-button' ); - targetTitle = title || $( '.cx-column--translation > h2' ).text(); - translatedContent = prepareTranslationForPublish( + targetCategories = mw.cx.categoryTool.categories.target; + if ( !targetCategories ) { + return []; + } + sortedKeys = Object.keys( targetCategories ).sort(); + categoryTitles = []; + for ( i = 0; i < sortedKeys.length; i++ ) { + categoryTitles.push( targetCategories[ sortedKeys[ i ] ] ); + } + return categoryTitles; + }; + + /** + * Get the current translation content + * @return {string} + */ + CXPublish.prototype.getContent = function () { + return this.prepareTranslationForPublish( $( '.cx-column--translation .cx-column__content' ).clone() ); + }; - publishedTitle = mw.cx.SiteMapper.prototype.getTargetTitle( targetTitle ); + /** + * Success handler for publishing + */ + CXPublish.prototype.onSuccess = function () { + $( '.cx-column--translation > h2' ) + .text( this.targetTitle ) + .keepAlignment(); + mw.hook( 'mw.cx.success' ) + .fire( mw.message( 'cx-publish-page-success', + $( '<a>' ).attr( { + href: mw.util.getUrl( this.targetTitle ), + target: '_blank' + } ).text( this.targetTitle )[ 0 ].outerHTML + ) ); + mw.hook( 'mw.cx.translation.published' ).fire( + mw.cx.sourceLanguage, + mw.cx.targetLanguage, + this.targetTitle + ); + mw.cx.dirty = false; + }; - checkTargetTitle( publishedTitle ) - .done( function ( titleExists ) { - var username; - - username = mw.user.getName(); - - if ( titleExists === false || overwrite === true ) { - $publishButton - .prop( 'disabled', true ) - .text( mw.msg( 'cx-publish-button-publishing' ) ); - - targetCategories = mw.cx.categoryTool.categories.target; - sortedKeys = Object.keys( targetCategories ).sort(); - categoryTitles = []; - for ( i = 0; i < sortedKeys.length; i++ ) { - categoryTitles.push( targetCategories[ sortedKeys[ i ] ] ); - } - categories = categoryTitles.join( '|' ); - - publisher = new CXPublish( $publishArea ); - publisher.publish( { - from: mw.cx.sourceLanguage, - to: mw.cx.targetLanguage, - sourcetitle: mw.cx.sourceTitle, - title: targetTitle, - html: translatedContent, - status: 'published', - sourcerevision: mw.cx.sourceRevision, - categories: categories, - progress: JSON.stringify( mw.cx.getProgress() ) - } ).done( function () { - $( '.cx-column--translation > h2' ) - .text( publishedTitle ) - .keepAlignment(); - mw.hook( 'mw.cx.success' ) - .fire( mw.message( 'cx-publish-page-success', - $( '<a>' ).attr( { - href: mw.util.getUrl( publishedTitle ), - target: '_blank' - } ).text( publishedTitle )[ 0 ].outerHTML - ) ); - mw.hook( 'mw.cx.translation.published' ).fire( - mw.cx.sourceLanguage, - mw.cx.targetLanguage, - targetTitle - ); - mw.cx.dirty = false; - } ).fail( function ( code, details ) { - var trace = { - sourceLanguage: mw.cx.sourceLanguage, - targetLanguage: mw.cx.targetLanguage, - sourceTitle: mw.cx.sourceTitle, - sourceRevision: mw.cx.sourceRevision, - targetTitle: targetTitle, - error: details - }; - mw.hook( 'mw.cx.error' ).fire( mw.msg( 'cx-publish-page-error' ) ); - mw.log( '[CX] Error while publishing:', code, trace ); - } ).always( function () { - $publishButton - .prop( 'disabled', true ) - .text( mw.msg( 'cx-publish-button' ) ); - } ); - } else if ( overwrite === false ) { - if ( new mw.Title( publishedTitle ).getNamespaceId() === 2 ) { - publishedTitle = increaseVersion( publishedTitle ); - } else { - publishedTitle = 'User:' + username + '/' + publishedTitle; - } - publish( false, publishedTitle ); - } else { - $publishButton.cxPublishingDialog(); - } - } ); - } + /** + * Failure handler for publishing + */ + CXPublish.prototype.onFail = function ( code, details ) { + var trace = { + sourceLanguage: mw.cx.sourceLanguage, + targetLanguage: mw.cx.targetLanguage, + sourceTitle: mw.cx.sourceTitle, + sourceRevision: mw.cx.sourceRevision, + targetTitle: this.targetTitle, + error: details + }; + mw.hook( 'mw.cx.error' ).fire( mw.msg( 'cx-publish-page-error' ) ); + mw.log( '[CX] Error while publishing:', code, trace ); + }; /** * Prepare the translated content for publishing by removing * unwanted parts. * @return {string} processed html */ - function prepareTranslationForPublish( $content ) { + CXPublish.prototype.prepareTranslationForPublish = function ( $content ) { $content.find( '.cx-segment' ).replaceWith( function () { return $( this ).html(); } ); - // TODO: This clean up should be done even before segmentation at server. $content.find( 'link, title' ).remove(); // Remove placeholder sections @@ -266,12 +305,17 @@ } ); return $content.html(); - } + }; // Expose the CXPublish mw.cx.publish = CXPublish; $( function () { - mw.hook( 'mw.cx.publish' ).add( $.proxy( publish, this ) ); + var cxPublish, $publishButton; + + $publishButton = $( '.cx-header__publish .cx-header__publish-button' ); + cxPublish = new mw.cx.publish( $publishButton ); + + mw.hook( 'mw.cx.publish' ).add( $.proxy( cxPublish.publish, cxPublish ) ); } ); }( jQuery, mediaWiki ) ); diff --git a/tests/qunit/publish/ext.cx.publish.test.js b/tests/qunit/publish/ext.cx.publish.test.js index 9e41487..2f31950 100644 --- a/tests/qunit/publish/ext.cx.publish.test.js +++ b/tests/qunit/publish/ext.cx.publish.test.js @@ -10,17 +10,23 @@ QUnit.module( 'ext.cx.publish', QUnit.newMwEnvironment( { setup: function () { this.server = this.sandbox.useFakeServer(); + this.sitemapper = new mw.cx.SiteMapper( + mw.config.get( 'wgContentTranslationSiteTemplates' ) + ); + mw.cx.categoryTool = new mw.cx.CategoryTool( this.sitemapper ); } } ) ); - QUnit.test( 'Captcha handling', function ( assert ) { + QUnit.test( 'Publishing with Captcha handling', function ( assert ) { var publisher, oldCaptchaHandler = mw.cx.publish.prototype.captchaHandler, + oldSuccessHandler = mw.cx.publish.prototype.onSuccess, + oldTitleExists = mw.cx.publish.prototype.titleExists, newCaptchaHandler, server = this.server, $trigger = $( '<div>' ); - QUnit.expect( 3 ); + QUnit.expect( 4 ); newCaptchaHandler = function ( captcha ) { var deferred = $.Deferred(); @@ -33,6 +39,12 @@ }; publisher = new mw.cx.publish( $trigger ); publisher.captchaHandler = newCaptchaHandler; + publisher.titleExists = function () { + return $.Deferred().resolve( false ); + }; + publisher.onSuccess = function () { + assert.ok( true, 'Success handler was called' ); + }; publisher.publish( { from: 'fi', to: 'en', @@ -42,11 +54,17 @@ } ).done( function () { assert.ok( true, 'Publishing was completed' ); mw.cx.publish.prototype.captchaHandler = oldCaptchaHandler; + mw.cx.publish.prototype.onSuccess = oldSuccessHandler; + mw.cx.publish.prototype.getTitle = oldTitleExists; } ); - server.requests[ 0 ].respond( 200, { 'Content-Type': 'application/json' }, + server.requests[ 0 ].respond( 200, { + 'Content-Type': 'application/json' + }, '{ "cxpublish": { "result": "error", "edit": { "captcha": {"captchaKey":"1234565"} } } }' ); - server.requests[ 1 ].respond( 200, { 'Content-Type': 'application/json' }, + server.requests[ 1 ].respond( 200, { + 'Content-Type': 'application/json' + }, '{ "cxpublish": { "result": "success" } }' ); } ); -- To view, visit https://gerrit.wikimedia.org/r/192786 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I95b904d9ab2f7ca882c32e91b2441d7cc5e796ab Gerrit-PatchSet: 4 Gerrit-Project: mediawiki/extensions/ContentTranslation Gerrit-Branch: master Gerrit-Owner: Santhosh <santhosh.thottin...@gmail.com> Gerrit-Reviewer: Jsahleen <jsahl...@wikimedia.org> Gerrit-Reviewer: Nikerabbit <niklas.laxst...@gmail.com> Gerrit-Reviewer: Santhosh <santhosh.thottin...@gmail.com> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits