www

Unnamed repository; edit this file 'description' to name the repository.
Log | Files | Refs | Submodules | README | LICENSE

commit d44eeb752b656b0f61abf1ace4316a7f90b7dfd4
parent 72bad4309b77a02ac642c4643f0981565a5b9aa6
Author: Dan Stillman <dstillman@zotero.org>
Date:   Thu,  7 Jul 2016 05:01:36 -0400

Fix error on CR of child note, and show parent item title in merge pane

Diffstat:
Mchrome/content/zotero/bindings/merge.xml | 30+++++++++++++++++++++++++++++-
Mchrome/content/zotero/merge.js | 4++++
Mchrome/content/zotero/xpcom/sync/syncEngine.js | 1+
Mchrome/content/zotero/xpcom/sync/syncLocal.js | 2++
Mchrome/skin/default/zotero/merge.css | 8++++++++
Mtest/tests/syncEngineTest.js | 160+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Mtest/tests/syncLocalTest.js | 41+++++++++++++++++++++++++++++++++++++++++
7 files changed, 245 insertions(+), 1 deletion(-)

diff --git a/chrome/content/zotero/bindings/merge.xml b/chrome/content/zotero/bindings/merge.xml @@ -28,7 +28,8 @@ <bindings xmlns="http://www.mozilla.org/xbl" xmlns:xbl="http://www.mozilla.org/xbl" - xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"> + xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" + xmlns:html="http://www.w3.org/1999/xhtml"> <binding id="merge-group"> <resources> @@ -44,6 +45,8 @@ ]]> </constructor> + <field name="libraryID"/> + <field name="_data"/> <property name="data" onget="return this._data;"> <setter> @@ -109,6 +112,9 @@ this._leftpane.showButton = showButton; this._rightpane.showButton = showButton; + this._leftpane.libraryID = this.libraryID; + this._rightpane.libraryID = this.libraryID; + this._mergepane.libraryID = this.libraryID; this._leftpane.data = this._data.left; this._rightpane.data = this._data.right; this._mergepane.data = this._data.merge; @@ -293,6 +299,8 @@ </setter> </property> + <field name="libraryID"/> + <field name="_data"/> <property name="data" onget="return this._data"> <setter> @@ -338,6 +346,24 @@ var objbox = document.createElement(elementName); + var parentRow = this._id('parent-row'); + if (val.parentItem) { + parentRow.textContent = ''; + + let label = document.createElement('span'); + label.textContent = Zotero.getString('pane.item.parentItem'); + parentRow.appendChild(label); + + let parentItem = Zotero.Items.getByLibraryAndKey(this.libraryID, val.parentItem); + let text = document.createTextNode(" " + parentItem.getDisplayTitle(true)); + parentRow.appendChild(text); + + parentRow.hidden = false; + } + else { + parentRow.hidden = true; + } + if (this._id('object-placeholder')) { var placeholder = this._id('object-placeholder'); placeholder.parentNode.replaceChild(objbox, placeholder); @@ -356,6 +382,7 @@ // Create item from JSON for metadata box var item = new Zotero.Item(val.itemType); + item.libraryID = this.libraryID; item.fromJSON(val); objbox.item = item; ]]> @@ -385,6 +412,7 @@ <xul:vbox flex="1"> <xul:groupbox anonid="merge-pane" flex="1"> <xul:caption anonid="caption"/> + <html:div anonid="parent-row" hidden="true"/> <xul:box anonid="object-placeholder"/> <xul:hbox anonid="delete-box" hidden="true" flex="1"> <xul:label value="&zotero.merge.deleted;"/> diff --git a/chrome/content/zotero/merge.js b/chrome/content/zotero/merge.js @@ -200,6 +200,10 @@ var Zotero_Merge_Window = new function () { var mergeInfo = _getMergeInfo(_pos); data.merge = mergeInfo.data; data.selected = mergeInfo.selected; + if (!_conflicts[_pos].libraryID) { + throw new Error("libraryID not provided in conflict object"); + } + _mergeGroup.libraryID = _conflicts[_pos].libraryID; _mergeGroup.data = data; _updateResolveAllCheckbox(); diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js @@ -643,6 +643,7 @@ Zotero.Sync.Data.Engine.prototype._downloadDeletions = Zotero.Promise.coroutine( // Conflict resolution else if (objectType == 'item') { conflicts.push({ + libraryID: this.libraryID, left: obj.toJSON(), right: { deleted: true diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js @@ -677,6 +677,7 @@ Zotero.Sync.Data.Local = { Zotero.debug(jsonData); Zotero.debug(result); results.push({ + libraryID, key: objectKey, processed: false, conflict: true, @@ -721,6 +722,7 @@ Zotero.Sync.Data.Local = { switch (objectType) { case 'item': results.push({ + libraryID, key: objectKey, processed: false, conflict: true, diff --git a/chrome/skin/default/zotero/merge.css b/chrome/skin/default/zotero/merge.css @@ -88,3 +88,11 @@ hbox:not([mergetype=note]) zoteromergepane:active[id=rightpane] groupbox caption zoteromergepane { min-width: 28em; } + +zoteromergepane div[anonid=parent-row] { + margin-bottom: 5px; +} + +zoteromergepane div[anonid=parent-row] span { + font-weight: bold; +} diff --git a/test/tests/syncEngineTest.js b/test/tests/syncEngineTest.js @@ -2025,6 +2025,103 @@ describe("Zotero.Sync.Data.Engine", function () { assert.lengthOf(keys, 0); }); + it("should show conflict resolution window on note conflicts", function* () { + var libraryID = Zotero.Libraries.userLibraryID; + ({ engine, client, caller } = yield setup()); + var type = 'item'; + var objects = []; + var values = []; + var dateAdded = Date.now() - 86400000; + var responseJSON = []; + + for (let i = 0; i < 2; i++) { + values.push({ + left: {}, + right: {} + }); + + // Create local object + let obj = objects[i] = new Zotero.Item('note'); + obj.setNote(Zotero.Utilities.randomString()); + obj.version = 10; + obj.dateAdded = Zotero.Date.dateToSQL(new Date(dateAdded), true); + // Set Date Modified values one minute apart to enforce order + obj.dateModified = Zotero.Date.dateToSQL( + new Date(dateAdded + (i * 60000)), true + ); + yield obj.saveTx(); + + let jsonData = obj.toJSON(); + jsonData.key = obj.key; + jsonData.version = 10; + let json = { + key: obj.key, + version: jsonData.version, + data: jsonData + }; + // Save original version in cache + yield Zotero.Sync.Data.Local.saveCacheObjects(type, libraryID, [json]); + + // Create updated JSON for download + values[i].right.note = jsonData.note = Zotero.Utilities.randomString(); + values[i].right.version = json.version = jsonData.version = 15; + responseJSON.push(json); + + // Modify object locally + obj.setNote(Zotero.Utilities.randomString()); + yield obj.saveTx({ + skipDateModifiedUpdate: true + }); + values[i].left.note = obj.getNote(); + values[i].left.version = obj.getField('version'); + } + + setResponse({ + method: "GET", + url: `users/1/items?format=json&itemKey=${objects.map(o => o.key).join('%2C')}` + + `&includeTrashed=1`, + status: 200, + headers: { + "Last-Modified-Version": 15 + }, + json: responseJSON + }); + + waitForWindow('chrome://zotero/content/merge.xul', function (dialog) { + var doc = dialog.document; + var wizard = doc.documentElement; + var mergeGroup = wizard.getElementsByTagName('zoteromergegroup')[0]; + + // 1 (remote) + // Remote version should be selected by default + assert.equal(mergeGroup.rightpane.getAttribute('selected'), 'true'); + wizard.getButton('next').click(); + + // 2 (local) + assert.equal(mergeGroup.rightpane.getAttribute('selected'), 'true'); + // Select local object + mergeGroup.leftpane.click(); + assert.equal(mergeGroup.leftpane.getAttribute('selected'), 'true'); + if (Zotero.isMac) { + assert.isTrue(wizard.getButton('next').hidden); + assert.isFalse(wizard.getButton('finish').hidden); + } + else { + // TODO + } + wizard.getButton('finish').click(); + }); + yield engine._downloadObjects('item', objects.map(o => o.key)); + + assert.equal(objects[0].getNote(), values[0].right.note); + assert.equal(objects[1].getNote(), values[1].left.note); + assert.equal(objects[0].getField('version'), values[0].right.version); + assert.equal(objects[1].getField('version'), values[1].left.version); + + var keys = yield Zotero.Sync.Data.Local.getObjectsFromSyncQueue('item', libraryID); + assert.lengthOf(keys, 0); + }); + it("should resolve all remaining conflicts with one side", function* () { var libraryID = Zotero.Libraries.userLibraryID; ({ engine, client, caller } = yield setup()); @@ -2190,6 +2287,69 @@ describe("Zotero.Sync.Data.Engine", function () { assert.lengthOf(keys, 0); }) + it("should handle local child note deletion, keeping deletion", function* () { + var libraryID = Zotero.Libraries.userLibraryID; + ({ engine, client, caller } = yield setup()); + var responseJSON = []; + + var parent = yield createDataObject('item'); + + // Create object, generate JSON, and delete + var obj = new Zotero.Item('note'); + obj.parentItemID = parent.id; + obj.setNote(Zotero.Utilities.randomString()); + obj.version = 10; + yield obj.saveTx(); + var jsonData = obj.toJSON(); + var key = jsonData.key = obj.key; + jsonData.version = 10; + let json = { + key: obj.key, + version: jsonData.version, + data: jsonData + }; + // Delete object locally + yield obj.eraseTx(); + + json.version = jsonData.version = 15; + jsonData.note = Zotero.Utilities.randomString(); + responseJSON.push(json); + + setResponse({ + method: "GET", + url: `users/1/items?format=json&itemKey=${obj.key}&includeTrashed=1`, + status: 200, + headers: { + "Last-Modified-Version": 15 + }, + json: responseJSON + }); + + var windowOpened = false; + waitForWindow('chrome://zotero/content/merge.xul', function (dialog) { + windowOpened = true; + + var doc = dialog.document; + var wizard = doc.documentElement; + var mergeGroup = wizard.getElementsByTagName('zoteromergegroup')[0]; + + // Remote version should be selected by default + assert.equal(mergeGroup.rightpane.getAttribute('selected'), 'true'); + assert.ok(mergeGroup.leftpane.pane.onclick); + // Select local deleted version + mergeGroup.leftpane.pane.click(); + wizard.getButton('finish').click(); + }); + yield engine._downloadObjects('item', [obj.key]); + assert.isTrue(windowOpened); + + obj = Zotero.Items.getByLibraryAndKey(libraryID, key); + assert.isFalse(obj); + + var keys = yield Zotero.Sync.Data.Local.getObjectsFromSyncQueue('item', libraryID); + assert.lengthOf(keys, 0); + }); + it("should restore locally deleted item", function* () { var libraryID = Zotero.Libraries.userLibraryID; ({ engine, client, caller } = yield setup()); diff --git a/test/tests/syncLocalTest.js b/test/tests/syncLocalTest.js @@ -557,6 +557,47 @@ describe("Zotero.Sync.Data.Local", function() { }); + describe("#showConflictResolutionWindow()", function () { + it("should show title of note parent", function* () { + var parentItem = yield createDataObject('item', { title: "Parent" }); + var note = new Zotero.Item('note'); + note.parentKey = parentItem.key; + note.setNote("Test"); + yield note.saveTx(); + + var promise = waitForWindow('chrome://zotero/content/merge.xul', function (dialog) { + var doc = dialog.document; + var wizard = doc.documentElement; + var mergeGroup = wizard.getElementsByTagName('zoteromergegroup')[0]; + + // Show title for middle and right panes + var parentText = Zotero.getString('pane.item.parentItem') + " Parent"; + assert.equal(mergeGroup.leftpane._id('parent-row').textContent, ""); + assert.equal(mergeGroup.rightpane._id('parent-row').textContent, parentText); + assert.equal(mergeGroup.mergepane._id('parent-row').textContent, parentText); + + wizard.getButton('finish').click(); + }); + + Zotero.Sync.Data.Local.showConflictResolutionWindow([ + { + libraryID: note.libraryID, + key: note.key, + processed: false, + conflict: true, + left: { + deleted: true, + dateDeleted: "2016-07-07 12:34:56" + }, + right: note.toJSON() + } + ]); + + yield promise; + }); + }); + + describe("#_reconcileChanges()", function () { describe("items", function () { it("should ignore non-conflicting local changes and return remote changes", function () {