From 1ed73b6531e29bfd3aa6956b1b4ebd1384eb287a Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Thu, 6 Jun 2019 16:27:31 -0400 Subject: [PATCH] Convert validation rules to validate against a specific graph, not always the current graph (re: #6459) --- modules/core/validator.js | 14 +- modules/ui/commit_warnings.js | 2 +- modules/ui/entity_issues.js | 6 +- modules/ui/issues.js | 2 +- modules/validations/almost_junction.js | 14 +- modules/validations/close_nodes.js | 430 +++++++++---------- modules/validations/crossing_ways.js | 35 +- modules/validations/disconnected_way.js | 64 ++- modules/validations/fixme_tag.js | 6 +- modules/validations/generic_name.js | 6 +- modules/validations/impossible_oneway.js | 395 +++++++++-------- modules/validations/incompatible_source.js | 4 +- modules/validations/invalid_format.js | 8 +- modules/validations/maprules.js | 3 +- modules/validations/missing_role.js | 24 +- modules/validations/missing_tag.js | 39 +- modules/validations/outdated_tags.js | 28 +- modules/validations/private_data.js | 8 +- modules/validations/tag_suggests_area.js | 14 +- modules/validations/unsquare_way.js | 18 +- test/spec/services/maprules.js | 4 +- test/spec/validations/almost_junction.js | 8 +- test/spec/validations/crossing_ways.js | 4 +- test/spec/validations/disconnected_way.js | 4 +- test/spec/validations/generic_name.js | 4 +- test/spec/validations/incompatible_source.js | 4 +- test/spec/validations/missing_role.js | 4 +- test/spec/validations/missing_tag.js | 4 +- test/spec/validations/outdated_tags.js | 4 +- test/spec/validations/private_data.js | 4 +- test/spec/validations/tag_suggests_area.js | 4 +- 31 files changed, 577 insertions(+), 591 deletions(-) diff --git a/modules/core/validator.js b/modules/core/validator.js index 376f7d5e2..1a60e908b 100644 --- a/modules/core/validator.js +++ b/modules/core/validator.js @@ -29,7 +29,7 @@ export function coreValidator(context) { Object.values(Validations).forEach(function(validation) { if (typeof validation !== 'function') return; - var fn = validation(); + var fn = validation(context); var key = fn.type; _rules[key] = fn; }); @@ -285,9 +285,9 @@ export function coreValidator(context) { // - // Run validation on a single entity + // Run validation on a single entity for the given graph // - function validateEntity(entity) { + function validateEntity(entity, graph) { var entityIssues = []; var ran = {}; @@ -303,7 +303,7 @@ export function coreValidator(context) { return true; } - var detected = fn(entity, context); + var detected = fn(entity, graph); detected.forEach(function(issue) { var hasIgnoreFix = issue.fixes && issue.fixes.length && issue.fixes[issue.fixes.length - 1].type === 'ignore'; if (issue.severity === 'warning' && !hasIgnoreFix) { @@ -407,16 +407,18 @@ export function coreValidator(context) { // function validateEntities(entityIDs) { + var graph = context.graph(); + // clear caches for existing issues related to these entities entityIDs.forEach(uncacheEntityID); // detect new issues and update caches entityIDs.forEach(function(entityID) { - var entity = context.graph().hasEntity(entityID); + var entity = graph.hasEntity(entityID); // don't validate deleted entities if (!entity) return; - var issues = validateEntity(entity); + var issues = validateEntity(entity, graph); issues.forEach(function(issue) { var entityIds = issue.entityIds || []; entityIds.forEach(function(entityId) { diff --git a/modules/ui/commit_warnings.js b/modules/ui/commit_warnings.js index 049cda400..a04f1881f 100644 --- a/modules/ui/commit_warnings.js +++ b/modules/ui/commit_warnings.js @@ -65,7 +65,7 @@ export function uiCommitWarnings(context) { items.selectAll('.issue-message') .text(function(d) { - return d.message(); + return d.message(context); }); items diff --git a/modules/ui/entity_issues.js b/modules/ui/entity_issues.js index a29f6bbf6..391601348 100644 --- a/modules/ui/entity_issues.js +++ b/modules/ui/entity_issues.js @@ -190,7 +190,7 @@ export function uiEntityIssues(context) { containers.selectAll('.issue-message') .text(function(d) { - return d.message(); + return d.message(context); }); // fixes @@ -217,8 +217,8 @@ export function uiEntityIssues(context) { utilHighlightEntities(d.issue.entityIds.concat(d.entityIds), false, context); new Promise(function(resolve, reject) { - d.onClick(resolve, reject); - if (!d.onClick.length) { + d.onClick(context, resolve, reject); + if (d.onClick.length <= 1) { // if the fix doesn't take any completion parameters then consider it resolved resolve(); } diff --git a/modules/ui/issues.js b/modules/ui/issues.js index b98a7e9fa..edfefa816 100644 --- a/modules/ui/issues.js +++ b/modules/ui/issues.js @@ -156,7 +156,7 @@ export function uiIssues(context) { items.selectAll('.issue-message') .text(function(d) { - return d.message(); + return d.message(context); }); diff --git a/modules/validations/almost_junction.js b/modules/validations/almost_junction.js index 31bb1c56b..844674219 100644 --- a/modules/validations/almost_junction.js +++ b/modules/validations/almost_junction.js @@ -11,12 +11,13 @@ import { t } from '../util/locale'; import { utilDisplayLabel } from '../util'; import { osmRoutableHighwayTagValues } from '../osm/tags'; import { validationIssue, validationIssueFix } from '../core/validation'; +import { services } from '../services'; /** * Look for roads that can be connected to other roads with a short extension */ -export function validationAlmostJunction() { +export function validationAlmostJunction(context) { var type = 'almost_junction'; @@ -32,11 +33,10 @@ export function validationAlmostJunction() { } - var validation = function checkAlmostJunction(entity, context) { + var validation = function checkAlmostJunction(entity, graph) { if (!isHighway(entity)) return []; if (entity.isDegenerate()) return []; - var graph = context.graph(); var tree = context.history().tree(); var issues = []; @@ -48,7 +48,7 @@ export function validationAlmostJunction() { var fixes = [new validationIssueFix({ icon: 'iD-icon-abutment', title: t('issues.fix.connect_features.title'), - onClick: function() { + onClick: function(context) { var endNodeId = this.issue.entityIds[1]; var endNode = context.entity(endNodeId); var targetEdge = this.issue.data.edge; @@ -78,7 +78,7 @@ export function validationAlmostJunction() { fixes.push(new validationIssueFix({ icon: 'maki-barrier', title: t('issues.fix.tag_as_disconnected.title'), - onClick: function() { + onClick: function(context) { var nodeID = this.issue.entityIds[1]; context.perform( actionChangeTags(nodeID, { noexit: 'yes' }), @@ -91,7 +91,7 @@ export function validationAlmostJunction() { issues.push(new validationIssue({ type: type, severity: 'warning', - message: function() { + message: function(context) { var entity1 = context.hasEntity(this.entityIds[0]); if (this.entityIds[0] === this.entityIds[2]) { return entity1 ? t('issues.almost_junction.self.message', { @@ -132,7 +132,7 @@ export function validationAlmostJunction() { function isExtendableCandidate(node, way) { // can not accurately test vertices on tiles not downloaded from osm - #5938 - var osm = context.connection(); + var osm = services.osm; if (osm && !osm.isDataLoaded(node.loc)) { return false; } diff --git a/modules/validations/close_nodes.js b/modules/validations/close_nodes.js index f682e8ce7..274f6dcbb 100644 --- a/modules/validations/close_nodes.js +++ b/modules/validations/close_nodes.js @@ -6,7 +6,7 @@ import { osmPathHighwayTagValues } from '../osm/tags'; import { geoMetersToLat, geoMetersToLon, geoSphericalDistance } from '../geo/geo'; import { geoExtent } from '../geo/extent'; -export function validationCloseNodes() { +export function validationCloseNodes(context) { var type = 'close_nodes'; var pointThresholdMeters = 0.2; @@ -17,224 +17,224 @@ export function validationCloseNodes() { var buildingThresholdMeters = 0.05; var pathThresholdMeters = 0.1; - function featureTypeForWay(way, graph) { - - if (way.tags.boundary && way.tags.boundary !== 'no') return 'boundary'; - if (way.tags.indoor && way.tags.indoor !== 'no') return 'indoor'; - if ((way.tags.building && way.tags.building !== 'no') || - (way.tags['building:part'] && way.tags['building:part'] !== 'no')) return 'building'; - if (osmPathHighwayTagValues[way.tags.highway]) return 'path'; - - var parentRelations = graph.parentRelations(way); - for (var i in parentRelations) { - var relation = parentRelations[i]; - - if (relation.tags.type === 'boundary') return 'boundary'; - - if (relation.isMultipolygon()) { - if (relation.tags.indoor && relation.tags.indoor !== 'no') return 'indoor'; - if ((relation.tags.building && relation.tags.building !== 'no') || - (relation.tags['building:part'] && relation.tags['building:part'] !== 'no')) return 'building'; - } - } - - return 'other'; - } - - function shouldCheckWay(way, context) { - - // don't flag issues where merging would create degenerate ways - if (way.nodes.length <= 2 || - (way.isClosed() && way.nodes.length <= 4)) return false; - - var featureType = featureTypeForWay(way, context.graph()); - if (featureType === 'boundary') return false; - - var bbox = way.extent(context.graph()).bbox(); - var hypotenuseMeters = geoSphericalDistance([bbox.minX, bbox.minY], [bbox.maxX, bbox.maxY]); - // don't flag close nodes in very small ways - if (hypotenuseMeters < 1.5) return false; - - return true; - } - - function getIssuesForWay(way, context) { - if (!shouldCheckWay(way, context)) return []; - - var issues = [], - nodes = context.graph().childNodes(way); - for (var i = 0; i < nodes.length - 1; i++) { - var node1 = nodes[i]; - var node2 = nodes[i+1]; - - var issue = getWayIssueIfAny(node1, node2, way, context); - if (issue) issues.push(issue); - } - return issues; - } - - function getIssuesForVertex(node, parentWays, context) { - var issues = []; - - function checkForCloseness(node1, node2, way) { - var issue = getWayIssueIfAny(node1, node2, way, context); - if (issue) issues.push(issue); - } - - for (var i = 0; i < parentWays.length; i++) { - var parentWay = parentWays[i]; - - if (!shouldCheckWay(parentWay, context)) continue; - - var lastIndex = parentWay.nodes.length - 1; - for (var j = 0; j < parentWay.nodes.length; j++) { - if (j !== 0) { - if (parentWay.nodes[j-1] === node.id) { - checkForCloseness(node, context.entity(parentWay.nodes[j]), parentWay); - } - } - if (j !== lastIndex) { - if (parentWay.nodes[j+1] === node.id) { - checkForCloseness(context.entity(parentWay.nodes[j]), node, parentWay); - } - } - } - } - return issues; - } - - function getIssuesForDetachedPoint(node, context) { - - var issues = []; - - var lon = node.loc[0]; - var lat = node.loc[1]; - var lon_range = geoMetersToLon(pointThresholdMeters, lat) / 2; - var lat_range = geoMetersToLat(pointThresholdMeters) / 2; - var queryExtent = geoExtent([ - [lon - lon_range, lat - lat_range], - [lon + lon_range, lat + lat_range] - ]); - - var intersected = context.history().tree().intersects(queryExtent, context.graph()); - for (var j = 0; j < intersected.length; j++) { - var nearby = intersected[j]; - - if (nearby.id === node.id) continue; - if (nearby.type !== 'node' || nearby.geometry(context.graph()) !== 'point') continue; - - if (nearby.loc === node.loc || - geoSphericalDistance(node.loc, nearby.loc) < pointThresholdMeters) { - - issues.push(new validationIssue({ - type: type, - severity: 'warning', - message: function() { - var entity = context.hasEntity(this.entityIds[0]), - entity2 = context.hasEntity(this.entityIds[1]); - return (entity && entity2) ? t('issues.close_nodes.detached.message', { - feature: utilDisplayLabel(entity, context), - feature2: utilDisplayLabel(entity2, context) - }) : ''; - }, - reference: showReference, - entityIds: [node.id, nearby.id], - fixes: [ - new validationIssueFix({ - icon: 'iD-operation-disconnect', - title: t('issues.fix.move_points_apart.title') - }) - ] - })); - } - } - - return issues; - - function showReference(selection) { - var referenceText = t('issues.close_nodes.detached.reference'); - selection.selectAll('.issue-reference') - .data([0]) - .enter() - .append('div') - .attr('class', 'issue-reference') - .text(referenceText); - } - } - - function getIssuesForNode(node, context) { - var parentWays = context.graph().parentWays(node); - if (parentWays.length) { - return getIssuesForVertex(node, parentWays, context); - } else { - return getIssuesForDetachedPoint(node, context); - } - } - - function getWayIssueIfAny(node1, node2, way, context) { - if (node1.id === node2.id || - (node1.hasInterestingTags() && node2.hasInterestingTags())) { - return null; - } - - if (node1.loc !== node2.loc) { - - var featureType = featureTypeForWay(way, context.graph()); - var threshold = defaultWayThresholdMeters; - if (featureType === 'indoor') threshold = indoorThresholdMeters; - else if (featureType === 'building') threshold = buildingThresholdMeters; - else if (featureType === 'path') threshold = pathThresholdMeters; - - var distance = geoSphericalDistance(node1.loc, node2.loc); - if (distance > threshold) return null; - } - - return new validationIssue({ - type: type, - severity: 'warning', - message: function() { - var entity = context.hasEntity(this.entityIds[0]); - return entity ? t('issues.close_nodes.message', { way: utilDisplayLabel(entity, context) }) : ''; - }, - reference: showReference, - entityIds: [way.id, node1.id, node2.id], - loc: node1.loc, - fixes: [ - new validationIssueFix({ - icon: 'iD-icon-plus', - title: t('issues.fix.merge_points.title'), - onClick: function() { - var entityIds = this.issue.entityIds; - var action = actionMergeNodes([entityIds[1], entityIds[2]]); - context.perform(action, t('issues.fix.merge_close_vertices.annotation')); - } - }), - new validationIssueFix({ - icon: 'iD-operation-disconnect', - title: t('issues.fix.move_points_apart.title') - }) - ] - }); - - function showReference(selection) { - var referenceText = t('issues.close_nodes.reference'); - selection.selectAll('.issue-reference') - .data([0]) - .enter() - .append('div') - .attr('class', 'issue-reference') - .text(referenceText); - } - } - - - var validation = function(entity, context) { + var validation = function(entity, graph) { if (entity.type === 'node') { - return getIssuesForNode(entity, context); + return getIssuesForNode(entity); } else if (entity.type === 'way') { - return getIssuesForWay(entity, context); + return getIssuesForWay(entity); } return []; + + function featureTypeForWay(way) { + + if (way.tags.boundary && way.tags.boundary !== 'no') return 'boundary'; + if (way.tags.indoor && way.tags.indoor !== 'no') return 'indoor'; + if ((way.tags.building && way.tags.building !== 'no') || + (way.tags['building:part'] && way.tags['building:part'] !== 'no')) return 'building'; + if (osmPathHighwayTagValues[way.tags.highway]) return 'path'; + + var parentRelations = graph.parentRelations(way); + for (var i in parentRelations) { + var relation = parentRelations[i]; + + if (relation.tags.type === 'boundary') return 'boundary'; + + if (relation.isMultipolygon()) { + if (relation.tags.indoor && relation.tags.indoor !== 'no') return 'indoor'; + if ((relation.tags.building && relation.tags.building !== 'no') || + (relation.tags['building:part'] && relation.tags['building:part'] !== 'no')) return 'building'; + } + } + + return 'other'; + } + + function shouldCheckWay(way) { + + // don't flag issues where merging would create degenerate ways + if (way.nodes.length <= 2 || + (way.isClosed() && way.nodes.length <= 4)) return false; + + var featureType = featureTypeForWay(way); + if (featureType === 'boundary') return false; + + var bbox = way.extent(graph).bbox(); + var hypotenuseMeters = geoSphericalDistance([bbox.minX, bbox.minY], [bbox.maxX, bbox.maxY]); + // don't flag close nodes in very small ways + if (hypotenuseMeters < 1.5) return false; + + return true; + } + + function getIssuesForWay(way) { + if (!shouldCheckWay(way)) return []; + + var issues = [], + nodes = graph.childNodes(way); + for (var i = 0; i < nodes.length - 1; i++) { + var node1 = nodes[i]; + var node2 = nodes[i+1]; + + var issue = getWayIssueIfAny(node1, node2, way); + if (issue) issues.push(issue); + } + return issues; + } + + function getIssuesForVertex(node, parentWays) { + var issues = []; + + function checkForCloseness(node1, node2, way) { + var issue = getWayIssueIfAny(node1, node2, way); + if (issue) issues.push(issue); + } + + for (var i = 0; i < parentWays.length; i++) { + var parentWay = parentWays[i]; + + if (!shouldCheckWay(parentWay)) continue; + + var lastIndex = parentWay.nodes.length - 1; + for (var j = 0; j < parentWay.nodes.length; j++) { + if (j !== 0) { + if (parentWay.nodes[j-1] === node.id) { + checkForCloseness(node, graph.entity(parentWay.nodes[j]), parentWay); + } + } + if (j !== lastIndex) { + if (parentWay.nodes[j+1] === node.id) { + checkForCloseness(graph.entity(parentWay.nodes[j]), node, parentWay); + } + } + } + } + return issues; + } + + function getIssuesForDetachedPoint(node) { + + var issues = []; + + var lon = node.loc[0]; + var lat = node.loc[1]; + var lon_range = geoMetersToLon(pointThresholdMeters, lat) / 2; + var lat_range = geoMetersToLat(pointThresholdMeters) / 2; + var queryExtent = geoExtent([ + [lon - lon_range, lat - lat_range], + [lon + lon_range, lat + lat_range] + ]); + + var intersected = context.history().tree().intersects(queryExtent, graph); + for (var j = 0; j < intersected.length; j++) { + var nearby = intersected[j]; + + if (nearby.id === node.id) continue; + if (nearby.type !== 'node' || nearby.geometry(graph) !== 'point') continue; + + if (nearby.loc === node.loc || + geoSphericalDistance(node.loc, nearby.loc) < pointThresholdMeters) { + + issues.push(new validationIssue({ + type: type, + severity: 'warning', + message: function(context) { + var entity = context.hasEntity(this.entityIds[0]), + entity2 = context.hasEntity(this.entityIds[1]); + return (entity && entity2) ? t('issues.close_nodes.detached.message', { + feature: utilDisplayLabel(entity, context), + feature2: utilDisplayLabel(entity2, context) + }) : ''; + }, + reference: showReference, + entityIds: [node.id, nearby.id], + fixes: [ + new validationIssueFix({ + icon: 'iD-operation-disconnect', + title: t('issues.fix.move_points_apart.title') + }) + ] + })); + } + } + + return issues; + + function showReference(selection) { + var referenceText = t('issues.close_nodes.detached.reference'); + selection.selectAll('.issue-reference') + .data([0]) + .enter() + .append('div') + .attr('class', 'issue-reference') + .text(referenceText); + } + } + + function getIssuesForNode(node) { + var parentWays = graph.parentWays(node); + if (parentWays.length) { + return getIssuesForVertex(node, parentWays); + } else { + return getIssuesForDetachedPoint(node); + } + } + + function getWayIssueIfAny(node1, node2, way) { + if (node1.id === node2.id || + (node1.hasInterestingTags() && node2.hasInterestingTags())) { + return null; + } + + if (node1.loc !== node2.loc) { + + var featureType = featureTypeForWay(way, graph); + var threshold = defaultWayThresholdMeters; + if (featureType === 'indoor') threshold = indoorThresholdMeters; + else if (featureType === 'building') threshold = buildingThresholdMeters; + else if (featureType === 'path') threshold = pathThresholdMeters; + + var distance = geoSphericalDistance(node1.loc, node2.loc); + if (distance > threshold) return null; + } + + return new validationIssue({ + type: type, + severity: 'warning', + message: function(context) { + var entity = context.hasEntity(this.entityIds[0]); + return entity ? t('issues.close_nodes.message', { way: utilDisplayLabel(entity, context) }) : ''; + }, + reference: showReference, + entityIds: [way.id, node1.id, node2.id], + loc: node1.loc, + fixes: [ + new validationIssueFix({ + icon: 'iD-icon-plus', + title: t('issues.fix.merge_points.title'), + onClick: function(context) { + var entityIds = this.issue.entityIds; + var action = actionMergeNodes([entityIds[1], entityIds[2]]); + context.perform(action, t('issues.fix.merge_close_vertices.annotation')); + } + }), + new validationIssueFix({ + icon: 'iD-operation-disconnect', + title: t('issues.fix.move_points_apart.title') + }) + ] + }); + + function showReference(selection) { + var referenceText = t('issues.close_nodes.reference'); + selection.selectAll('.issue-reference') + .data([0]) + .enter() + .append('div') + .attr('class', 'issue-reference') + .text(referenceText); + } + } + }; diff --git a/modules/validations/crossing_ways.js b/modules/validations/crossing_ways.js index 0e04dd0a1..54b4410f1 100644 --- a/modules/validations/crossing_ways.js +++ b/modules/validations/crossing_ways.js @@ -9,7 +9,7 @@ import { utilDisplayLabel } from '../util'; import { validationIssue, validationIssueFix } from '../core/validation'; -export function validationCrossingWays() { +export function validationCrossingWays(context) { var type = 'crossing_ways'; /* @@ -305,7 +305,7 @@ export function validationCrossingWays() { } - function waysToCheck(entity, context) { + function waysToCheck(entity, graph) { if (!getFeatureTypeForTags(entity.tags)) { return []; } @@ -318,8 +318,8 @@ export function validationCrossingWays() { return entity.members.reduce(function(array, member) { if (member.type === 'way' && //(member.role === 'outer' || member.role === 'inner') && - context.hasEntity(member.id)) { - var entity = context.entity(member.id); + graph.hasEntity(member.id)) { + var entity = graph.entity(member.id); array.push(entity); } return array; @@ -329,11 +329,11 @@ export function validationCrossingWays() { } - var validation = function checkCrossingWays(entity, context) { - var graph = context.graph(); + var validation = function checkCrossingWays(entity, graph) { + var tree = context.history().tree(); - var ways = waysToCheck(entity, context); + var ways = waysToCheck(entity, graph); var issues = []; // declare these here to reduce garbage collection @@ -344,7 +344,7 @@ export function validationCrossingWays() { for (crossingIndex in crossings) { crossing = crossings[crossingIndex]; var way2 = crossing.ways[1]; - issue = createIssue(crossing, context); + issue = createIssue(crossing, graph); // cache the issues for each way _issueCache[way.id][way2.id].push(issue); _issueCache[way2.id][way.id].push(issue); @@ -357,8 +357,7 @@ export function validationCrossingWays() { }; - function createIssue(crossing, context) { - var graph = context.graph(); + function createIssue(crossing, graph) { // use the entities with the tags that define the feature type var entities = crossing.ways.sort(function(entity1, entity2) { @@ -405,7 +404,7 @@ export function validationCrossingWays() { var fixes = []; if (connectionTags) { - fixes.push(makeConnectWaysFix(context)); + fixes.push(makeConnectWaysFix()); } var useFixIcon = 'iD-icon-layers'; @@ -425,8 +424,8 @@ export function validationCrossingWays() { useFixID = 'use_different_layers'; } if (useFixID === 'use_different_layers') { - fixes.push(makeChangeLayerFix('higher', context)); - fixes.push(makeChangeLayerFix('lower', context)); + fixes.push(makeChangeLayerFix('higher')); + fixes.push(makeChangeLayerFix('lower')); } else { fixes.push(new validationIssueFix({ icon: useFixIcon, @@ -441,7 +440,7 @@ export function validationCrossingWays() { return new validationIssue({ type: type, severity: 'warning', - message: function() { + message: function(context) { var entity1 = context.hasEntity(this.entityIds[0]), entity2 = context.hasEntity(this.entityIds[1]); return (entity1 && entity2) ? t('issues.crossing_ways.message', { @@ -477,11 +476,11 @@ export function validationCrossingWays() { } } - function makeConnectWaysFix(context) { + function makeConnectWaysFix() { return new validationIssueFix({ icon: 'iD-icon-crossing', title: t('issues.fix.connect_features.title'), - onClick: function() { + onClick: function(context) { var loc = this.issue.loc; var connectionTags = this.issue.data.connectionTags; var edges = this.issue.data.edges; @@ -520,11 +519,11 @@ export function validationCrossingWays() { }); } - function makeChangeLayerFix(higherOrLower, context) { + function makeChangeLayerFix(higherOrLower) { return new validationIssueFix({ icon: 'iD-icon-' + (higherOrLower === 'higher' ? 'up' : 'down'), title: t('issues.fix.tag_this_as_' + higherOrLower + '.title'), - onClick: function() { + onClick: function(context) { var mode = context.mode(); if (!mode || mode.id !== 'select') return; diff --git a/modules/validations/disconnected_way.js b/modules/validations/disconnected_way.js index 5d064a80a..b16323f25 100644 --- a/modules/validations/disconnected_way.js +++ b/modules/validations/disconnected_way.js @@ -4,7 +4,7 @@ import { operationDelete } from '../operations/delete'; import { utilDisplayLabel } from '../util'; import { osmRoutableHighwayTagValues } from '../osm/tags'; import { validationIssue, validationIssueFix } from '../core/validation'; - +import { services } from '../services'; export function validationDisconnectedWay() { var type = 'disconnected_way'; @@ -13,8 +13,7 @@ export function validationDisconnectedWay() { return osmRoutableHighwayTagValues[entity.tags.highway]; } - var validation = function checkDisconnectedWay(entity, context) { - var graph = context.graph(); + var validation = function checkDisconnectedWay(entity, graph) { var routingIslandWays = routingIslandForEntity(entity); if (!routingIslandWays) return []; @@ -29,13 +28,13 @@ export function validationDisconnectedWay() { var firstID = entity.first(); var lastID = entity.last(); - var first = context.entity(firstID); + var first = graph.entity(firstID); if (first.tags.noexit !== 'yes') { fixes.push(new validationIssueFix({ icon: 'iD-operation-continue-left', title: t('issues.fix.continue_from_start.title'), entityIds: [firstID], - onClick: function() { + onClick: function(context) { var wayId = this.issue.entityIds[0]; var way = context.entity(wayId); var vertexId = this.entityIds[0]; @@ -44,13 +43,13 @@ export function validationDisconnectedWay() { } })); } - var last = context.entity(lastID); + var last = graph.entity(lastID); if (last.tags.noexit !== 'yes') { fixes.push(new validationIssueFix({ icon: 'iD-operation-continue', title: t('issues.fix.continue_from_end.title'), entityIds: [lastID], - onClick: function() { + onClick: function(context) { var wayId = this.issue.entityIds[0]; var way = context.entity(wayId); var vertexId = this.entityIds[0]; @@ -66,20 +65,18 @@ export function validationDisconnectedWay() { })); } - 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.entityIds[0]; - var operation = operationDelete([id], context); - if (!operation.disabled()) { - operation(); - } + fixes.push(new validationIssueFix({ + icon: 'iD-operation-delete', + title: t('issues.fix.delete_feature.title'), + entityIds: [entity.id], + onClick: function(context) { + var id = this.issue.entityIds[0]; + var operation = operationDelete([id], context); + if (!operation.disabled()) { + operation(); } - })); - } + } + })); } else { fixes.push(new validationIssueFix({ title: t('issues.fix.connect_features.title') @@ -89,7 +86,7 @@ export function validationDisconnectedWay() { return [new validationIssue({ type: type, severity: 'warning', - message: function() { + message: function(context) { if (this.entityIds.length === 1) { var entity = context.hasEntity(this.entityIds[0]); return entity ? t('issues.disconnected_way.highway.message', { highway: utilDisplayLabel(entity, context) }) : ''; @@ -145,7 +142,7 @@ export function validationDisconnectedWay() { function isConnectedVertex(vertex, routingIslandWays) { // assume ways overlapping unloaded tiles are connected to the wider road network - #5938 - var osm = context.connection(); + var osm = services.osm; if (osm && !osm.isDataLoaded(vertex.loc)) return true; // entrances are considered connected @@ -187,19 +184,20 @@ export function validationDisconnectedWay() { }); } - function continueDrawing(way, vertex) { - // make sure the vertex is actually visible and editable - var map = context.map(); - if (!map.editable() || !map.trimmedExtent().contains(vertex.loc)) { - map.zoomToEase(vertex); - } - - context.enter( - modeDrawLine(context, way.id, context.graph(), context.graph(), 'line', way.affix(vertex.id), true) - ); - } }; + function continueDrawing(way, vertex, context) { + // make sure the vertex is actually visible and editable + var map = context.map(); + if (!map.editable() || !map.trimmedExtent().contains(vertex.loc)) { + map.zoomToEase(vertex); + } + + context.enter( + modeDrawLine(context, way.id, context.graph(), context.graph(), 'line', way.affix(vertex.id), true) + ); + } + validation.type = type; diff --git a/modules/validations/fixme_tag.js b/modules/validations/fixme_tag.js index ea7be0039..8033dbcc8 100644 --- a/modules/validations/fixme_tag.js +++ b/modules/validations/fixme_tag.js @@ -3,11 +3,11 @@ import { utilDisplayLabel } from '../util'; import { validationIssue } from '../core/validation'; -export function validationFixmeTag() { +export function validationFixmeTag(context) { var type = 'fixme_tag'; - var validation = function checkFixmeTag(entity, context) { + var validation = function checkFixmeTag(entity) { if (!entity.tags.fixme) return []; @@ -23,7 +23,7 @@ export function validationFixmeTag() { return [new validationIssue({ type: type, severity: 'warning', - message: function() { + message: function(context) { var entity = context.hasEntity(this.entityIds[0]); return entity ? t('issues.fixme_tag.message', { feature: utilDisplayLabel(entity, context) }) : ''; }, diff --git a/modules/validations/generic_name.js b/modules/validations/generic_name.js index 8d31703cf..6f27b1bbc 100644 --- a/modules/validations/generic_name.js +++ b/modules/validations/generic_name.js @@ -49,14 +49,14 @@ export function validationGenericName() { } - var validation = function checkGenericName(entity, context) { + var validation = function checkGenericName(entity) { var generic = isGenericName(entity); if (!generic) return []; return [new validationIssue({ type: type, severity: 'warning', - message: function() { + message: function(context) { var entity = context.hasEntity(this.entityIds[0]); if (!entity) return ''; var preset = utilPreset(entity, context); @@ -69,7 +69,7 @@ export function validationGenericName() { new validationIssueFix({ icon: 'iD-operation-delete', title: t('issues.fix.remove_generic_name.title'), - onClick: function() { + onClick: function(context) { var entityId = this.issue.entityIds[0]; var entity = context.entity(entityId); var tags = Object.assign({}, entity.tags); // shallow copy diff --git a/modules/validations/impossible_oneway.js b/modules/validations/impossible_oneway.js index 7d49ef87e..007d3f437 100644 --- a/modules/validations/impossible_oneway.js +++ b/modules/validations/impossible_oneway.js @@ -4,30 +4,212 @@ import { actionReverse } from '../actions/reverse'; import { utilDisplayLabel } from '../util'; import { osmFlowingWaterwayTagValues, osmOneWayTags, osmRoutableHighwayTagValues } from '../osm/tags'; import { validationIssue, validationIssueFix } from '../core/validation'; - +import { services } from '../services'; export function validationImpossibleOneway() { var type = 'impossible_oneway'; - function typeForWay(way, context) { - if (way.geometry(context.graph()) !== 'line') return null; + var validation = function checkImpossibleOneway(entity, graph) { - if (osmRoutableHighwayTagValues[way.tags.highway]) return 'highway'; - if (osmFlowingWaterwayTagValues[way.tags.waterway]) return 'waterway'; - return null; - } + if (entity.type !== 'way' || entity.geometry(graph) !== 'line') return []; - function isOneway(way) { - if (way.tags.oneway === 'yes') return true; - if (way.tags.oneway) return false; + if (entity.isClosed()) return []; - for (var key in way.tags) { - if (osmOneWayTags[key] && osmOneWayTags[key][way.tags[key]]) { - return true; + if (!typeForWay(entity)) return []; + + if (!isOneway(entity)) return []; + + var firstIssues = issuesForNode(entity, entity.first()); + var lastIssues = issuesForNode(entity, entity.last()); + + return firstIssues.concat(lastIssues); + + function typeForWay(way) { + if (way.geometry(graph) !== 'line') return null; + + if (osmRoutableHighwayTagValues[way.tags.highway]) return 'highway'; + if (osmFlowingWaterwayTagValues[way.tags.waterway]) return 'waterway'; + return null; + } + + function isOneway(way) { + if (way.tags.oneway === 'yes') return true; + if (way.tags.oneway) return false; + + for (var key in way.tags) { + if (osmOneWayTags[key] && osmOneWayTags[key][way.tags[key]]) { + return true; + } + } + return false; + } + + function nodeOccursMoreThanOnce(way, nodeID) { + var occurences = 0; + for (var index in way.nodes) { + if (way.nodes[index] === nodeID) { + occurences += 1; + if (occurences > 1) return true; + } + } + return false; + } + + function isConnectedViaOtherTypes(way, node) { + + var wayType = typeForWay(way); + + if (wayType === 'highway') { + // entrances are considered connected + if (node.tags.entrance && node.tags.entrance !== 'no') return true; + if (node.tags.amenity === 'parking_entrance') return true; + } else if (wayType === 'waterway') { + if (node.id === way.first()) { + // multiple waterways may start at the same spring + if (node.tags.natural === 'spring') return true; + } else { + // multiple waterways may end at the same drain + if (node.tags.manhole === 'drain') return true; + } + } + + return graph.parentWays(node).some(function(parentWay) { + if (parentWay.id === way.id) return false; + + if (wayType === 'highway') { + + // allow connections to highway areas + if (parentWay.geometry(graph) === 'area' && + osmRoutableHighwayTagValues[parentWay.tags.highway]) return true; + + // count connections to ferry routes as connected + if (parentWay.tags.route === 'ferry') return true; + + return graph.parentRelations(parentWay).some(function(parentRelation) { + if (parentRelation.tags.type === 'route' && + parentRelation.tags.route === 'ferry') return true; + + // allow connections to highway multipolygons + return parentRelation.isMultipolygon() && osmRoutableHighwayTagValues[parentRelation.tags.highway]; + }); + } else if (wayType === 'waterway') { + // multiple waterways may start or end at a water body at the same node + if (parentWay.tags.natural === 'water' || + parentWay.tags.natural === 'coastline') return true; + } + return false; + }); + } + + function issuesForNode(way, nodeID) { + + var isFirst = nodeID === way.first(); + + var wayType = typeForWay(way); + + // ignore if this way is self-connected at this node + if (nodeOccursMoreThanOnce(way, nodeID)) return []; + + var osm = services.osm; + if (!osm) return []; + + var node = graph.hasEntity(nodeID); + + // ignore if this node or its tile are unloaded + if (!node || !osm.isDataLoaded(node.loc)) return []; + + if (isConnectedViaOtherTypes(way, node)) return []; + + var attachedWaysOfSameType = graph.parentWays(node).filter(function(parentWay) { + if (parentWay.id === way.id) return false; + return typeForWay(parentWay) === wayType; + }); + + // assume it's okay for waterways to start or end disconnected for now + if (wayType === 'waterway' && attachedWaysOfSameType.length === 0) return []; + + var attachedOneways = attachedWaysOfSameType.filter(function(attachedWay) { + return isOneway(attachedWay); + }); + + // ignore if the way is connected to some non-oneway features + if (attachedOneways.length < attachedWaysOfSameType.length) return []; + + if (attachedOneways.length) { + var connectedEndpointsOkay = attachedOneways.some(function(attachedOneway) { + if ((isFirst ? attachedOneway.first() : attachedOneway.last()) !== nodeID) return true; + if (nodeOccursMoreThanOnce(attachedOneway, nodeID)) return true; + return false; + }); + if (connectedEndpointsOkay) return []; + } + + var fixes = []; + + if (attachedOneways.length) { + fixes.push(new validationIssueFix({ + icon: 'iD-operation-reverse', + title: t('issues.fix.reverse_feature.title'), + entityIds: [way.id], + onClick: function(context) { + var id = this.issue.entityIds[0]; + context.perform(actionReverse(id), t('operations.reverse.annotation')); + } + })); + } + if (node.tags.noexit !== 'yes') { + fixes.push(new validationIssueFix({ + icon: 'iD-operation-continue' + (isFirst ? '-left' : ''), + title: t('issues.fix.continue_from_' + (isFirst ? 'start' : 'end') + '.title'), + onClick: function(context) { + var entityID = this.issue.entityIds[0]; + var vertexID = this.issue.entityIds[1]; + var way = context.entity(entityID); + var vertex = context.entity(vertexID); + continueDrawing(way, vertex, context); + } + })); + } + + var placement = isFirst ? 'start' : 'end', + messageID = wayType + '.', + referenceID = wayType + '.'; + + if (wayType === 'waterway') { + messageID += 'connected.' + placement; + referenceID += 'connected'; + } else { + messageID += placement; + referenceID += placement; + } + + return [new validationIssue({ + type: type, + subtype: wayType, + severity: 'warning', + message: function(context) { + var entity = context.hasEntity(this.entityIds[0]); + return entity ? t('issues.impossible_oneway.' + messageID + '.message', { + feature: utilDisplayLabel(entity, context) + }) : ''; + }, + reference: getReference(referenceID), + entityIds: [way.id, node.id], + fixes: fixes + })]; + + function getReference(referenceID) { + return function showReference(selection) { + selection.selectAll('.issue-reference') + .data([0]) + .enter() + .append('div') + .attr('class', 'issue-reference') + .text(t('issues.impossible_oneway.' + referenceID + '.reference')); + }; } } - return false; - } + }; function continueDrawing(way, vertex, context) { // make sure the vertex is actually visible and editable @@ -41,189 +223,6 @@ export function validationImpossibleOneway() { ); } - function nodeOccursMoreThanOnce(way, nodeID) { - var occurences = 0; - for (var index in way.nodes) { - if (way.nodes[index] === nodeID) { - occurences += 1; - if (occurences > 1) return true; - } - } - return false; - } - - function isConnectedViaOtherTypes(context, way, node) { - - var wayType = typeForWay(way, context); - - if (wayType === 'highway') { - // entrances are considered connected - if (node.tags.entrance && node.tags.entrance !== 'no') return true; - if (node.tags.amenity === 'parking_entrance') return true; - } else if (wayType === 'waterway') { - if (node.id === way.first()) { - // multiple waterways may start at the same spring - if (node.tags.natural === 'spring') return true; - } else { - // multiple waterways may end at the same drain - if (node.tags.manhole === 'drain') return true; - } - } - - return context.graph().parentWays(node).some(function(parentWay) { - if (parentWay.id === way.id) return false; - - if (wayType === 'highway') { - - // allow connections to highway areas - if (parentWay.geometry(context.graph()) === 'area' && - osmRoutableHighwayTagValues[parentWay.tags.highway]) return true; - - // count connections to ferry routes as connected - if (parentWay.tags.route === 'ferry') return true; - - return context.graph().parentRelations(parentWay).some(function(parentRelation) { - if (parentRelation.tags.type === 'route' && - parentRelation.tags.route === 'ferry') return true; - - // allow connections to highway multipolygons - return parentRelation.isMultipolygon() && osmRoutableHighwayTagValues[parentRelation.tags.highway]; - }); - } else if (wayType === 'waterway') { - // multiple waterways may start or end at a water body at the same node - if (parentWay.tags.natural === 'water' || - parentWay.tags.natural === 'coastline') return true; - } - return false; - }); - } - - function issuesForNode(context, way, nodeID) { - - var isFirst = nodeID === way.first(); - - var wayType = typeForWay(way, context); - - // ignore if this way is self-connected at this node - if (nodeOccursMoreThanOnce(way, nodeID)) return []; - - var osm = context.connection(); - if (!osm) return []; - - var node = context.hasEntity(nodeID); - - // ignore if this node or its tile are unloaded - if (!node || !osm.isDataLoaded(node.loc)) return []; - - if (isConnectedViaOtherTypes(context, way, node)) return []; - - var attachedWaysOfSameType = context.graph().parentWays(node).filter(function(parentWay) { - if (parentWay.id === way.id) return false; - return typeForWay(parentWay, context) === wayType; - }); - - // assume it's okay for waterways to start or end disconnected for now - if (wayType === 'waterway' && attachedWaysOfSameType.length === 0) return []; - - var attachedOneways = attachedWaysOfSameType.filter(function(attachedWay) { - return isOneway(attachedWay); - }); - - // ignore if the way is connected to some non-oneway features - if (attachedOneways.length < attachedWaysOfSameType.length) return []; - - if (attachedOneways.length) { - var connectedEndpointsOkay = attachedOneways.some(function(attachedOneway) { - if ((isFirst ? attachedOneway.first() : attachedOneway.last()) !== nodeID) return true; - if (nodeOccursMoreThanOnce(attachedOneway, nodeID)) return true; - return false; - }); - if (connectedEndpointsOkay) return []; - } - - var fixes = []; - - if (attachedOneways.length) { - fixes.push(new validationIssueFix({ - icon: 'iD-operation-reverse', - title: t('issues.fix.reverse_feature.title'), - entityIds: [way.id], - onClick: function() { - var id = this.issue.entityIds[0]; - context.perform(actionReverse(id), t('operations.reverse.annotation')); - } - })); - } - if (node.tags.noexit !== 'yes') { - fixes.push(new validationIssueFix({ - icon: 'iD-operation-continue' + (isFirst ? '-left' : ''), - title: t('issues.fix.continue_from_' + (isFirst ? 'start' : 'end') + '.title'), - onClick: function() { - var entityID = this.issue.entityIds[0]; - var vertexID = this.issue.entityIds[1]; - var way = context.entity(entityID); - var vertex = context.entity(vertexID); - continueDrawing(way, vertex, context); - } - })); - } - - var placement = isFirst ? 'start' : 'end', - messageID = wayType + '.', - referenceID = wayType + '.'; - - if (wayType === 'waterway') { - messageID += 'connected.' + placement; - referenceID += 'connected'; - } else { - messageID += placement; - referenceID += placement; - } - - return [new validationIssue({ - type: type, - subtype: wayType, - severity: 'warning', - message: function() { - var entity = context.hasEntity(this.entityIds[0]); - return entity ? t('issues.impossible_oneway.' + messageID + '.message', { - feature: utilDisplayLabel(entity, context) - }) : ''; - }, - reference: getReference(referenceID), - entityIds: [way.id, node.id], - fixes: fixes - })]; - - function getReference(referenceID) { - return function showReference(selection) { - selection.selectAll('.issue-reference') - .data([0]) - .enter() - .append('div') - .attr('class', 'issue-reference') - .text(t('issues.impossible_oneway.' + referenceID + '.reference')); - }; - } - } - - var validation = function checkImpossibleOneway(entity, context) { - - if (entity.type !== 'way' || entity.geometry(context.graph()) !== 'line') return []; - - if (entity.isClosed()) return []; - - if (!typeForWay(entity, context)) return []; - - if (!isOneway(entity)) return []; - - var firstIssues = issuesForNode(context, entity, entity.first()); - var lastIssues = issuesForNode(context, entity, entity.last()); - - return firstIssues.concat(lastIssues); - }; - - validation.type = type; return validation; diff --git a/modules/validations/incompatible_source.js b/modules/validations/incompatible_source.js index 50b0d3eed..dd277e6b7 100644 --- a/modules/validations/incompatible_source.js +++ b/modules/validations/incompatible_source.js @@ -7,7 +7,7 @@ export function validationIncompatibleSource() { var type = 'incompatible_source'; var invalidSources = [{id:'google', regex:'google'}]; - var validation = function checkIncompatibleSource(entity, context) { + var validation = function checkIncompatibleSource(entity) { var issues = []; if (entity.tags && entity.tags.source) { @@ -18,7 +18,7 @@ export function validationIncompatibleSource() { issues.push(new validationIssue({ type: type, severity: 'warning', - message: function() { + message: function(context) { var entity = context.hasEntity(this.entityIds[0]); return entity ? t('issues.incompatible_source.' + invalidSource.id + '.feature.message', { feature: utilDisplayLabel(entity, context) diff --git a/modules/validations/invalid_format.js b/modules/validations/invalid_format.js index 16902ce19..e264af49f 100644 --- a/modules/validations/invalid_format.js +++ b/modules/validations/invalid_format.js @@ -5,7 +5,7 @@ import { validationIssue } from '../core/validation'; export function validationFormatting() { var type = 'invalid_format'; - var validation = function(entity, context) { + var validation = function(entity) { var issues = []; function isValidEmail(email) { @@ -50,7 +50,7 @@ export function validationFormatting() { type: type, subtype: 'website', severity: 'warning', - message: function() { + message: function(context) { var entity = context.hasEntity(this.entityIds[0]); return entity ? t('issues.invalid_format.website.message' + this.data, { feature: utilDisplayLabel(entity, context), site: websites.join(', ') }) : ''; @@ -72,7 +72,7 @@ export function validationFormatting() { type: type, subtype: 'email', severity: 'warning', - message: function() { + message: function(context) { var entity = context.hasEntity(this.entityIds[0]); return entity ? t('issues.invalid_format.email.message' + this.data, { feature: utilDisplayLabel(entity, context), email: emails.join(', ') }) : ''; @@ -91,4 +91,4 @@ export function validationFormatting() { validation.type = type; return validation; -} \ No newline at end of file +} diff --git a/modules/validations/maprules.js b/modules/validations/maprules.js index 5a66b8ec9..08477727f 100644 --- a/modules/validations/maprules.js +++ b/modules/validations/maprules.js @@ -4,10 +4,9 @@ import { services } from '../services'; export function validationMaprules() { var type = 'maprules'; - var validation = function checkMaprules(entity, context) { + var validation = function checkMaprules(entity, graph) { if (!services.maprules) return []; - var graph = context.graph(); var rules = services.maprules.validationRules(); var issues = []; diff --git a/modules/validations/missing_role.js b/modules/validations/missing_role.js index 0ab0f8331..33bfe7758 100644 --- a/modules/validations/missing_role.js +++ b/modules/validations/missing_role.js @@ -8,22 +8,22 @@ import { validationIssue, validationIssueFix } from '../core/validation'; export function validationMissingRole() { var type = 'missing_role'; - var validation = function checkMissingRole(entity, context) { + var validation = function checkMissingRole(entity, graph) { var issues = []; if (entity.type === 'way') { - context.graph().parentRelations(entity).forEach(function(relation) { + graph.parentRelations(entity).forEach(function(relation) { if (!relation.isMultipolygon()) return; var member = relation.memberById(entity.id); if (member && isMissingRole(member)) { - issues.push(makeIssue(entity, relation, member, context)); + issues.push(makeIssue(entity, relation, member)); } }); } else if (entity.type === 'relation' && entity.isMultipolygon()) { entity.indexedMembers().forEach(function(member) { - var way = context.hasEntity(member.id); + var way = graph.hasEntity(member.id); if (way && isMissingRole(member)) { - issues.push(makeIssue(way, entity, member, context)); + issues.push(makeIssue(way, entity, member)); } }); } @@ -37,11 +37,11 @@ export function validationMissingRole() { } - function makeIssue(way, relation, member, context) { + function makeIssue(way, relation, member) { return new validationIssue({ type: type, severity: 'warning', - message: function() { + message: function(context) { var member = context.hasEntity(this.entityIds[1]), relation = context.hasEntity(this.entityIds[0]); return (member && relation) ? t('issues.missing_role.message', { @@ -56,12 +56,12 @@ export function validationMissingRole() { }, hash: member.index.toString(), fixes: [ - makeAddRoleFix('inner', context), - makeAddRoleFix('outer', context), + makeAddRoleFix('inner'), + makeAddRoleFix('outer'), new validationIssueFix({ icon: 'iD-operation-delete', title: t('issues.fix.remove_from_relation.title'), - onClick: function() { + onClick: function(context) { context.perform( actionDeleteMember(this.issue.entityIds[0], this.issue.data.member.index), t('operations.delete_member.annotation') @@ -83,10 +83,10 @@ export function validationMissingRole() { } - function makeAddRoleFix(role, context) { + function makeAddRoleFix(role) { return new validationIssueFix({ title: t('issues.fix.set_as_' + role + '.title'), - onClick: function() { + onClick: function(context) { var oldMember = this.issue.data.member; var member = { id: this.issue.entityIds[1], type: oldMember.type, role: role }; context.perform( diff --git a/modules/validations/missing_tag.js b/modules/validations/missing_tag.js index 1e69ba43f..2ec9ce4d0 100644 --- a/modules/validations/missing_tag.js +++ b/modules/validations/missing_tag.js @@ -35,8 +35,7 @@ export function validationMissingTag() { } - var validation = function checkMissingTag(entity, context) { - var graph = context.graph(); + var validation = function checkMissingTag(entity, graph) { // ignore vertex features and relation members if (entity.geometry(graph) === 'vertex' || entity.hasParentRelations(graph)) { @@ -63,7 +62,7 @@ export function validationMissingTag() { new validationIssueFix({ icon: 'iD-icon-search', title: t('issues.fix.' + selectFixType + '.title'), - onClick: function() { + onClick: function(context) { context.ui().sidebar.showPresetList(); } }) @@ -71,27 +70,19 @@ export function validationMissingTag() { // can always delete if the user created it in the first place.. var canDelete = (entity.version === undefined || entity.v !== undefined); - - // otherwise check with operationDelete whether we can delete this entity - if (!canDelete) { - canDelete = !operationDelete([entity.id], context).disabled(); - } - - if (canDelete) { - fixes.push( - new validationIssueFix({ - icon: 'iD-operation-delete', - title: t('issues.fix.delete_feature.title'), - onClick: function() { - var id = this.issue.entityIds[0]; - var operation = operationDelete([id], context); - if (!operation.disabled()) { - operation(); - } + fixes.push( + new validationIssueFix({ + icon: 'iD-operation-delete', + title: t('issues.fix.delete_feature.title'), + onClick: function(context) { + var id = this.issue.entityIds[0]; + var operation = operationDelete([id], context); + if (!operation.disabled()) { + operation(); } - }) - ); - } + } + }) + ); var messageID = subtype === 'highway_classification' ? 'unknown_road' : 'missing_tag.' + subtype; var referenceID = subtype === 'highway_classification' ? 'unknown_road' : 'missing_tag'; @@ -102,7 +93,7 @@ export function validationMissingTag() { type: type, subtype: subtype, severity: severity, - message: function() { + message: function(context) { var entity = context.hasEntity(this.entityIds[0]); return entity ? t('issues.' + messageID + '.message', { feature: utilDisplayLabel(entity, context) diff --git a/modules/validations/outdated_tags.js b/modules/validations/outdated_tags.js index 22bd00bed..ef0a2ea4e 100644 --- a/modules/validations/outdated_tags.js +++ b/modules/validations/outdated_tags.js @@ -9,7 +9,7 @@ import { utilDisplayLabel, utilTagDiff } from '../util'; import { validationIssue, validationIssueFix } from '../core/validation'; -export function validationOutdatedTags() { +export function validationOutdatedTags(context) { var type = 'outdated_tags'; // initialize name-suggestion-index matcher @@ -28,8 +28,7 @@ export function validationOutdatedTags() { }); - function oldTagIssues(entity, context) { - var graph = context.graph(); + function oldTagIssues(entity, graph) { var oldTags = Object.assign({}, entity.tags); // shallow copy var preset = context.presets().match(entity, graph); var explicitPresetUpgrade = preset.replacement; @@ -127,7 +126,7 @@ export function validationOutdatedTags() { new validationIssueFix({ autoArgs: [doUpgrade, t('issues.fix.upgrade_tags.annotation')], title: t('issues.fix.upgrade_tags.title'), - onClick: function() { + onClick: function(context) { context.perform(doUpgrade, t('issues.fix.upgrade_tags.annotation')); } }) @@ -136,7 +135,7 @@ export function validationOutdatedTags() { function doUpgrade(graph) { - var currEntity = context.hasEntity(entity.id); + var currEntity = graph.hasEntity(entity.id); if (!currEntity) return graph; var newTags = Object.assign({}, currEntity.tags); // shallow copy @@ -152,7 +151,7 @@ export function validationOutdatedTags() { } - function showMessage() { + function showMessage(context) { var currEntity = context.hasEntity(entity.id); if (!currEntity) return ''; @@ -194,8 +193,7 @@ export function validationOutdatedTags() { } - function oldMultipolygonIssues(entity, context) { - var graph = context.graph(); + function oldMultipolygonIssues(entity, graph) { var multipolygon, outerWay; if (entity.type === 'relation') { @@ -221,7 +219,7 @@ export function validationOutdatedTags() { new validationIssueFix({ autoArgs: [doUpgrade, t('issues.fix.move_tags.annotation')], title: t('issues.fix.move_tags.title'), - onClick: function() { + onClick: function(context) { context.perform(doUpgrade, t('issues.fix.move_tags.annotation')); } }) @@ -230,8 +228,8 @@ export function validationOutdatedTags() { function doUpgrade(graph) { - var currMultipolygon = context.hasEntity(multipolygon.id); - var currOuterWay = context.hasEntity(outerWay.id); + var currMultipolygon = graph.hasEntity(multipolygon.id); + var currOuterWay = graph.hasEntity(outerWay.id); if (!currMultipolygon || !currOuterWay) return graph; currMultipolygon = currMultipolygon.mergeTags(currOuterWay.tags); @@ -240,7 +238,7 @@ export function validationOutdatedTags() { } - function showMessage() { + function showMessage(context) { var currMultipolygon = context.hasEntity(multipolygon.id); if (!currMultipolygon) return ''; @@ -261,9 +259,9 @@ export function validationOutdatedTags() { } - var validation = function checkOutdatedTags(entity, context) { - var issues = oldMultipolygonIssues(entity, context); - if (!issues.length) issues = oldTagIssues(entity, context); + var validation = function checkOutdatedTags(entity, graph) { + var issues = oldMultipolygonIssues(entity, graph); + if (!issues.length) issues = oldTagIssues(entity, graph); return issues; }; diff --git a/modules/validations/private_data.js b/modules/validations/private_data.js index c0e6cff63..3317ea991 100644 --- a/modules/validations/private_data.js +++ b/modules/validations/private_data.js @@ -40,7 +40,7 @@ export function validationPrivateData() { }; - var validation = function checkPrivateData(entity, context) { + var validation = function checkPrivateData(entity) { var tags = entity.tags; if (!tags.building || !privateBuildingValues[tags.building]) return []; @@ -67,7 +67,7 @@ export function validationPrivateData() { new validationIssueFix({ icon: 'iD-operation-delete', title: t('issues.fix.' + fixID + '.title'), - onClick: function() { + onClick: function(context) { context.perform(doUpgrade, t('issues.fix.upgrade_tags.annotation')); } }) @@ -76,7 +76,7 @@ export function validationPrivateData() { function doUpgrade(graph) { - var currEntity = context.hasEntity(entity.id); + var currEntity = graph.hasEntity(entity.id); if (!currEntity) return graph; var newTags = Object.assign({}, currEntity.tags); // shallow copy @@ -92,7 +92,7 @@ export function validationPrivateData() { } - function showMessage() { + function showMessage(context) { var currEntity = context.hasEntity(this.entityIds[0]); if (!currEntity) return ''; diff --git a/modules/validations/tag_suggests_area.js b/modules/validations/tag_suggests_area.js index 59e1e79fd..be61d774a 100644 --- a/modules/validations/tag_suggests_area.js +++ b/modules/validations/tag_suggests_area.js @@ -7,11 +7,11 @@ import { utilDisplayLabel, utilTagText } from '../util'; import { validationIssue, validationIssueFix } from '../core/validation'; -export function validationTagSuggestsArea() { +export function validationTagSuggestsArea(context) { var type = 'tag_suggests_area'; - var validation = function checkTagSuggestsArea(entity, context) { + var validation = function checkTagSuggestsArea(entity, graph) { if (entity.type !== 'way' || entity.isClosed()) return []; var tagSuggestingArea = entity.tagSuggestingArea(); @@ -32,7 +32,7 @@ export function validationTagSuggestsArea() { // must have at least three nodes to close this automatically if (entity.nodes.length >= 3) { - var nodes = context.graph().childNodes(entity), testNodes; + var nodes = graph.childNodes(entity), testNodes; var firstToLastDistanceMeters = geoSphericalDistance(nodes[0].loc, nodes[nodes.length-1].loc); // if the distance is very small, attempt to merge the endpoints @@ -42,7 +42,7 @@ export function validationTagSuggestsArea() { testNodes.push(testNodes[0]); // make sure this will not create a self-intersection if (!geoHasSelfIntersections(testNodes, testNodes[0].id)) { - connectEndpointsOnClick = function() { + connectEndpointsOnClick = function(context) { var way = context.entity(this.issue.entityIds[0]); context.perform( actionMergeNodes([way.nodes[0], way.nodes[way.nodes.length-1]], nodes[0].loc), @@ -58,7 +58,7 @@ export function validationTagSuggestsArea() { testNodes.push(testNodes[0]); // make sure this will not create a self-intersection if (!geoHasSelfIntersections(testNodes, testNodes[0].id)) { - connectEndpointsOnClick = function() { + connectEndpointsOnClick = function(context) { var wayId = this.issue.entityIds[0]; var way = context.entity(wayId); var nodeId = way.nodes[0]; @@ -80,7 +80,7 @@ export function validationTagSuggestsArea() { fixes.push(new validationIssueFix({ icon: 'iD-operation-delete', title: t('issues.fix.remove_tag.title'), - onClick: function() { + onClick: function(context) { var entityId = this.issue.entityIds[0]; var entity = context.entity(entityId); var tags = Object.assign({}, entity.tags); // shallow copy @@ -97,7 +97,7 @@ export function validationTagSuggestsArea() { return [new validationIssue({ type: type, severity: 'warning', - message: function() { + message: function(context) { var entity = context.hasEntity(this.entityIds[0]); return entity ? t('issues.tag_suggests_area.message', { feature: utilDisplayLabel(entity, context), diff --git a/modules/validations/unsquare_way.js b/modules/validations/unsquare_way.js index 8ecdebb05..4702b122b 100644 --- a/modules/validations/unsquare_way.js +++ b/modules/validations/unsquare_way.js @@ -4,9 +4,9 @@ import { actionOrthogonalize } from '../actions/orthogonalize'; import { geoOrthoCanOrthogonalize } from '../geo/ortho'; import { utilDisplayLabel } from '../util'; import { validationIssue, validationIssueFix } from '../core/validation'; +import { services } from '../services'; - -export function validationUnsquareWay() { +export function validationUnsquareWay(context) { var type = 'unsquare_way'; var DEFAULT_DEG_THRESHOLD = 5; // see also issues.js @@ -20,8 +20,8 @@ export function validationUnsquareWay() { } - var validation = function checkUnsquareWay(entity, context) { - var graph = context.graph(); + var validation = function checkUnsquareWay(entity, graph) { + if (!isBuilding(entity, graph)) return []; // don't flag ways marked as physically unsquare @@ -31,11 +31,11 @@ export function validationUnsquareWay() { if (!isClosed) return []; // this building has bigger problems // don't flag ways with lots of nodes since they are likely detail-mapped - var nodes = context.childNodes(entity).slice(); // shallow copy + var nodes = graph.childNodes(entity).slice(); // shallow copy if (nodes.length > nodeThreshold + 1) return []; // +1 because closing node appears twice // ignore if not all nodes are fully downloaded - var osm = context.connection(); + var osm = services.osm; if (!osm || nodes.some(function(node) { return !osm.isDataLoaded(node.loc); })) return []; // don't flag connected ways to avoid unresolvable unsquare loops @@ -72,7 +72,7 @@ export function validationUnsquareWay() { return [new validationIssue({ type: type, severity: 'warning', - message: function() { + message: function(context) { var entity = context.hasEntity(this.entityIds[0]); return entity ? t('issues.unsquare_way.message', { feature: utilDisplayLabel(entity, context) }) : ''; }, @@ -84,7 +84,7 @@ export function validationUnsquareWay() { icon: 'iD-operation-orthogonalize', title: t('issues.fix.square_feature.title'), autoArgs: autoArgs, - onClick: function(completionHandler) { + onClick: function(context, completionHandler) { var entityId = this.issue.entityIds[0]; // use same degree threshold as for detection context.perform( @@ -97,7 +97,7 @@ export function validationUnsquareWay() { }), new validationIssueFix({ title: t('issues.fix.tag_as_unsquare.title'), - onClick: function() { + onClick: function(context) { var entityId = this.issue.entityIds[0]; var entity = context.entity(entityId); var tags = Object.assign({}, entity.tags); // shallow copy diff --git a/test/spec/services/maprules.js b/test/spec/services/maprules.js index 4e27774b6..a1b8ac95e 100644 --- a/test/spec/services/maprules.js +++ b/test/spec/services/maprules.js @@ -562,10 +562,10 @@ describe('maprules', function() { expect(issues.length).to.eql(1); expect(issue.entityIds).to.eql([entity.id]); - expect(issue.message()).to.eql(selector[type]); + expect(issue.message(context)).to.eql(selector[type]); expect(type).to.eql(issue.severity); }); }); }); }); -}); \ No newline at end of file +}); diff --git a/test/spec/validations/almost_junction.js b/test/spec/validations/almost_junction.js index df7fb686e..ce600a5d2 100644 --- a/test/spec/validations/almost_junction.js +++ b/test/spec/validations/almost_junction.js @@ -127,12 +127,12 @@ describe('iD.validations.almost_junction', function () { } function validate() { - var validator = iD.validationAlmostJunction(); + var validator = iD.validationAlmostJunction(context); var changes = context.history().changes(); var entities = changes.modified.concat(changes.created); var issues = []; entities.forEach(function(entity) { - issues = issues.concat(validator(entity, context)); + issues = issues.concat(validator(entity, context.graph())); }); return issues; } @@ -166,7 +166,7 @@ describe('iD.validations.almost_junction', function () { expect(issue.data.cross_loc[1]).to.eql(0); expect(issue.fixes).to.have.lengthOf(2); - issue.fixes[0].onClick(); + issue.fixes[0].onClick(context); issues = validate(); expect(issues).to.have.lengthOf(0); }); @@ -195,7 +195,7 @@ describe('iD.validations.almost_junction', function () { expect(issue.data.cross_loc[1]).to.eql(0); expect(issue.fixes).to.have.lengthOf(2); - issue.fixes[1].onClick(); + issue.fixes[1].onClick(context); issues = validate(); expect(issues).to.have.lengthOf(0); }); diff --git a/test/spec/validations/crossing_ways.js b/test/spec/validations/crossing_ways.js index 972fc54c7..7e3dadc9e 100644 --- a/test/spec/validations/crossing_ways.js +++ b/test/spec/validations/crossing_ways.js @@ -54,12 +54,12 @@ describe('iD.validations.crossing_ways', function () { } function validate() { - var validator = iD.validationCrossingWays(); + var validator = iD.validationCrossingWays(context); var changes = context.history().changes(); var entities = changes.modified.concat(changes.created); var issues = []; entities.forEach(function(entity) { - issues = issues.concat(validator(entity, context)); + issues = issues.concat(validator(entity, context.graph())); }); return issues; } diff --git a/test/spec/validations/disconnected_way.js b/test/spec/validations/disconnected_way.js index 30a104560..b0090a1a8 100644 --- a/test/spec/validations/disconnected_way.js +++ b/test/spec/validations/disconnected_way.js @@ -34,12 +34,12 @@ describe('iD.validations.disconnected_way', function () { } function validate() { - var validator = iD.validationDisconnectedWay(); + var validator = iD.validationDisconnectedWay(context); var changes = context.history().changes(); var entities = changes.modified.concat(changes.created); var issues = []; entities.forEach(function(entity) { - issues = issues.concat(validator(entity, context)); + issues = issues.concat(validator(entity, context.graph())); }); return issues; } diff --git a/test/spec/validations/generic_name.js b/test/spec/validations/generic_name.js index 337c239cc..f128fb8ff 100644 --- a/test/spec/validations/generic_name.js +++ b/test/spec/validations/generic_name.js @@ -20,12 +20,12 @@ describe('iD.validations.generic_name', function () { } function validate() { - var validator = iD.validationGenericName(); + var validator = iD.validationGenericName(context); var changes = context.history().changes(); var entities = changes.modified.concat(changes.created); var issues = []; entities.forEach(function(entity) { - issues = issues.concat(validator(entity, context)); + issues = issues.concat(validator(entity, context.graph())); }); return issues; } diff --git a/test/spec/validations/incompatible_source.js b/test/spec/validations/incompatible_source.js index 600f91403..c4f4cd9cf 100644 --- a/test/spec/validations/incompatible_source.js +++ b/test/spec/validations/incompatible_source.js @@ -20,12 +20,12 @@ describe('iD.validations.incompatible_source', function () { } function validate() { - var validator = iD.validationIncompatibleSource(); + var validator = iD.validationIncompatibleSource(context); var changes = context.history().changes(); var entities = changes.modified.concat(changes.created); var issues = []; entities.forEach(function(entity) { - issues = issues.concat(validator(entity, context)); + issues = issues.concat(validator(entity, context.graph())); }); return issues; } diff --git a/test/spec/validations/missing_role.js b/test/spec/validations/missing_role.js index 77a79e463..8aa02cf2b 100644 --- a/test/spec/validations/missing_role.js +++ b/test/spec/validations/missing_role.js @@ -34,12 +34,12 @@ describe('iD.validations.missing_role', function () { } function validate() { - var validator = iD.validationMissingRole(); + var validator = iD.validationMissingRole(context); var changes = context.history().changes(); var entities = changes.modified.concat(changes.created); var issues = []; entities.forEach(function(entity) { - issues = issues.concat(validator(entity, context)); + issues = issues.concat(validator(entity, context.graph())); }); return issues; } diff --git a/test/spec/validations/missing_tag.js b/test/spec/validations/missing_tag.js index e25ec29e0..43655541f 100644 --- a/test/spec/validations/missing_tag.js +++ b/test/spec/validations/missing_tag.js @@ -34,12 +34,12 @@ describe('iD.validations.missing_tag', function () { } function validate() { - var validator = iD.validationMissingTag(); + var validator = iD.validationMissingTag(context); var changes = context.history().changes(); var entities = changes.modified.concat(changes.created); var issues = []; entities.forEach(function(entity) { - issues = issues.concat(validator(entity, context)); + issues = issues.concat(validator(entity, context.graph())); }); return issues; } diff --git a/test/spec/validations/outdated_tags.js b/test/spec/validations/outdated_tags.js index 5c61746c8..d03609176 100644 --- a/test/spec/validations/outdated_tags.js +++ b/test/spec/validations/outdated_tags.js @@ -34,12 +34,12 @@ describe('iD.validations.outdated_tags', function () { } function validate() { - var validator = iD.validationOutdatedTags(); + var validator = iD.validationOutdatedTags(context); var changes = context.history().changes(); var entities = changes.modified.concat(changes.created); var issues = []; entities.forEach(function(entity) { - issues = issues.concat(validator(entity, context)); + issues = issues.concat(validator(entity, context.graph())); }); return issues; } diff --git a/test/spec/validations/private_data.js b/test/spec/validations/private_data.js index a927fc68a..93a935597 100644 --- a/test/spec/validations/private_data.js +++ b/test/spec/validations/private_data.js @@ -20,12 +20,12 @@ describe('iD.validations.private_data', function () { } function validate() { - var validator = iD.validationPrivateData(); + var validator = iD.validationPrivateData(context); var changes = context.history().changes(); var entities = changes.modified.concat(changes.created); var issues = []; entities.forEach(function(entity) { - issues = issues.concat(validator(entity, context)); + issues = issues.concat(validator(entity, context.graph())); }); return issues; } diff --git a/test/spec/validations/tag_suggests_area.js b/test/spec/validations/tag_suggests_area.js index 20e8cdc48..5f6d99b13 100644 --- a/test/spec/validations/tag_suggests_area.js +++ b/test/spec/validations/tag_suggests_area.js @@ -41,12 +41,12 @@ describe('iD.validations.tag_suggests_area', function () { } function validate() { - var validator = iD.validationTagSuggestsArea(); + var validator = iD.validationTagSuggestsArea(context); var changes = context.history().changes(); var entities = changes.modified.concat(changes.created); var issues = []; entities.forEach(function(entity) { - issues = issues.concat(validator(entity, context)); + issues = issues.concat(validator(entity, context.graph())); }); return issues; }