jenkins-bot has submitted this change and it was merged.

Change subject: Do not rely on draft param to restore a draft
......................................................................


Do not rely on draft param to restore a draft

In case of reloads at translation view, there is no draft Id in the URL
and it will result presenting a fresh translation view. Even though,
CX had saved all translations in database. This can be interpreted
by users as 'data loss'. The page refresh might be accidental too.

With this patch, we dont rely on draft id in URL to restore the existing
translation. But we will query CX to see if there is a translation for
given source title and language pair. If so fetch the translation content
and restore.

References to draft id is removed, instead, the translation id found while
restoring is set to mw.cx.translationId. We need this for section level saving
of translation very soon.

Also cleaned up the Deferred usage to make this module more testable.

Testcase:
Start a fresh translation in CX. Translate a few paragraphs. Make sure
the content is saved by autosave or just press Control+S. Then reload
the whole page. The previously saved translations should get restored.

Bug: T120635
Change-Id: I157b0204425cfe18805ae0ecf86309a9df866e13
---
M modules/translation/ext.cx.translation.draft.js
M tests/qunit/translation/ext.cx.translation.draft.test.js
2 files changed, 38 insertions(+), 43 deletions(-)

Approvals:
  Nikerabbit: Checked; Looks good to me, approved
  jenkins-bot: Verified



diff --git a/modules/translation/ext.cx.translation.draft.js 
b/modules/translation/ext.cx.translation.draft.js
index edfecdf..f928170 100644
--- a/modules/translation/ext.cx.translation.draft.js
+++ b/modules/translation/ext.cx.translation.draft.js
@@ -13,26 +13,41 @@
 
        /**
         * @class
-        * @param {number} draftId Draft id
         */
-       function ContentTranslationDraft( draftId ) {
-               this.draftId = draftId;
+       function ContentTranslationDraft() {
                this.$draft = null;
                this.disabled = false;
                this.$source = null;
                this.$translation = null;
-               this.init();
                this.listen();
        }
 
+       /**
+        * Initalize the draft storage.
+        *
+        * @return {jQuery.Promise}
+        */
        ContentTranslationDraft.prototype.init = function () {
                var self = this;
-
-               this.hasConflictingTranslation().done( function ( translation ) 
{
-                       if ( translation !== null ) {
+               // There is no known consumer for this return value. Just 
returning it
+               // to help testing in future.
+               return this.find().then( function ( translation ) {
+                       if ( !translation ) {
+                               return false;
+                       }
+                       // If this translation is draft and not by current 
user, there is an
+                       // existing translation.
+                       if ( translation.translatorName !== mw.user.getName() &&
+                               translation.status === 'draft'
+                       ) {
                                self.showConflictWarning( translation );
                                self.disabled = true;
+                               return false;
                        }
+                       // Set the translationId
+                       mw.cx.translationId = translation.id;
+                       // Fetch the translation content
+                       return self.fetch();
                } );
        };
 
@@ -101,53 +116,37 @@
         * Find if there is a draft existing for the current title and language 
pair.
         * @return {jQuery.Promise}
         */
-       ContentTranslationDraft.prototype.hasConflictingTranslation = function 
() {
-               var deferred = $.Deferred(),
-                       api = new mw.Api();
+       ContentTranslationDraft.prototype.find = function () {
+               var api = new mw.Api();
 
-               api.get( {
+               return api.get( {
                        action: 'query',
                        list: 'contenttranslation',
                        sourcetitle: mw.cx.sourceTitle,
                        from: mw.cx.sourceLanguage,
                        to: mw.cx.targetLanguage,
                        format: 'json'
-               } ).done( function ( response ) {
-                       var translation;
-
-                       translation = response.query && 
response.query.contenttranslation.translation;
-                       // If this translation is draft and not by current 
user, there is an
-                       // existing translation.
-                       if ( translation &&
-                               translation.translatorName !== 
mw.user.getName() &&
-                               translation.status === 'draft'
-                       ) {
-                               deferred.resolve( translation );
-                       } else {
-                               deferred.resolve( null );
-                       }
+               } ).then( function ( response ) {
+                       return response.query && 
response.query.contenttranslation.translation;
                } );
-
-               return deferred.promise();
        };
 
        /**
         * Fetch a draft content and restore it.
+        * @return {jQuery.Promise}
         */
        ContentTranslationDraft.prototype.fetch = function () {
                var self = this,
                        api = new mw.Api();
 
                mw.hook( 'mw.cx.draft.restoring' ).fire();
-               // TODO: The fetch can start immediately when the module loaded
-               // Only the restoring part need to delay till placeholders are 
rendered.
-               // Now there is a visible delay between placeholder rendering 
and restoring draft.
-               api.get( {
+
+               return api.get( {
                        action: 'query',
                        list: 'contenttranslation',
-                       translationid: this.draftId,
+                       translationid: mw.cx.translationId,
                        format: 'json'
-               } ).done( function ( response ) {
+               } ).then( function ( response ) {
                        var translation, draftContent;
 
                        translation = 
response.query.contenttranslation.translation;
@@ -158,7 +157,7 @@
                                self.restore();
                                mw.hook( 'mw.cx.draft.restored' ).fire();
                        } );
-               } ).fail( function ( errorCode, details ) {
+               }, function ( errorCode, details ) {
                        var uri = new mw.Uri();
 
                        // Wrong draft id passed.
@@ -396,17 +395,16 @@
 
        mw.cx.ContentTranslationDraft = ContentTranslationDraft;
        $( function () {
-               var draftId, draft;
+               var draft,
+                       query = new mw.Uri().query;
 
                if ( mw.config.get( 'wgContentTranslationDatabase' ) === null ) 
{
                        mw.log( 'The ext.cx.translation.draft module can only 
work if CX Database configured.' );
                        return;
                }
-
-               draftId = new mw.Uri().query.draft;
-               draft = new ContentTranslationDraft( draftId );
-               if ( draftId ) {
-                       draft.fetch();
+               draft = new ContentTranslationDraft();
+               if ( query.to && query.from && query.page ) {
+                       draft.init();
                }
        } );
 }( jQuery, mediaWiki ) );
diff --git a/tests/qunit/translation/ext.cx.translation.draft.test.js 
b/tests/qunit/translation/ext.cx.translation.draft.test.js
index ce5c028..c5e8215 100644
--- a/tests/qunit/translation/ext.cx.translation.draft.test.js
+++ b/tests/qunit/translation/ext.cx.translation.draft.test.js
@@ -60,9 +60,6 @@
 
        QUnit.module( 'ext.cx.translation.draft', QUnit.newMwEnvironment( {
                setup: function () {
-                       
mw.cx.ContentTranslationDraft.prototype.hasConflictingTranslation = function () 
{
-                               return $.Deferred().resolve( false );
-                       };
                        this.cxDraft = new mw.cx.ContentTranslationDraft();
                }
        } ) );

-- 
To view, visit https://gerrit.wikimedia.org/r/257298
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I157b0204425cfe18805ae0ecf86309a9df866e13
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/ContentTranslation
Gerrit-Branch: master
Gerrit-Owner: Santhosh <[email protected]>
Gerrit-Reviewer: KartikMistry <[email protected]>
Gerrit-Reviewer: Nikerabbit <[email protected]>
Gerrit-Reviewer: Santhosh <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to