diff --git a/js/id/actions/merge_remote_changes.js b/js/id/actions/merge_remote_changes.js index f9e53b367..b6e72f362 100644 --- a/js/id/actions/merge_remote_changes.js +++ b/js/id/actions/merge_remote_changes.js @@ -1,4 +1,4 @@ -iD.actions.MergeRemoteChanges = function(id, remoteGraph, formatUser) { +iD.actions.MergeRemoteChanges = function(id, localGraph, remoteGraph, formatUser) { var option = 'safe', // 'safe', 'force_local', 'force_remote' conflicts = []; @@ -132,9 +132,19 @@ iD.actions.MergeRemoteChanges = function(id, remoteGraph, formatUser) { } + // `graph.base()` is the common ancestor of the two graphs. + // `localGraph` contains user's edits up to saving + // `remoteGraph` contains remote edits to modified nodes + // `graph` must be a descendent of `localGraph` and may include + // some conflict resolution actions performed on it. + // + // --- ... --- `localGraph` -- ... -- `graph` + // / + // `graph.base()` --- ... --- `remoteGraph` + // var action = function(graph) { var base = graph.base().entities[id], - local = graph.entity(id), + local = localGraph.entity(id), remote = remoteGraph.entity(id), target = iD.Entity(local, { version: remote.version }), replacements = []; @@ -142,6 +152,7 @@ iD.actions.MergeRemoteChanges = function(id, remoteGraph, formatUser) { if (target.type === 'node') { target = mergeLocation(remote, target); } else if (target.type === 'way') { + // pull in any child nodes that may not be present locally.. graph.rebase(remoteGraph.childNodes(remote), [graph], false); target = mergeChildNodes(target, graph.childNodes(local), replacements); target = mergeNodes(base, remote, target); diff --git a/js/id/modes/save.js b/js/id/modes/save.js index 6873c779b..3e9170a00 100644 --- a/js/id/modes/save.js +++ b/js/id/modes/save.js @@ -11,7 +11,8 @@ iD.modes.Save = function(context) { var loading = iD.ui.Loading(context).message(t('save.uploading')).blocking(true), history = context.history(), origChanges = history.changes(iD.actions.DiscardTags(history.difference())), - altGraph = iD.Graph(history.base(), true), + localGraph = context.graph(), + remoteGraph = iD.Graph(history.base(), true), modified = _.filter(history.difference().summary(), {changeType: 'modified'}), toCheck = _.pluck(_.pluck(modified, 'entity'), 'id'), deletedIds = [], @@ -45,7 +46,7 @@ iD.modes.Save = function(context) { } } else { - _.each(result.data, function(entity) { altGraph.replace(entity); }); + _.each(result.data, function(entity) { remoteGraph.replace(entity); }); checkConflicts(id); } @@ -85,17 +86,17 @@ iD.modes.Save = function(context) { function checkConflicts(id) { var graph = context.graph(), local = graph.entity(id), - remote = altGraph.entity(id); + remote = remoteGraph.entity(id); if (local.version !== remote.version) { var action = iD.actions.MergeRemoteChanges, - merge = action(id, altGraph, formatUser), + merge = action(id, localGraph, remoteGraph, formatUser), diff = history.replace(merge); if (diff.length()) return; // merged safely - var forceLocal = action(id, altGraph, formatUser).withOption('force_local'), - forceRemote = action(id, altGraph, formatUser).withOption('force_remote'); + var forceLocal = action(id, localGraph, remoteGraph, formatUser).withOption('force_local'), + forceRemote = action(id, localGraph, remoteGraph, formatUser).withOption('force_remote'); conflicts.push({ id: id, @@ -270,6 +271,16 @@ iD.modes.Save = function(context) { .text(function(d) { return d.text; }) .on('click', function(d) { d.action(); + d3.event.preventDefault(); + }); + + details + .append('div') + .attr('class', 'conflict-choice-buttons joined cf') + .append('button') + .attr('class', 'conflict-choice-button action col4') + .text(t('confirm.okay')) + .on('click', function(d) { var container = this.parentElement.parentElement.parentElement; var next = container.parentElement.firstElementChild.classList.contains('expanded') ? container.nextElementSibling : container.parentElement.firstElementChild; @@ -290,12 +301,14 @@ iD.modes.Save = function(context) { .transition() .style('opacity', 0) .remove(); + d3.event.preventDefault(); }); items.exit() .remove(); + function toggleExpanded(el, d) { var error = d3.select(el), detail = d3.select(el.getElementsByTagName('div')[0]), diff --git a/test/spec/actions/merge_remote_changes.js b/test/spec/actions/merge_remote_changes.js index 2f0b31869..80fd7ae1b 100644 --- a/test/spec/actions/merge_remote_changes.js +++ b/test/spec/actions/merge_remote_changes.js @@ -99,8 +99,8 @@ describe("iD.actions.MergeRemoteChanges", function () { 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); + remoteGraph = makeGraph([remote]), + action = iD.actions.MergeRemoteChanges('a', graph, remoteGraph); graph = action(graph); @@ -115,8 +115,8 @@ describe("iD.actions.MergeRemoteChanges", function () { 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); + remoteGraph = makeGraph([remote]), + action = iD.actions.MergeRemoteChanges('a', graph, remoteGraph); graph = action(graph); @@ -131,8 +131,8 @@ describe("iD.actions.MergeRemoteChanges", function () { 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); + remoteGraph = makeGraph([remote]), + action = iD.actions.MergeRemoteChanges('a', graph, remoteGraph); graph = action(graph); @@ -147,8 +147,8 @@ describe("iD.actions.MergeRemoteChanges", function () { 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); + remoteGraph = makeGraph([remote]), + action = iD.actions.MergeRemoteChanges('a', graph, remoteGraph); graph = action(graph); @@ -164,8 +164,8 @@ describe("iD.actions.MergeRemoteChanges", function () { 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); + remoteGraph = makeGraph([remote]), + action = iD.actions.MergeRemoteChanges('a', graph, remoteGraph); graph = action(graph); @@ -183,8 +183,8 @@ describe("iD.actions.MergeRemoteChanges", function () { local = iD.Way({id: 'w1', nodes: localNodes, version: '1', v: 2, tags: localTags}), remote = iD.Way({id: 'w1', nodes: remoteNodes, version: '2', tags: remoteTags}), graph = makeGraph([local]), - altGraph = makeGraph([remote]), - action = iD.actions.MergeRemoteChanges('w1', altGraph); + remoteGraph = makeGraph([remote]), + action = iD.actions.MergeRemoteChanges('w1', graph, remoteGraph); graph = action(graph); @@ -199,8 +199,8 @@ describe("iD.actions.MergeRemoteChanges", function () { local = iD.Way({id: 'w1', nodes: localNodes, version: '1', v: 2, tags: localTags}), remote = iD.Way({id: 'w1', nodes: remoteNodes, version: '2', tags: remoteTags}), graph = makeGraph([local]), - altGraph = makeGraph([remote]), - action = iD.actions.MergeRemoteChanges('w1', altGraph); + remoteGraph = makeGraph([remote]), + action = iD.actions.MergeRemoteChanges('w1', graph, remoteGraph); graph = action(graph); @@ -216,8 +216,8 @@ describe("iD.actions.MergeRemoteChanges", function () { local = iD.Way({id: 'w1', nodes: localNodes, version: '1', v: 2, tags: localTags}), remote = iD.Way({id: 'w1', nodes: remoteNodes, version: '2', tags: remoteTags}), graph = makeGraph([local]), - altGraph = makeGraph([remote, r2, r3]), - action = iD.actions.MergeRemoteChanges('w1', altGraph); + remoteGraph = makeGraph([remote, r2, r3]), + action = iD.actions.MergeRemoteChanges('w1', graph, remoteGraph); graph = action(graph); @@ -236,8 +236,8 @@ describe("iD.actions.MergeRemoteChanges", function () { local = iD.Way({id: 'w1', nodes: localNodes, version: '1', v: 2, tags: localTags}), remote = iD.Way({id: 'w1', nodes: remoteNodes, version: '2', tags: remoteTags}), graph = makeGraph([local, r2, r3]), - altGraph = makeGraph([remote]), - action = iD.actions.MergeRemoteChanges('w1', altGraph); + remoteGraph = makeGraph([remote]), + action = iD.actions.MergeRemoteChanges('w1', graph, remoteGraph); graph = action(graph); @@ -254,8 +254,8 @@ describe("iD.actions.MergeRemoteChanges", function () { local = iD.Way({id: 'w1', nodes: localNodes, version: '1', v: 2, tags: localTags}), remote = iD.Way({id: 'w1', nodes: remoteNodes, version: '2', tags: remoteTags}), graph = makeGraph([local, r1, r2]), - altGraph = makeGraph([remote, r3, r4]), - action = iD.actions.MergeRemoteChanges('w1', altGraph); + remoteGraph = makeGraph([remote, r3, r4]), + action = iD.actions.MergeRemoteChanges('w1', graph, remoteGraph); graph = action(graph); @@ -274,8 +274,8 @@ describe("iD.actions.MergeRemoteChanges", function () { local = iD.Way({id: 'w1', nodes: localNodes, version: '1', v: 2, tags: localTags}), remote = iD.Way({id: 'w1', nodes: remoteNodes, version: '2', tags: remoteTags}), graph = makeGraph([local, r1, r2]), - altGraph = makeGraph([remote, r3, r4]), - action = iD.actions.MergeRemoteChanges('w1', altGraph); + remoteGraph = makeGraph([remote, r3, r4]), + action = iD.actions.MergeRemoteChanges('w1', graph, remoteGraph); graph = action(graph); @@ -293,8 +293,8 @@ describe("iD.actions.MergeRemoteChanges", function () { local = iD.Relation({id: 'r', members: localMembers, version: '1', v: 2, tags: localRelTags}), remote = iD.Relation({id: 'r', members: remoteMembers, version: '2', tags: remoteRelTags}), graph = makeGraph([local]), - altGraph = makeGraph([s1, s2, s3, s4, w4]); - action = iD.actions.MergeRemoteChanges('r', altGraph); + remoteGraph = makeGraph([s1, s2, s3, s4, w4]); + action = iD.actions.MergeRemoteChanges('r', graph, remoteGraph); graph = action(graph); @@ -308,8 +308,8 @@ describe("iD.actions.MergeRemoteChanges", function () { local = iD.Relation({id: 'r', members: relMembers, version: '1', v: 2, tags: localRelTags}), remote = iD.Relation({id: 'r', members: relMembers, version: '2', tags: remoteRelTags}), graph = makeGraph([local]), - altGraph = makeGraph([remote]); - action = iD.actions.MergeRemoteChanges('r', altGraph); + remoteGraph = makeGraph([remote]); + action = iD.actions.MergeRemoteChanges('r', graph, remoteGraph); graph = action(graph); @@ -323,8 +323,8 @@ describe("iD.actions.MergeRemoteChanges", function () { local = iD.Relation({id: 'r', members: relMembers, version: '1', v: 2, tags: localRelTags}), remote = iD.Relation({id: 'r', members: relMembers, version: '2', tags: remoteRelTags}), graph = makeGraph([local]), - altGraph = makeGraph([remote]); - action = iD.actions.MergeRemoteChanges('r', altGraph); + remoteGraph = makeGraph([remote]); + action = iD.actions.MergeRemoteChanges('r', graph, remoteGraph); graph = action(graph); @@ -340,8 +340,8 @@ describe("iD.actions.MergeRemoteChanges", function () { 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', altGraph); + remoteGraph = makeGraph([remote]), + action = iD.actions.MergeRemoteChanges('a', graph, remoteGraph); graph = action(graph); @@ -360,8 +360,8 @@ describe("iD.actions.MergeRemoteChanges", function () { 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).withOption('force_local'); + remoteGraph = makeGraph([remote]), + action = iD.actions.MergeRemoteChanges('a', graph, remoteGraph).withOption('force_local'); graph = action(graph); @@ -378,8 +378,8 @@ describe("iD.actions.MergeRemoteChanges", function () { 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).withOption('force_remote'); + remoteGraph = makeGraph([remote]), + action = iD.actions.MergeRemoteChanges('a', graph, remoteGraph).withOption('force_remote'); graph = action(graph); @@ -398,8 +398,8 @@ describe("iD.actions.MergeRemoteChanges", function () { local = iD.Way({id: 'w1', nodes: localNodes, version: '1', v: 2, tags: localTags}), remote = iD.Way({id: 'w1', nodes: remoteNodes, version: '2', tags: remoteTags}), graph = makeGraph([local, r1]), - altGraph = makeGraph([remote, s3]), - action = iD.actions.MergeRemoteChanges('w1', altGraph).withOption('force_local'); + remoteGraph = makeGraph([remote, s3]), + action = iD.actions.MergeRemoteChanges('w1', graph, remoteGraph).withOption('force_local'); graph = action(graph); @@ -416,8 +416,8 @@ describe("iD.actions.MergeRemoteChanges", function () { local = iD.Way({id: 'w1', nodes: localNodes, version: '1', v: 2, tags: localTags}), remote = iD.Way({id: 'w1', nodes: remoteNodes, version: '2', tags: remoteTags}), graph = makeGraph([local, r1]), - altGraph = makeGraph([remote, s3]), - action = iD.actions.MergeRemoteChanges('w1', altGraph).withOption('force_remote'); + remoteGraph = makeGraph([remote, s3]), + action = iD.actions.MergeRemoteChanges('w1', graph, remoteGraph).withOption('force_remote'); graph = action(graph); @@ -437,8 +437,8 @@ describe("iD.actions.MergeRemoteChanges", function () { local = iD.Relation({id: 'r', members: localMembers, version: '1', v: 2, tags: localRelTags}), remote = iD.Relation({id: 'r', members: remoteMembers, version: '2', tags: remoteRelTags}), graph = makeGraph([local, r1, r2, r3, r4, w3]), - altGraph = makeGraph([remote, s1, s2, s3, s4, w4]), - action = iD.actions.MergeRemoteChanges('r', altGraph).withOption('force_local'); + remoteGraph = makeGraph([remote, s1, s2, s3, s4, w4]), + action = iD.actions.MergeRemoteChanges('r', graph, remoteGraph).withOption('force_local'); graph = action(graph); @@ -455,8 +455,8 @@ describe("iD.actions.MergeRemoteChanges", function () { local = iD.Relation({id: 'r', members: localMembers, version: '1', v: 2, tags: localRelTags}), remote = iD.Relation({id: 'r', members: remoteMembers, version: '2', tags: remoteRelTags}), graph = makeGraph([local, r1, r2, r3, r4, w3]), - altGraph = makeGraph([remote, s1, s2, s3, s4, w4]), - action = iD.actions.MergeRemoteChanges('r', altGraph).withOption('force_remote'); + remoteGraph = makeGraph([remote, s1, s2, s3, s4, w4]), + action = iD.actions.MergeRemoteChanges('r', graph, remoteGraph).withOption('force_remote'); graph = action(graph);