diff --git a/js/id/actions/merge_remote_changes.js b/js/id/actions/merge_remote_changes.js index 5f6fcfe94..f9e53b367 100644 --- a/js/id/actions/merge_remote_changes.js +++ b/js/id/actions/merge_remote_changes.js @@ -108,18 +108,21 @@ iD.actions.MergeRemoteChanges = function(id, remoteGraph, formatUser) { } var ccount = conflicts.length, - keys = _.reject(_.union(_.keys(base.tags), _.keys(remote.tags)), ignoreKey), - tags = _.clone(target.tags), + o = base.tags || {}, + a = target.tags || {}, + b = remote.tags || {}, + keys = _.reject(_.union(_.keys(o), _.keys(a), _.keys(b)), ignoreKey), + tags = _.clone(a), changed = false; for (var i = 0; i < keys.length; i++) { var k = keys[i]; - if (remote.tags[k] !== base.tags[k]) { // tag modified remotely.. - if (target.tags[k] && target.tags[k] !== remote.tags[k]) { + if (o[k] !== b[k] && a[k] !== b[k]) { // changed remotely.. + if (o[k] !== a[k]) { // changed locally.. conflicts.push(t('merge_remote_changes.conflict.tags', - { tag: k, local: target.tags[k], remote: remote.tags[k], user: user(remote.user) })); + { tag: k, local: a[k], remote: b[k], user: user(remote.user) })); } else { - tags[k] = remote.tags[k]; + tags[k] = b[k]; // unchanged locally, accept remote tag.. changed = true; } } diff --git a/test/spec/actions/merge_remote_changes.js b/test/spec/actions/merge_remote_changes.js index b90a1976c..2f0b31869 100644 --- a/test/spec/actions/merge_remote_changes.js +++ b/test/spec/actions/merge_remote_changes.js @@ -107,7 +107,7 @@ describe("iD.actions.MergeRemoteChanges", function () { expect(graph.entity('a')).to.eql(local); }); - it("doesn't merge nodes if changed tags conflict", function () { + it("doesn't merge nodes if changed tags conflict (tag change)", function () { var localTags = {foo: 'foo_local'}, // changed tag foo remoteTags = {foo: 'foo_remote', bar: 'bar_remote'}, // changed tag foo, added tag bar localLoc = [1, 1], // didn't move node @@ -123,7 +123,23 @@ describe("iD.actions.MergeRemoteChanges", function () { expect(graph.entity('a')).to.eql(local); }); - it("merges nodes if location is same and changed tags don't conflict", function () { + it("doesn't merge nodes if changed tags conflict (tag delete)", function () { + var localTags = {}, // deleted tag foo + remoteTags = {foo: 'foo_remote'}, // changed tag foo + localLoc = [1, 1], // didn't move node + remoteLoc = [1, 1], // didn't move node + local = iD.Node({id: 'a', loc: localLoc, version: '1', v: 2, tags: localTags }), + remote = iD.Node({id: 'a', loc: remoteLoc, version: '2', tags: remoteTags}), + graph = makeGraph([local]), + altGraph = makeGraph([remote]), + action = iD.actions.MergeRemoteChanges('a', altGraph); + + graph = action(graph); + + expect(graph.entity('a')).to.eql(local); + }); + + it("merges nodes if location is same and changed tags don't conflict (tag change)", function () { var localTags = {foo: 'foo_local'}, // changed tag foo remoteTags = {foo: 'foo', bar: 'bar_remote'}, // didn't change tag foo, added tag bar localLoc = [1, 1], // didn't move node @@ -139,6 +155,23 @@ describe("iD.actions.MergeRemoteChanges", function () { expect(graph.entity('a').version).to.eql('2'); expect(graph.entity('a').tags).to.eql({foo: 'foo_local', bar: 'bar_remote'}); }); + + it("merges nodes if location is same and changed tags don't conflict (tag delete)", function () { + var localTags = {}, // deleted tag foo + remoteTags = {foo: 'foo', bar: 'bar_remote'}, // didn't change tag foo, added tag bar + localLoc = [1, 1], // didn't move node + remoteLoc = [1, 1], // didn't move node + local = iD.Node({id: 'a', loc: localLoc, version: '1', v: 2, tags: localTags }), + remote = iD.Node({id: 'a', loc: remoteLoc, version: '2', tags: remoteTags}), + graph = makeGraph([local]), + altGraph = makeGraph([remote]), + action = iD.actions.MergeRemoteChanges('a', altGraph); + + graph = action(graph); + + expect(graph.entity('a').version).to.eql('2'); + expect(graph.entity('a').tags).to.eql({bar: 'bar_remote'}); + }); }); describe("ways", function () {