From b8e88a7aaf21ff61cac268493bb183335096c9ca Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 23 Oct 2013 13:20:57 -0700 Subject: [PATCH] Convert relevantChanges to a method on Difference --- index.html | 1 - js/id/core/difference.js | 44 +++++++++- js/id/core/history.js | 4 - js/id/ui/commit.js | 23 ++--- js/id/util/relevant_changes.js | 43 --------- test/index.html | 2 - test/index_packaged.html | 1 - test/spec/core/difference.js | 135 +++++++++++++++++++++++++++++ test/spec/util/relevant_changes.js | 107 ----------------------- 9 files changed, 184 insertions(+), 176 deletions(-) delete mode 100644 js/id/util/relevant_changes.js delete mode 100644 test/spec/util/relevant_changes.js diff --git a/index.html b/index.html index dae31a0fd..c88bace38 100644 --- a/index.html +++ b/index.html @@ -37,7 +37,6 @@ - diff --git a/js/id/core/difference.js b/js/id/core/difference.js index 9c1a80ca5..8b8ecbb3e 100644 --- a/js/id/core/difference.js +++ b/js/id/core/difference.js @@ -84,7 +84,6 @@ iD.Difference = function(base, head) { }; difference.addParents = function(entities) { - for (var i in entities) { addParents(head.parentWays(entities[i]), entities); addParents(head.parentRelations(entities[i]), entities); @@ -92,6 +91,49 @@ iD.Difference = function(base, head) { return entities; }; + difference.summary = function() { + var relevant = {}; + + function addEntity(entity, graph, changeType) { + relevant[entity.id] = { + entity: entity, + graph: graph, + changeType: changeType + }; + } + + function addParents(entity) { + var parents = head.parentWays(entity); + for (var j = parents.length - 1; j >= 0; j--) { + var parent = parents[j]; + if (!(parent.id in relevant)) addEntity(parent, head, 'modified'); + } + } + + _.each(changes, function(change) { + if (change.head && change.head.geometry(head) !== 'vertex') { + addEntity(change.head, head, change.base ? 'modified' : 'created'); + + } else if (change.base && change.base.geometry(base) !== 'vertex') { + addEntity(change.base, base, 'deleted'); + + } else if (change.base && change.head) { // modified vertex + var moved = change.base.loc !== change.head.loc, + retagged = change.base.tags !== change.head.tags; + + if (moved) { + addParents(change.head); + } + + if (retagged || (moved && change.head.hasInterestingTags())) { + addEntity(change.head, head, 'modified'); + } + } + }); + + return d3.values(relevant); + }; + difference.complete = function(extent) { var result = {}, id, change; diff --git a/js/id/core/history.js b/js/id/core/history.js index dbc5aaa2a..98f27a6d7 100644 --- a/js/id/core/history.js +++ b/js/id/core/history.js @@ -41,10 +41,6 @@ iD.History = function(context) { return stack[index].graph; }, - base: function() { - return stack[0].graph; - }, - merge: function(entities, extent) { var base = stack[0].graph.base(), diff --git a/js/id/ui/commit.js b/js/id/ui/commit.js index 76794e486..6adc9511f 100644 --- a/js/id/ui/commit.js +++ b/js/id/ui/commit.js @@ -1,15 +1,9 @@ iD.ui.Commit = function(context) { - var event = d3.dispatch('cancel', 'save'), - presets = context.presets(); + var event = d3.dispatch('cancel', 'save'); function commit(selection) { var changes = context.history().changes(), - base = context.history().base(), - relevantChanges = iD.util.relevantChanges( - context.graph(), - changes, - base - ); + summary = context.history().difference().summary(); function zoomToEntity(change) { var entity = change.entity; @@ -131,14 +125,12 @@ iD.ui.Commit = function(context) { .attr('class', 'commit-section modal-section fillL2'); changeSection.append('h3') - .text(relevantChanges.length + ' Changes'); + .text(summary.length + ' Changes'); var li = changeSection.append('ul') .attr('class', 'changeset-list') .selectAll('li') - .data(function(d) { - return relevantChanges; - }) + .data(summary) .enter() .append('li') .on('mouseover', mouseover) @@ -147,9 +139,7 @@ iD.ui.Commit = function(context) { li.append('span') .attr('class', function(d) { - var graph = d.changeType === 'deleted' ? base : context.graph(); - return graph.entity(d.entity.id).geometry(graph) + ' ' + d.changeType + - ' icon icon-pre-text'; + return d.entity.geometry(d.graph) + ' ' + d.changeType + ' icon icon-pre-text'; }); li.append('span') @@ -161,8 +151,7 @@ iD.ui.Commit = function(context) { li.append('strong') .attr('class', 'entity-type') .text(function(d) { - var graph = d.changeType === 'deleted' ? base : context.graph(); - return context.presets().match(d.entity, graph).name(); + return context.presets().match(d.entity, d.graph).name(); }); li.append('span') diff --git a/js/id/util/relevant_changes.js b/js/id/util/relevant_changes.js deleted file mode 100644 index 869ae27c5..000000000 --- a/js/id/util/relevant_changes.js +++ /dev/null @@ -1,43 +0,0 @@ -// filters out verticies where the parent entity is already present -// for simpler changeset listing -iD.util.relevantChanges = function(graph, changes, base) { - var relevant = {}; - - function addEntity(entity, changeType) { - relevant[entity.id] = { - entity: entity, - changeType: changeType - }; - } - - function addParents(entity, theGraph) { - if (!theGraph) theGraph = graph; - var parents = theGraph.parentWays(entity); - for (var j = parents.length - 1; j >= 0; j--) { - var parent = parents[j]; - if (!(parent.id in relevant)) addEntity(parent, 'modified'); - } - } - - _.each(changes, function(entities, change) { - _.each(entities, function(entity) { - if (entity.geometry(change === 'deleted' ? base : graph) !== 'vertex') { - addEntity(entity, change); - - } else if (change === 'modified') { - var moved = entity.loc !== base.entity(entity.id).loc, - retagged = entity.tags !== base.entity(entity.id).tags; - - if (moved) { - addParents(entity, graph); - } - - if (retagged || (moved && entity.hasInterestingTags())) { - addEntity(entity, change); - } - } - }); - }); - - return d3.values(relevant); -}; diff --git a/test/index.html b/test/index.html index 170cc5ea4..14fb4087e 100644 --- a/test/index.html +++ b/test/index.html @@ -192,7 +192,6 @@ - @@ -274,7 +273,6 @@ - diff --git a/test/index_packaged.html b/test/index_packaged.html index 650a0b886..356e4b687 100644 --- a/test/index_packaged.html +++ b/test/index_packaged.html @@ -91,7 +91,6 @@ - diff --git a/test/spec/core/difference.js b/test/spec/core/difference.js index 5fb006224..5f128acce 100644 --- a/test/spec/core/difference.js +++ b/test/spec/core/difference.js @@ -126,6 +126,141 @@ describe("iD.Difference", function () { }); }); + describe("#summary", function () { + var base = iD.Graph({ + 'a': iD.Node({id: 'a', tags: {crossing: 'zebra'}}), + 'b': iD.Node({id: 'b'}), + '-': iD.Way({id: '-', nodes: ['a', 'b']}) + }); + + it("reports a created way as created", function() { + var way = iD.Way({id: '+'}), + head = base.replace(way), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'created', + entity: way, + graph: head + }]); + }); + + it("reports a deleted way as deleted", function() { + var way = base.entity('-'), + head = base.remove(way), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'deleted', + entity: way, + graph: base + }]); + }); + + it("reports a modified way as modified", function() { + var way = base.entity('-').mergeTags({highway: 'primary'}), + head = base.replace(way), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'modified', + entity: way, + graph: head + }]); + }); + + it("reports a way as modified when a member vertex is moved", function() { + var vertex = base.entity('b').move([0,3]), + head = base.replace(vertex), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'modified', + entity: head.entity('-'), + graph: head + }]); + }); + + it("reports a way as modified when a member vertex is added", function() { + var vertex = iD.Node({id: 'c'}), + way = base.entity('-').addNode('c'), + head = base.replace(vertex).replace(way), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'modified', + entity: way, + graph: head + }]); + }); + + it("reports a way as modified when a member vertex is removed", function() { + var way = base.entity('-').removeNode('b'), + head = base.replace(way), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'modified', + entity: way, + graph: head + }]); + }); + + it("reports a created way containing a moved vertex as being created", function() { + var vertex = base.entity('b').move([0,3]), + way = iD.Way({id: '+', nodes: ['b']}), + head = base.replace(way).replace(vertex), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'created', + entity: way, + graph: head + }, { + changeType: 'modified', + entity: head.entity('-'), + graph: head + }]); + }); + + it("reports a created way with a created vertex as being created", function() { + var vertex = iD.Node({id: 'c'}), + way = iD.Way({id: '+', nodes: ['c']}), + head = base.replace(vertex).replace(way), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'created', + entity: way, + graph: head + }]); + }); + + it("reports an existing vertex with added tags as modified", function() { + var vertex = base.entity('a').mergeTags({highway: 'traffic_signals'}), + head = base.replace(vertex), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'modified', + entity: vertex, + graph: head + }]); + }); + + it("reports an existing tagged vertex that is moved as modified", function() { + var vertex = base.entity('a').move([1, 2]), + head = base.replace(vertex), + diff = iD.Difference(base, head); + + expect(diff.summary()[1]).to.eql({ + changeType: 'modified', + entity: vertex, + graph: head + }); + }); + }); + describe("#complete", function () { it("includes created entities", function () { var node = iD.Node({id: 'n'}), diff --git a/test/spec/util/relevant_changes.js b/test/spec/util/relevant_changes.js deleted file mode 100644 index 23ee5f631..000000000 --- a/test/spec/util/relevant_changes.js +++ /dev/null @@ -1,107 +0,0 @@ -describe("iD.util.relevantChanges", function() { - var base = iD.Graph({ - 'a': iD.Node({id: 'a', loc: [0, 0], tags: {crossing: 'zebra'}}), - 'b': iD.Node({id: 'b', loc: [2, 0]}), - 'c': iD.Node({id: 'c', loc: [2, 2]}), - 'd': iD.Node({id: 'd', loc: [0, 2]}), - 'e': iD.Node({id: 'e', loc: [0, 2]}), - '-': iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'e', 'a']}) - }); - - it("returns a way that changed", function() { - var way = iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'a']}), - graph = base.replace(way), - changes = { modified: [way] }, - a = iD.util.relevantChanges(graph, changes, base); - expect(a).to.eql([{ - changeType: 'modified', - entity: way - }]); - }); - - it("reports an existing modified way, leaving out the verticies", function() { - var way = iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'e', 'a']}), - vertex = iD.Node({id: 'e', loc: [0, 3]}), - graph = base.replace(way).replace(vertex), - changes = { modified: [way, vertex] }, - a = iD.util.relevantChanges(graph, changes, base); - expect(a).to.eql([{ - changeType: 'modified', - entity: way - }]); - }); - - it("reports an existing way as modified when a member vertex is modified", function() { - var vertex = base.entity('e').move([0,3]), - graph = base.replace(vertex), - changes = { modified: [vertex], deleted: [] }, - a = iD.util.relevantChanges(graph, changes, base); - expect(a).to.eql([{ - changeType: 'modified', - entity: graph.entity('-') - }]); - }); - - it("reports a created way containing a moved vertex as being created", function() { - var vertex = base.entity('e').move([0,3]), - way = iD.Way({id: '+', nodes: ['e']}), - graph = base.replace(way).replace(vertex), - changes = { created: [way], modified: [vertex, graph.entity('-')] }, - a = iD.util.relevantChanges(graph, changes, base); - expect(a).to.eql([{ - changeType: 'created', - entity: way - }, { - changeType: 'modified', - entity: graph.entity('-') - }]); - }); - - it("reports an existing way with an added vertex as being modified", function() { - var vertex = iD.Node({id: 'f'}), - graph = base.replace(vertex).replace(base.entity('-').addNode('f')); - var changes = { - created: [vertex], - modified: [graph.entity('-')] - }, - a = iD.util.relevantChanges(graph, changes, base); - expect(a).to.eql([{ - changeType: 'modified', - entity: graph.entity('-') - }]); - }); - - it("reports a created way with a created vertex as being created", function() { - var vertex = iD.Node({id: 'f'}), - way = iD.Way({id: '+', nodes: ['f']}), - graph = base.replace(vertex).replace(way), - changes = { created: [way, vertex] }, - a = iD.util.relevantChanges(graph, changes, base); - expect(a).to.eql([{ - changeType: 'created', - entity: way - }]); - }); - - it("reports an existing vertex with added tags as modified", function() { - var vertex = base.entity('a').mergeTags({highway: 'traffic_signals'}), - graph = base.replace(vertex), - changes = { modified: [vertex] }, - a = iD.util.relevantChanges(graph, changes, base); - expect(a).to.eql([{ - changeType: 'modified', - entity: vertex - }]); - }); - - it("reports an existing tagged vertex that is moved as modified", function() { - var vertex = base.entity('a').move([1, 2]), - graph = base.replace(vertex), - changes = { modified: [vertex] }, - a = iD.util.relevantChanges(graph, changes, base); - expect(a[1]).to.eql({ - changeType: 'modified', - entity: vertex - }); - }); -});