From 3bbf31902a96c2ca4f546a79d98698a45607ed6c Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 29 Dec 2014 22:47:44 -0500 Subject: [PATCH] Add accessor to get conflict details from iD.actions.MergeRemoteChanges --- data/core.yaml | 12 +++++- dist/locales/en.json | 14 ++++++- js/id/actions/merge_remote_changes.js | 21 +++++++--- js/id/util.js | 8 ++++ test/spec/actions/merge_remote_changes.js | 48 ++++++++++++++++++++++- 5 files changed, 94 insertions(+), 9 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index 840439a17..26ed9526d 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -312,10 +312,20 @@ en: title: Save help: "Save changes to OpenStreetMap, making them visible to other users." no_changes: No changes to save. - error: An error occurred while trying to save + error: Errors occurred while trying to save + status_code: "Server returned status code {code}" + status_gone: '{type} "{id}" {name} has already been deleted.' unknown_error_details: "Please ensure you are connected to the internet." uploading: Uploading changes to OpenStreetMap. unsaved_changes: You have unsaved changes + merge_remote_changes: + annotation: Merged remote changes from server. + conflict: + general: 'Conflicting edits were made to {type} "{id}" {name}' + location: Location was changed both locally and remotely. + nodelist: Nodes were changed both locally and remotely. + memberlist: Relation members were changed both locally and remotely. + tags: 'Tag "{tag}" was changed to "{local}" locally and "{remote}" remotely.' success: edited_osm: "Edited OSM!" just_edited: "You just edited OpenStreetMap!" diff --git a/dist/locales/en.json b/dist/locales/en.json index acd84156f..226480525 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -386,11 +386,23 @@ "title": "Save", "help": "Save changes to OpenStreetMap, making them visible to other users.", "no_changes": "No changes to save.", - "error": "An error occurred while trying to save", + "error": "Errors occurred while trying to save", + "status_code": "Server returned status code {code}", + "status_gone": "{type} \"{id}\" {name} has already been deleted.", "unknown_error_details": "Please ensure you are connected to the internet.", "uploading": "Uploading changes to OpenStreetMap.", "unsaved_changes": "You have unsaved changes" }, + "merge_remote_changes": { + "annotation": "Merged remote changes from server.", + "conflict": { + "general": "Conflicting edits were made to {type} \"{id}\" {name}", + "location": "Location was changed both locally and remotely.", + "nodelist": "Nodes were changed both locally and remotely.", + "memberlist": "Relation members were changed both locally and remotely.", + "tags": "Tag \"{tag}\" was changed to \"{local}\" locally and \"{remote}\" remotely." + } + }, "success": { "edited_osm": "Edited OSM!", "just_edited": "You just edited OpenStreetMap!", diff --git a/js/id/actions/merge_remote_changes.js b/js/id/actions/merge_remote_changes.js index d1c94908a..b5c74635f 100644 --- a/js/id/actions/merge_remote_changes.js +++ b/js/id/actions/merge_remote_changes.js @@ -2,7 +2,8 @@ iD.actions.MergeRemoteChanges = function(id, localGraph, remoteGraph) { var base = localGraph.base().entities[id], local = localGraph.entity(id), remote = remoteGraph.entity(id), - option = 'safe'; // 'safe', 'force_local', 'force_remote' + option = 'safe', // 'safe', 'force_local', 'force_remote' + conflicts = []; function mergeLocation(target) { @@ -20,6 +21,7 @@ iD.actions.MergeRemoteChanges = function(id, localGraph, remoteGraph) { return target.update({loc: remote.loc}); } + conflicts.push(t('merge_remote_changes.conflict.location')); return; // fail merge } @@ -55,6 +57,7 @@ iD.actions.MergeRemoteChanges = function(id, localGraph, remoteGraph) { nodes.push.apply(nodes, c.a); } else { // changed both locally and remotely + conflicts.push(t('merge_remote_changes.conflict.nodelist')); return; // fail merge.. } } @@ -73,6 +76,7 @@ iD.actions.MergeRemoteChanges = function(id, localGraph, remoteGraph) { return target.update({members: remote.members}); } + conflicts.push(t('merge_remote_changes.conflict.memberlist')); return; // fail merge } @@ -87,8 +91,9 @@ iD.actions.MergeRemoteChanges = function(id, localGraph, remoteGraph) { } var keys = _.reject(_.union(_.keys(base.tags), _.keys(remote.tags)), ignoreKey), - tags = _.cloneDeep(target.tags), - changed = false; + tags = _.clone(target.tags), + changed = false, + fail = false; function ignoreKey(k) { return k.indexOf('tiger:') === 0 || _.contains(iD.data.discarded, k); @@ -98,7 +103,9 @@ iD.actions.MergeRemoteChanges = function(id, localGraph, remoteGraph) { var k = keys[i]; if (remote.tags[k] !== base.tags[k]) { // tag modified remotely.. if (target.tags[k] && target.tags[k] !== remote.tags[k]) { - return; // fail merge.. + conflicts.push(t('merge_remote_changes.conflict.tags', + { tag: k, local: target.tags[k], remote: remote.tags[k] })); + fail = true; } else { tags[k] = remote.tags[k]; @@ -107,7 +114,7 @@ iD.actions.MergeRemoteChanges = function(id, localGraph, remoteGraph) { } } - return changed ? target.update({tags: tags}) : target; + return fail ? undefined : changed ? target.update({tags: tags}) : target; } var action = function(graph) { @@ -137,5 +144,9 @@ iD.actions.MergeRemoteChanges = function(id, localGraph, remoteGraph) { return action; }; + action.conflicts = function() { + return conflicts; + }; + return action; }; diff --git a/js/id/util.js b/js/id/util.js index 660f164dd..273d40338 100644 --- a/js/id/util.js +++ b/js/id/util.js @@ -30,6 +30,14 @@ iD.util.displayName = function(entity) { return entity.tags[localeName] || entity.tags.name || entity.tags.ref; }; +iD.util.displayType = function(id) { + return { + n: t('inspector.node'), + w: t('inspector.way'), + r: t('inspector.relation') + }[id.charAt(0)]; +}; + iD.util.stringQs = function(str) { return str.split('&').reduce(function(obj, pair){ var parts = pair.split('='); diff --git a/test/spec/actions/merge_remote_changes.js b/test/spec/actions/merge_remote_changes.js index 17e6a2722..d6c193b7d 100644 --- a/test/spec/actions/merge_remote_changes.js +++ b/test/spec/actions/merge_remote_changes.js @@ -53,7 +53,36 @@ describe("iD.actions.MergeRemoteChanges", function () { nodes: ['s1', 's2', 's3', 's4', 's1'], version: '1', tags: {foo: 'foo_new', area: 'yes'} - }); + }), + + saved, error; + + // setup mock locale object.. + beforeEach(function() { + saved = locale; + error = console.error; + console.error = function () {}; + locale = { + _current: 'en', + en: { + "merge_remote_changes": { + "annotation": "Merged remote changes from server.", + "conflict": { + "general": "Conflicting edits were made to {type} {id} {name}", + "location": "Location was changed both locally and remotely.", + "nodelist": "Nodes were changed both locally and remotely.", + "memberlist": "Relation members were changed both locally and remotely.", + "tags": "Tag \"{tag}\" was changed to \"{local}\" locally and \"{remote}\" remotely." + } + } + } + }; + }); + + afterEach(function() { + locale = saved; + console.error = error; + }); function makeGraph(entities) { return _.reduce(entities, function(graph, entity) { @@ -61,7 +90,6 @@ describe("iD.actions.MergeRemoteChanges", function () { }, iD.Graph(base)); } - describe("non-destuctive merging", function () { describe("nodes", function () { it("doesn't merge nodes if location is different", function () { @@ -272,6 +300,22 @@ describe("iD.actions.MergeRemoteChanges", function () { expect(graph.entity('r').tags).to.eql({type: 'multipolygon', foo: 'foo_local', bar: 'bar_remote'}); }); }); + + describe("#conflicts", function () { + it("returns conflict details", function () { + var localLoc = [1, 1], // didn't move node + remoteLoc = [3, 3], // moved node + local = iD.Node({id: 'a', loc: localLoc, version: '1', v: 2}), + remote = iD.Node({id: 'a', loc: remoteLoc, version: '2'}), + graph = makeGraph([local]), + altGraph = makeGraph([remote]), + action = iD.actions.MergeRemoteChanges('a', graph, altGraph); + + graph = action(graph); + + expect(action.conflicts()).not.to.be.empty; + }); + }); }); describe("destuctive merging", function () {