From fc7cc3177d1d34c5ba4ba6f49935a8a69db6a1ae Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Tue, 19 Mar 2019 12:39:41 -0400 Subject: [PATCH] Make conditions for being able to delete features via quick fixes the same as for the delete operation (close #6062) Simplify some validation code --- modules/validations/disconnected_way.js | 82 +++++++++++++------------ modules/validations/generic_name.js | 51 ++++++++------- modules/validations/many_deletions.js | 7 +-- modules/validations/missing_tag.js | 57 +++++++++-------- modules/validations/old_multipolygon.js | 59 +++++++++--------- modules/validations/unknown_road.js | 44 ++++++++----- 6 files changed, 152 insertions(+), 148 deletions(-) diff --git a/modules/validations/disconnected_way.js b/modules/validations/disconnected_way.js index 5993d7663..1e7c2be9b 100644 --- a/modules/validations/disconnected_way.js +++ b/modules/validations/disconnected_way.js @@ -31,61 +31,63 @@ export function validationDisconnectedWay() { var validation = function(entity, context) { - var issues = []; var graph = context.graph(); - if (isDisconnectedHighway(entity, graph)) { - var entityLabel = utilDisplayLabel(entity, context); - var fixes = []; + if (!isDisconnectedHighway(entity, graph)) return []; - if (!entity.isClosed()) { - var first = context.entity(entity.first()); - if (first.tags.noexit !== 'yes') { - fixes.push(new validationIssueFix({ - icon: 'iD-operation-continue-left', - title: t('issues.fix.continue_from_start.title'), - entityIds: [entity.first()], - onClick: function() { - var vertex = context.entity(entity.first()); - continueDrawing(entity, vertex, context); - } - })); - } - var last = context.entity(entity.last()); - if (last.tags.noexit !== 'yes') { - fixes.push(new validationIssueFix({ - icon: 'iD-operation-continue', - title: t('issues.fix.continue_from_end.title'), - entityIds: [entity.last()], - onClick: function() { - var vertex = context.entity(entity.last()); - continueDrawing(entity, vertex, context); - } - })); - } + var entityLabel = utilDisplayLabel(entity, context); + var fixes = []; + + if (!entity.isClosed()) { + var first = context.entity(entity.first()); + if (first.tags.noexit !== 'yes') { + fixes.push(new validationIssueFix({ + icon: 'iD-operation-continue-left', + title: t('issues.fix.continue_from_start.title'), + entityIds: [entity.first()], + onClick: function() { + var vertex = context.entity(entity.first()); + continueDrawing(entity, vertex, context); + } + })); } + var last = context.entity(entity.last()); + if (last.tags.noexit !== 'yes') { + fixes.push(new validationIssueFix({ + icon: 'iD-operation-continue', + title: t('issues.fix.continue_from_end.title'), + entityIds: [entity.last()], + onClick: function() { + var vertex = context.entity(entity.last()); + continueDrawing(entity, vertex, context); + } + })); + } + } + if (!operationDelete([entity.id], context).disabled()) { fixes.push(new validationIssueFix({ icon: 'iD-operation-delete', title: t('issues.fix.delete_feature.title'), entityIds: [entity.id], onClick: function() { var id = this.issue.entities[0].id; - operationDelete([id], context)(); + var operation = operationDelete([id], context); + if (!operation.disabled()) { + operation(); + } } })); - - issues.push(new validationIssue({ - type: type, - severity: 'warning', - message: t('issues.disconnected_way.highway.message', { highway: entityLabel }), - tooltip: t('issues.disconnected_way.highway.tip'), - entities: [entity], - fixes: fixes - })); } - return issues; + return [new validationIssue({ + type: type, + severity: 'warning', + message: t('issues.disconnected_way.highway.message', { highway: entityLabel }), + tooltip: t('issues.disconnected_way.highway.tip'), + entities: [entity], + fixes: fixes + })]; function continueDrawing(way, vertex) { diff --git a/modules/validations/generic_name.js b/modules/validations/generic_name.js index d8232650a..85f303827 100644 --- a/modules/validations/generic_name.js +++ b/modules/validations/generic_name.js @@ -50,35 +50,32 @@ export function validationGenericName() { var validation = function(entity, context) { - var issues = []; var generic = isGenericName(entity); - if (generic) { - var preset = utilPreset(entity, context); - issues.push(new validationIssue({ - type: type, - severity: 'warning', - message: t('issues.generic_name.message', {feature: preset.name(), name: generic}), - tooltip: t('issues.generic_name.tip'), - entities: [entity], - fixes: [ - new validationIssueFix({ - icon: 'iD-operation-delete', - title: t('issues.fix.remove_generic_name.title'), - onClick: function() { - var entity = this.issue.entities[0]; - var tags = _clone(entity.tags); - delete tags.name; - context.perform( - actionChangeTags(entity.id, tags), - t('issues.fix.remove_generic_name.annotation') - ); - } - }) - ] - })); - } + if (!generic) return []; - return issues; + var preset = utilPreset(entity, context); + return [new validationIssue({ + type: type, + severity: 'warning', + message: t('issues.generic_name.message', {feature: preset.name(), name: generic}), + tooltip: t('issues.generic_name.tip'), + entities: [entity], + fixes: [ + new validationIssueFix({ + icon: 'iD-operation-delete', + title: t('issues.fix.remove_generic_name.title'), + onClick: function() { + var entity = this.issue.entities[0]; + var tags = _clone(entity.tags); + delete tags.name; + context.perform( + actionChangeTags(entity.id, tags), + t('issues.fix.remove_generic_name.annotation') + ); + } + }) + ] + })]; }; validation.type = type; diff --git a/modules/validations/many_deletions.js b/modules/validations/many_deletions.js index 676ce73a5..e934e0d5c 100644 --- a/modules/validations/many_deletions.js +++ b/modules/validations/many_deletions.js @@ -9,7 +9,6 @@ export function validationManyDeletions() { var type = 'many_deletions'; var validation = function(changes, context) { - var issues = []; var points = 0, lines = 0, areas = 0, relations = 0; var base = context.history().base(); var geometry; @@ -36,7 +35,7 @@ export function validationManyDeletions() { if (relations > 0) { messageType += '-relations'; } - issues.push(new validationIssue({ + return [new validationIssue({ type: type, severity: 'warning', message: t( @@ -45,10 +44,10 @@ export function validationManyDeletions() { ), tooltip: t('issues.many_deletions.tip'), hash: [points, lines, areas, relations].join() - })); + })]; } - return issues; + return []; }; validation.type = type; diff --git a/modules/validations/missing_tag.js b/modules/validations/missing_tag.js index a3464a9e9..6083d4e39 100644 --- a/modules/validations/missing_tag.js +++ b/modules/validations/missing_tag.js @@ -55,44 +55,43 @@ export function validationMissingTag() { messageObj.feature = utilDisplayLabel(entity, context); - var issues = []; + var fixes = [ + new validationIssueFix({ + icon: 'iD-icon-search', + title: t('issues.fix.select_preset.title'), + onClick: function() { + context.ui().sidebar.showPresetList(); + } + }) + ]; - var deleteFixOnClick = function() { - var id = this.issue.entities[0].id; - operationDelete([id], context)(); - }; - var canDelete = true; - - if (entity.type === 'relation' && - !entity.members.every(function(member) { return context.hasEntity(member.id); })) { - deleteFixOnClick = null; - canDelete = false; + var canDelete = false; + if (!operationDelete([entity.id], context).disabled()) { + canDelete = true; + fixes.push( + new validationIssueFix({ + icon: 'iD-operation-delete', + title: t('issues.fix.delete_feature.title'), + onClick: function() { + var id = this.issue.entities[0].id; + var operation = operationDelete([id], context); + if (!operation.disabled()) { + operation(); + } + } + }) + ); } - issues.push(new validationIssue({ + return [new validationIssue({ type: type, // error if created or modified and is deletable, else warning severity: (!entity.version || entity.v) && canDelete ? 'error' : 'warning', message: t('issues.missing_tag.' + missingTagType + '.message', messageObj), tooltip: t('issues.missing_tag.tip'), entities: [entity], - fixes: [ - new validationIssueFix({ - icon: 'iD-icon-search', - title: t('issues.fix.select_preset.title'), - onClick: function() { - context.ui().sidebar.showPresetList(); - } - }), - new validationIssueFix({ - icon: 'iD-operation-delete', - title: t('issues.fix.delete_feature.title'), - onClick: deleteFixOnClick - }) - ] - })); - - return issues; + fixes: fixes + })]; }; validation.type = type; diff --git a/modules/validations/old_multipolygon.js b/modules/validations/old_multipolygon.js index 67c690e59..670777025 100644 --- a/modules/validations/old_multipolygon.js +++ b/modules/validations/old_multipolygon.js @@ -11,7 +11,6 @@ export function validationOldMultipolygon() { var validation = function(entity, context) { - var issues = []; var graph = context.graph(); var multipolygon, outerWay; @@ -22,38 +21,36 @@ export function validationOldMultipolygon() { multipolygon = osmIsOldMultipolygonOuterMember(entity, graph); outerWay = entity; } else { - return issues; + return []; } - if (multipolygon && outerWay) { - var multipolygonLabel = utilDisplayLabel(multipolygon, context); - issues.push(new validationIssue({ - type: type, - severity: 'warning', - message: t('issues.old_multipolygon.message', { multipolygon: multipolygonLabel }), - tooltip: t('issues.old_multipolygon.tip'), - entities: [outerWay, multipolygon], - fixes: [ - new validationIssueFix({ - title: t('issues.fix.move_tags.title'), - onClick: function() { - var outerWay = this.issue.entities[0]; - var multipolygon = this.issue.entities[1]; - context.perform( - function(graph) { - multipolygon = multipolygon.mergeTags(outerWay.tags); - graph = graph.replace(multipolygon); - graph = actionChangeTags(outerWay.id, {})(graph); - return graph; - }, - t('issues.fix.move_tags.annotation') - ); - } - }) - ] - })); - } - return issues; + if (!multipolygon || !outerWay) return []; + var multipolygonLabel = utilDisplayLabel(multipolygon, context); + return [new validationIssue({ + type: type, + severity: 'warning', + message: t('issues.old_multipolygon.message', { multipolygon: multipolygonLabel }), + tooltip: t('issues.old_multipolygon.tip'), + entities: [outerWay, multipolygon], + fixes: [ + new validationIssueFix({ + title: t('issues.fix.move_tags.title'), + onClick: function() { + var outerWay = this.issue.entities[0]; + var multipolygon = this.issue.entities[1]; + context.perform( + function(graph) { + multipolygon = multipolygon.mergeTags(outerWay.tags); + graph = graph.replace(multipolygon); + graph = actionChangeTags(outerWay.id, {})(graph); + return graph; + }, + t('issues.fix.move_tags.annotation') + ); + } + }) + ] + })]; }; validation.type = type; diff --git a/modules/validations/unknown_road.js b/modules/validations/unknown_road.js index c5db4bf66..77a67feb8 100644 --- a/modules/validations/unknown_road.js +++ b/modules/validations/unknown_road.js @@ -12,6 +12,32 @@ export function validationUnknownRoad() { if (entity.type !== 'way' || entity.tags.highway !== 'road') return []; + var fixes = [ + new validationIssueFix({ + icon: 'iD-icon-search', + title: t('issues.fix.select_road_type.title'), + onClick: function() { + context.ui().sidebar.showPresetList(); + } + }) + ]; + + if (!operationDelete([entity.id], context).disabled()) { + fixes.push( + new validationIssueFix({ + icon: 'iD-operation-delete', + title: t('issues.fix.delete_feature.title'), + onClick: function() { + var id = this.issue.entities[0].id; + var operation = operationDelete([id], context); + if (!operation.disabled()) { + operation(); + } + } + }) + ); + } + return [new validationIssue({ type: type, severity: 'warning', @@ -20,23 +46,7 @@ export function validationUnknownRoad() { }), tooltip: t('issues.unknown_road.tip'), entities: [entity], - fixes: [ - new validationIssueFix({ - icon: 'iD-icon-search', - title: t('issues.fix.select_road_type.title'), - onClick: function() { - context.ui().sidebar.showPresetList(); - } - }), - new validationIssueFix({ - icon: 'iD-operation-delete', - title: t('issues.fix.delete_feature.title'), - onClick: function() { - var id = this.issue.entities[0].id; - operationDelete([id], context)(); - } - }) - ] + fixes: fixes })]; };