From da9602795cdc0585e5ec2ce74617eb4d822f27ec Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 16 May 2013 16:43:41 -0700 Subject: [PATCH] Don't allow deleting incomplete relations This will fail with an "entity not found" error. --- data/core.yaml | 1 + dist/locales/en.json | 6 +++--- js/id/actions/delete_multiple.js | 22 ++++++++++++++++------ js/id/actions/delete_node.js | 8 +++++++- js/id/actions/delete_relation.js | 9 ++++++++- js/id/actions/delete_way.js | 8 +++++++- js/id/operations/delete.js | 11 ++++++++--- test/spec/actions/delete_multiple.js | 10 ++++++++++ test/spec/actions/delete_relation.js | 9 +++++++++ 9 files changed, 69 insertions(+), 15 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index 555d9bcd8..6ad38943d 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -65,6 +65,7 @@ en: area: Deleted an area. relation: Deleted a relation. multiple: "Deleted {n} objects." + incomplete_relation: This feature can't be deleted because it hasn't been fully downloaded. connect: annotation: point: Connected a way to a point. diff --git a/dist/locales/en.json b/dist/locales/en.json index cb78fa1e1..e00242851 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -84,7 +84,8 @@ "area": "Deleted an area.", "relation": "Deleted a relation.", "multiple": "Deleted {n} objects." - } + }, + "incomplete_relation": "This feature can't be deleted because it hasn't been fully downloaded." }, "connect": { "annotation": { @@ -156,7 +157,7 @@ }, "nothing_to_undo": "Nothing to undo.", "nothing_to_redo": "Nothing to redo.", - "tooltip_keyhint": "Shortcut: ", + "tooltip_keyhint": "Shortcut:", "just_edited": "You just edited OpenStreetMap!", "browser_notice": "This editor is supported in Firefox, Chrome, Safari, Opera, and Internet Explorer 9 and above. Please upgrade your browser or use Potlatch 2 to edit the map.", "view_on_osm": "View on OSM", @@ -166,7 +167,6 @@ "localized_translation_language": "Choose language", "localized_translation_name": "Name" }, - "localized_translation_label": "Translation", "zoom_in_edit": "zoom in to edit the map", "logout": "logout", "loading_auth": "Connecting to OpenStreetMap...", diff --git a/js/id/actions/delete_multiple.js b/js/id/actions/delete_multiple.js index c3e6d8477..f91cc0fdb 100644 --- a/js/id/actions/delete_multiple.js +++ b/js/id/actions/delete_multiple.js @@ -1,11 +1,11 @@ iD.actions.DeleteMultiple = function(ids) { - return function(graph) { - var actions = { - way: iD.actions.DeleteWay, - node: iD.actions.DeleteNode, - relation: iD.actions.DeleteRelation - }; + var actions = { + way: iD.actions.DeleteWay, + node: iD.actions.DeleteNode, + relation: iD.actions.DeleteRelation + }; + var action = function(graph) { ids.forEach(function(id) { if (graph.hasEntity(id)) { // It may have been deleted aready. graph = actions[graph.entity(id).type](id)(graph); @@ -14,4 +14,14 @@ iD.actions.DeleteMultiple = function(ids) { return graph; }; + + action.disabled = function(graph) { + for (var i = 0; i < ids.length; i++) { + var id = ids[i], + disabled = actions[graph.entity(id).type](id).disabled(graph); + if (disabled) return disabled; + } + }; + + return action; }; diff --git a/js/id/actions/delete_node.js b/js/id/actions/delete_node.js index a93129ce3..25e2dee0d 100644 --- a/js/id/actions/delete_node.js +++ b/js/id/actions/delete_node.js @@ -1,6 +1,6 @@ // https://github.com/openstreetmap/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/DeleteNodeAction.as iD.actions.DeleteNode = function(nodeId) { - return function(graph) { + var action = function(graph) { var node = graph.entity(nodeId); graph.parentWays(node) @@ -20,4 +20,10 @@ iD.actions.DeleteNode = function(nodeId) { return graph.remove(node); }; + + action.disabled = function() { + return false; + }; + + return action; }; diff --git a/js/id/actions/delete_relation.js b/js/id/actions/delete_relation.js index 783f1533c..b19522316 100644 --- a/js/id/actions/delete_relation.js +++ b/js/id/actions/delete_relation.js @@ -6,7 +6,7 @@ iD.actions.DeleteRelation = function(relationId) { !entity.hasInterestingTags(); } - return function(graph) { + var action = function(graph) { var relation = graph.entity(relationId); graph.parentRelations(relation) @@ -25,4 +25,11 @@ iD.actions.DeleteRelation = function(relationId) { return graph.remove(relation); }; + + action.disabled = function(graph) { + if (!graph.entity(relationId).isComplete(graph)) + return 'incomplete_relation'; + }; + + return action; }; diff --git a/js/id/actions/delete_way.js b/js/id/actions/delete_way.js index 9d98a697b..911b8ac8a 100644 --- a/js/id/actions/delete_way.js +++ b/js/id/actions/delete_way.js @@ -6,7 +6,7 @@ iD.actions.DeleteWay = function(wayId) { !node.hasInterestingTags(); } - return function(graph) { + var action = function(graph) { var way = graph.entity(wayId); graph.parentRelations(way) @@ -25,4 +25,10 @@ iD.actions.DeleteWay = function(wayId) { return graph.remove(way); }; + + action.disabled = function() { + return false; + }; + + return action; }; diff --git a/js/id/operations/delete.js b/js/id/operations/delete.js index 8d6d9d3d7..0ac6b5108 100644 --- a/js/id/operations/delete.js +++ b/js/id/operations/delete.js @@ -1,4 +1,6 @@ iD.operations.Delete = function(selection, context) { + var action = iD.actions.DeleteMultiple(selection); + var operation = function() { var annotation; @@ -9,7 +11,7 @@ iD.operations.Delete = function(selection, context) { } context.perform( - iD.actions.DeleteMultiple(selection), + action, annotation); context.enter(iD.modes.Browse(context)); @@ -20,11 +22,14 @@ iD.operations.Delete = function(selection, context) { }; operation.disabled = function() { - return false; + return action.disabled(context.graph()); }; operation.tooltip = function() { - return t('operations.delete.description'); + var disable = operation.disabled(); + return disable ? + t('operations.delete.' + disable) : + t('operations.delete.description'); }; operation.id = "delete"; diff --git a/test/spec/actions/delete_multiple.js b/test/spec/actions/delete_multiple.js index 3f73b42f9..8ef2eb2d8 100644 --- a/test/spec/actions/delete_multiple.js +++ b/test/spec/actions/delete_multiple.js @@ -18,4 +18,14 @@ describe("iD.actions.DeleteMultiple", function () { expect(graph.hasEntity(w.id)).to.be.undefined; expect(graph.hasEntity(n.id)).to.be.undefined; }); + + describe("#disabled", function () { + it("returns the result of the first action that is disabled", function () { + var node = iD.Node(), + relation = iD.Relation({members: [{id: 'w'}]}), + graph = iD.Graph([node, relation]), + action = iD.actions.DeleteMultiple([node.id, relation.id]); + expect(action.disabled(graph)).to.equal('incomplete_relation'); + }); + }); }); diff --git a/test/spec/actions/delete_relation.js b/test/spec/actions/delete_relation.js index a6ca0714f..b3637e4c9 100644 --- a/test/spec/actions/delete_relation.js +++ b/test/spec/actions/delete_relation.js @@ -73,4 +73,13 @@ describe("iD.actions.DeleteRelation", function () { graph = action(iD.Graph([node, way, relation])); expect(graph.hasEntity(node.id)).to.be.undefined; }); + + describe("#disabled", function() { + it("returns 'incomplete_relation' if the relation is incomplete", function() { + var relation = iD.Relation({members: [{id: 'w'}]}), + graph = iD.Graph([relation]), + action = iD.actions.DeleteRelation(relation.id); + expect(action.disabled(graph)).to.equal('incomplete_relation'); + }); + }); });