From 098655d913f2faa8c26d32af8636cd24d6ae5b05 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Tue, 26 Apr 2016 16:27:49 -0400 Subject: [PATCH] Prompt about username change at sync time, not just in prefs This is necessary because you can copy a database synced with a different account into the data directory without affecting the stored pref. Also tweak the text to use proper quotes and remove quaint references to "the server". --- .../zotero/preferences/preferences_sync.js | 89 +------------------ chrome/content/zotero/xpcom/sync/syncLocal.js | 87 ++++++++++++++++++ .../content/zotero/xpcom/sync/syncRunner.js | 14 +-- chrome/locale/en-US/zotero/zotero.properties | 6 +- test/tests/preferences_syncTest.js | 57 ------------ test/tests/syncLocalTest.js | 57 ++++++++++++ 6 files changed, 157 insertions(+), 153 deletions(-) diff --git a/chrome/content/zotero/preferences/preferences_sync.js b/chrome/content/zotero/preferences/preferences_sync.js index ba0da06e0..4e51bb031 100644 --- a/chrome/content/zotero/preferences/preferences_sync.js +++ b/chrome/content/zotero/preferences/preferences_sync.js @@ -140,7 +140,7 @@ Zotero_Preferences.Sync = { return; } - if (!(yield this.checkUser(json.userID, json.username))) { + if (!(yield Zotero.Sync.Data.Local.checkUser(window, json.userID, json.username))) { // createAPIKeyFromCredentials will have created an API key, // but user decided not to use it, so we remove it here. Zotero.Sync.Runner.deleteAPIKey(); @@ -179,93 +179,6 @@ Zotero_Preferences.Sync = { }), - /** - * Make sure we're syncing with the same account we used last time, and prompt if not. - * If user accepts, change the current user, delete existing groups, and update relation - * URIs to point to the new user's library. - * - * @param {Integer} userID New userID - * @param {Integer} libraryID New libraryID - * @return {Boolean} - True to continue, false to cancel - */ - checkUser: Zotero.Promise.coroutine(function* (userID, username) { - var lastUserID = Zotero.Users.getCurrentUserID(); - var lastUsername = Zotero.Users.getCurrentUsername(); - - if (lastUserID && lastUserID != userID) { - var groups = Zotero.Groups.getAll(); - - var ps = Components.classes["@mozilla.org/embedcomp/prompt-service;1"] - .getService(Components.interfaces.nsIPromptService); - var buttonFlags = (ps.BUTTON_POS_0) * (ps.BUTTON_TITLE_IS_STRING) - + (ps.BUTTON_POS_1) * (ps.BUTTON_TITLE_CANCEL) - + (ps.BUTTON_POS_2) * (ps.BUTTON_TITLE_IS_STRING) - + ps.BUTTON_POS_1_DEFAULT - + ps.BUTTON_DELAY_ENABLE; - - var msg = Zotero.getString('sync.lastSyncWithDifferentAccount', [lastUsername, username]); - var syncButtonText = Zotero.getString('sync.sync'); - - msg += " " + Zotero.getString('sync.localDataWillBeCombined', username); - // If there are local groups belonging to the previous user, - // we need to remove them - if (groups.length) { - msg += " " + Zotero.getString('sync.localGroupsWillBeRemoved1'); - var syncButtonText = Zotero.getString('sync.removeGroupsAndSync'); - } - msg += "\n\n" + Zotero.getString('sync.avoidCombiningData', lastUsername); - - var index = ps.confirmEx( - null, - Zotero.getString('general.warning'), - msg, - buttonFlags, - syncButtonText, - null, - Zotero.getString('sync.openSyncPreferences'), - null, {} - ); - - if (index > 0) { - if (index == 2) { - var wm = Components.classes["@mozilla.org/appshell/window-mediator;1"] - .getService(Components.interfaces.nsIWindowMediator); - var lastWin = wm.getMostRecentWindow("navigator:browser"); - lastWin.ZoteroPane.openPreferences('zotero-prefpane-sync'); - } - return false; - } - } - - yield Zotero.DB.executeTransaction(function* () { - if (lastUserID != userID) { - if (lastUserID) { - // Delete all local groups if changing users - for (let group of groups) { - yield group.erase(); - } - - // Update relations pointing to the old library to point to this one - yield Zotero.Relations.updateUser(userID); - } - // Replace local user key with libraryID, in case duplicates were - // merged before the first sync - else { - yield Zotero.Relations.updateUser(userID); - } - - yield Zotero.Users.setCurrentUserID(userID); - } - - if (lastUsername != username) { - yield Zotero.Users.setCurrentUsername(username); - } - }) - - return true; - }), - - updateStorageSettingsUI: Zotero.Promise.coroutine(function* () { this.unverifyStorageServer(); diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js index b81666c5c..d207cb370 100644 --- a/chrome/content/zotero/xpcom/sync/syncLocal.js +++ b/chrome/content/zotero/xpcom/sync/syncLocal.js @@ -86,6 +86,93 @@ Zotero.Sync.Data.Local = { }, + /** + * Make sure we're syncing with the same account we used last time, and prompt if not. + * If user accepts, change the current user, delete existing groups, and update relation + * URIs to point to the new user's library. + * + * @param {Window} + * @param {Integer} userID - New userID + * @param {Integer} libraryID - New libraryID + * @return {Boolean} - True to continue, false to cancel + */ + checkUser: Zotero.Promise.coroutine(function* (win, userID, username) { + var lastUserID = Zotero.Users.getCurrentUserID(); + var lastUsername = Zotero.Users.getCurrentUsername(); + + if (lastUserID && lastUserID != userID) { + var groups = Zotero.Groups.getAll(); + + var ps = Components.classes["@mozilla.org/embedcomp/prompt-service;1"] + .getService(Components.interfaces.nsIPromptService); + var buttonFlags = (ps.BUTTON_POS_0) * (ps.BUTTON_TITLE_IS_STRING) + + (ps.BUTTON_POS_1) * (ps.BUTTON_TITLE_CANCEL) + + (ps.BUTTON_POS_2) * (ps.BUTTON_TITLE_IS_STRING) + + ps.BUTTON_POS_1_DEFAULT + + ps.BUTTON_DELAY_ENABLE; + + var msg = Zotero.getString( + 'sync.lastSyncWithDifferentAccount', [ZOTERO_CONFIG.CLIENT_NAME, lastUsername, username] + ); + var syncButtonText = Zotero.getString('sync.sync'); + + msg += " " + Zotero.getString('sync.localDataWillBeCombined', [username, ZOTERO_CONFIG.DOMAIN_NAME]); + // If there are local groups belonging to the previous user, + // we need to remove them + if (groups.length) { + msg += " " + Zotero.getString('sync.localGroupsWillBeRemoved1'); + var syncButtonText = Zotero.getString('sync.removeGroupsAndSync'); + } + msg += "\n\n" + Zotero.getString('sync.avoidCombiningData', lastUsername); + + var index = ps.confirmEx( + win, + Zotero.getString('general.warning'), + msg, + buttonFlags, + syncButtonText, + null, + Zotero.getString('sync.openSyncPreferences'), + null, {} + ); + + if (index > 0) { + if (index == 2) { + win.ZoteroPane.openPreferences('zotero-prefpane-sync'); + } + return false; + } + } + + yield Zotero.DB.executeTransaction(function* () { + if (lastUserID != userID) { + if (lastUserID) { + // Delete all local groups if changing users + for (let group of groups) { + yield group.erase(); + } + + // Update relations pointing to the old library to point to this one + yield Zotero.Relations.updateUser(userID); + } + // Replace local user key with libraryID, in case duplicates were + // merged before the first sync + else { + yield Zotero.Relations.updateUser(userID); + } + + yield Zotero.Users.setCurrentUserID(userID); + } + + if (lastUsername != username) { + yield Zotero.Users.setCurrentUsername(username); + } + }) + + return true; + }), + + /** * @return {nsILoginInfo|false} */ diff --git a/chrome/content/zotero/xpcom/sync/syncRunner.js b/chrome/content/zotero/xpcom/sync/syncRunner.js index ef44be6f8..bc7242bfa 100644 --- a/chrome/content/zotero/xpcom/sync/syncRunner.js +++ b/chrome/content/zotero/xpcom/sync/syncRunner.js @@ -140,12 +140,16 @@ Zotero.Sync.Runner_Module = function (options = {}) { Zotero.debug("Syncing cancelled because user library is empty"); return false; } - - if (!Zotero.Users.getCurrentUserID()) { - Zotero.Users.setCurrentUserID(keyInfo.userID); - Zotero.Users.setCurrentUsername(keyInfo.username); + + let wm = Components.classes["@mozilla.org/appshell/window-mediator;1"] + .getService(Components.interfaces.nsIWindowMediator); + let lastWin = wm.getMostRecentWindow("navigator:browser"); + if (!(yield Zotero.Sync.Data.Local.checkUser(lastWin, keyInfo.userID, keyInfo.username))) { + yield this.end(options); + Zotero.debug("User cancelled sync on username mismatch"); + return false; } - + let engineOptions = { apiClient: client, caller: this.caller, diff --git a/chrome/locale/en-US/zotero/zotero.properties b/chrome/locale/en-US/zotero/zotero.properties index e2a9ca967..a23a16b2f 100644 --- a/chrome/locale/en-US/zotero/zotero.properties +++ b/chrome/locale/en-US/zotero/zotero.properties @@ -841,10 +841,10 @@ sync.error.emptyResponseServer = Empty response from server. sync.error.invalidCharsFilename = The filename '%S' contains invalid characters.\n\nRename the file and try again. If you rename the file via the OS, you will need to relink it in Zotero. sync.error.apiKeyInvalid = %S could not authenticate your account. Please re-enter your account details. -sync.lastSyncWithDifferentAccount = This Zotero database was last synced with a different zotero.org account ('%1$S') from the current one ('%2$S'). -sync.localDataWillBeCombined = If you continue, local Zotero data will be combined with data from the '%S' account stored on the server. +sync.lastSyncWithDifferentAccount = This Zotero database was last synced with a different %1$S account (‘%2$S’) from the current one (‘%3$S’). +sync.localDataWillBeCombined = If you continue, local data will be combined with data from the ‘%1$S’ account on %2$S. sync.localGroupsWillBeRemoved1 = Local groups, including any with changed items, will also be removed from this computer. -sync.avoidCombiningData = To avoid combining data, revert to the '%S' account or use the Reset options in the Sync pane of the Zotero preferences. +sync.avoidCombiningData = To avoid combining data, revert to the ‘%S’ account or use the Reset options in the Sync pane of the Zotero preferences. sync.unlinkWarning = Are you sure you want to unlink this account?\n\n%S will no longer sync your data, but your data will remain locally. sync.warning.emptyLibrary = You are about to sync the ‘%1$S’ account to an empty %2$S database. This could happen if you removed your previous %2$S database or if the location of your data directory changed. sync.warning.existingDataElsewhere = If your %S data exists elsewhere on your computer, you should move it to your current data directory or change your data directory location to point to the existing data. diff --git a/test/tests/preferences_syncTest.js b/test/tests/preferences_syncTest.js index 9175f6dd1..c5e30e66a 100644 --- a/test/tests/preferences_syncTest.js +++ b/test/tests/preferences_syncTest.js @@ -98,62 +98,5 @@ describe("Sync Preferences", function () { }) }) - - describe("#checkUser()", function () { - it("should prompt for user update and perform on accept", function* () { - yield Zotero.Users.setCurrentUserID(1); - yield Zotero.Users.setCurrentUsername("A"); - - waitForDialog(function (dialog) { - var text = dialog.document.documentElement.textContent; - var matches = text.match(/'[^']*'/g); - assert.equal(matches.length, 4); - assert.equal(matches[0], "'A'"); - assert.equal(matches[1], "'B'"); - assert.equal(matches[2], "'B'"); - assert.equal(matches[3], "'A'"); - }); - var cont = yield win.Zotero_Preferences.Sync.checkUser(2, "B"); - assert.isTrue(cont); - - assert.equal(Zotero.Users.getCurrentUserID(), 2); - assert.equal(Zotero.Users.getCurrentUsername(), "B"); - }) - - it("should prompt for user update and cancel", function* () { - yield Zotero.Users.setCurrentUserID(1); - yield Zotero.Users.setCurrentUsername("A"); - - waitForDialog(false, 'cancel'); - var cont = yield win.Zotero_Preferences.Sync.checkUser(2, "B"); - assert.isFalse(cont); - - assert.equal(Zotero.Users.getCurrentUserID(), 1); - assert.equal(Zotero.Users.getCurrentUsername(), "A"); - }) - - it("should update local relations when syncing for the first time", function* () { - yield resetDB({ - thisArg: this, - skipBundledFiles: true - }); - - var item1 = yield createDataObject('item'); - var item2 = yield createDataObject( - 'item', { libraryID: Zotero.Libraries.publicationsLibraryID } - ); - - yield item1.addLinkedItem(item2); - - var cont = yield win.Zotero_Preferences.Sync.checkUser(1, "A"); - assert.isTrue(cont); - - var json = item1.toJSON(); - var uri = json.relations[Zotero.Relations.linkedObjectPredicate][0]; - assert.notInclude(uri, 'users/local'); - assert.include(uri, 'users/1/publications'); - }) - }) - }) diff --git a/test/tests/syncLocalTest.js b/test/tests/syncLocalTest.js index d7fbe2124..25f7ba4dc 100644 --- a/test/tests/syncLocalTest.js +++ b/test/tests/syncLocalTest.js @@ -21,6 +21,63 @@ describe("Zotero.Sync.Data.Local", function() { }) + describe("#checkUser()", function () { + it("should prompt for user update and perform on accept", function* () { + yield Zotero.Users.setCurrentUserID(1); + yield Zotero.Users.setCurrentUsername("A"); + + waitForDialog(function (dialog) { + var text = dialog.document.documentElement.textContent; + var matches = text.match(/'[^']*'/g); + assert.equal(matches.length, 4); + assert.equal(matches[0], "'A'"); + assert.equal(matches[1], "'B'"); + assert.equal(matches[2], "'B'"); + assert.equal(matches[3], "'A'"); + }); + var cont = yield Zotero.Sync.Data.Local.checkUser(win, 2, "B"); + assert.isTrue(cont); + + assert.equal(Zotero.Users.getCurrentUserID(), 2); + assert.equal(Zotero.Users.getCurrentUsername(), "B"); + }) + + it("should prompt for user update and cancel", function* () { + yield Zotero.Users.setCurrentUserID(1); + yield Zotero.Users.setCurrentUsername("A"); + + waitForDialog(false, 'cancel'); + var cont = yield Zotero.Sync.Data.Local.checkUser(win, 2, "B"); + assert.isFalse(cont); + + assert.equal(Zotero.Users.getCurrentUserID(), 1); + assert.equal(Zotero.Users.getCurrentUsername(), "A"); + }) + + it("should update local relations when syncing for the first time", function* () { + yield resetDB({ + thisArg: this, + skipBundledFiles: true + }); + + var item1 = yield createDataObject('item'); + var item2 = yield createDataObject( + 'item', { libraryID: Zotero.Libraries.publicationsLibraryID } + ); + + yield item1.addLinkedItem(item2); + + var cont = yield Zotero.Sync.Data.Local.checkUser(win, 1, "A"); + assert.isTrue(cont); + + var json = item1.toJSON(); + var uri = json.relations[Zotero.Relations.linkedObjectPredicate][0]; + assert.notInclude(uri, 'users/local'); + assert.include(uri, 'users/1/publications'); + }) + }); + + describe("#getLatestCacheObjectVersions", function () { before(function* () { yield resetDB({