Extend data model for validation issues

Add the Issues pane

1. Add a class to represent the validation issue
2. Extend the data model for an validation issue to
  (1) add a severity level field (useful for identify save-blocking issues later)
  (2) replace single entity with an array of entities (useful for issues involving multiple entities)
  (3) add a coordinates field for highlighting the location of the issue on the map
  (4) add a fixes field for possible automatic fixes

3. Update existing validation modules to use the new data model
This commit is contained in:
Xiaoming Gao
2018-12-18 20:03:57 -05:00
committed by Xiaoming Gao
parent 0d0521c936
commit 07a53fe6ea
14 changed files with 225 additions and 80 deletions

62
dist/locales/en.json vendored
View File

@@ -7579,9 +7579,6 @@
"SPW_PICC": {
"name": "SPW(allonie) PICC numerical imagery"
},
"US-TIGER-Roads-2012": {
"name": "TIGER Roads 2012"
},
"US-TIGER-Roads-2014": {
"description": "At zoom level 16+, public domain map data from the US Census. At lower zooms, only changes since 2006 minus changes already incorporated into OpenStreetMap",
"name": "TIGER Roads 2014"
@@ -7590,6 +7587,10 @@
"description": "Yellow = Public domain map data from the US Census. Red = Data not found in OpenStreetMap",
"name": "TIGER Roads 2017"
},
"US-TIGER-Roads-2018": {
"description": "Yellow = Public domain map data from the US Census. Red = Data not found in OpenStreetMap",
"name": "TIGER Roads 2018"
},
"US_Forest_Service_roads_overlay": {
"description": "Highway: Green casing = unclassified. Brown casing = track. Surface: gravel = light brown fill, Asphalt = black, paved = gray, ground =white, concrete = blue, grass = green. Seasonal = white bars",
"name": "U.S. Forest Roads Overlay"
@@ -7606,6 +7607,12 @@
},
"name": "UrbIS-Ortho 2017"
},
"UrbISOrtho2018": {
"attribution": {
"text": "Realized by means of Brussels UrbIS®© - Distribution & Copyright CIRB"
},
"name": "UrbIS-Ortho 2018"
},
"UrbisAdmFR": {
"attribution": {
"text": "Realized by means of Brussels UrbIS®© - Distribution & Copyright CIRB"
@@ -7690,11 +7697,33 @@
"description": "Japan GSI Standard Map. Widely covered.",
"name": "Japan GSI Standard Map"
},
"hike_n_bike": {
"helsingborg-orto": {
"attribution": {
"text": OpenStreetMap contributors"
"text": Helsingborg municipality"
},
"name": "Hike & Bike"
"description": "Orthophotos from the municipality of Helsingborg 2016, public domain",
"name": "Helsingborg Orthophoto"
},
"kalmar-orto-2014": {
"attribution": {
"text": "© Kalmar municipality"
},
"description": "Orthophotos for the north coast of the municipality of Kalmar 2014",
"name": "Kalmar North Orthophoto 2014"
},
"kalmar-orto-2016": {
"attribution": {
"text": "© Kalmar municipality"
},
"description": "Orthophotos for the south coast of the municipality of Kalmar 2016",
"name": "Kalmar South Orthophoto 2016"
},
"kalmar-orto-2018": {
"attribution": {
"text": "© Kalmar municipality"
},
"description": "Orthophotos for urban areas of the municipality of Kalmar 2018",
"name": "Kalmar Urban Orthophoto 2018"
},
"kelkkareitit": {
"attribution": {
@@ -7717,6 +7746,20 @@
"description": "Mosaic of Swedish orthophotos from the period 19701980. Is under construction.",
"name": "Lantmäteriet Historic Orthophoto 1975"
},
"lantmateriet-topowebb": {
"attribution": {
"text": "© Lantmäteriet, CC0"
},
"description": "Topographic map of Sweden 1:50 000",
"name": "Lantmäteriet Topographic Map"
},
"linkoping-orto": {
"attribution": {
"text": "© Linköping municipality"
},
"description": "Orthophotos from the municipality of Linköping 2010, open data",
"name": "Linköping Orthophoto"
},
"mapbox_locator_overlay": {
"attribution": {
"text": "Terms & Feedback"
@@ -7781,6 +7824,13 @@
},
"name": "Stamen Terrain"
},
"stockholm-orto": {
"attribution": {
"text": "© Stockholm municipality, CC0"
},
"description": "Orthophotos from the municipality of Stockholm 2015, CC0 license",
"name": "Stockholm Orthophoto"
},
"tf-cycle": {
"attribution": {
"text": "Maps © Thunderforest, Data © OpenStreetMap contributors"

View File

@@ -281,9 +281,12 @@ export function coreHistory(context) {
validate: function(changes) {
return _flatten(_map(Validations, function(fn) {
return fn(context)(changes, _stack[_index].graph);
}));
return _flatten(_map(
_filter(Validations, _isFunction),
function(fn) {
return fn(context)(changes, _stack[_index].graph);
}
));
},

View File

@@ -206,14 +206,17 @@ export default {
}
},
// when geometries match and tag matches are present, return a warning...
findWarnings: function (entity, graph, warnings) {
findIssues: function (entity, graph, issues) {
if (this.geometryMatches(entity, graph) && this.matches(entity)) {
var type = Object.keys(selector).indexOf('error') > -1 ? 'error' : 'warning';
warnings.push({
severity: type,
message: selector[type],
entity: entity
});
var severity = Object.keys(selector).indexOf('error') > -1
? ValidationIssueSeverity.error
: ValidationIssueSeverity.warning;
issues.push(new validationIssue({
type: ValidationIssueType.map_rule_issue,
severity: severity,
message: selector[severity],
entities: [entity],
}));
}
}
};

View File

@@ -1,3 +1,5 @@
import _map from 'lodash-es/map';
import { t } from '../util/locale';
import { modeSelect } from '../modes';
import { svgIcon } from '../svg';
@@ -11,10 +13,9 @@ export function uiCommitWarnings(context) {
function commitWarnings(selection) {
// maybe call these issues now?
var validations = context.issueManager().validate();
var issues = context.issueManager().validate();
validations = _reduce(validations, function(validations, val) {
validations = _reduce(issues, function(validations, val) {
var severity = val.severity;
if (validations.hasOwnProperty(severity)) {
validations[severity].push(val);
@@ -24,10 +25,10 @@ export function uiCommitWarnings(context) {
return validations;
}, {});
_forEach(validations, function(instances, type) {
_forEach(validations, function(instances, severity) {
instances = _uniqBy(instances, function(val) { return val.id + '_' + val.message.replace(/\s+/g,''); });
var section = type + '-section';
var instanceItem = type + '-item';
var section = severity + '-section';
var instanceItem = severity + '-item';
var container = selection.selectAll('.' + section)
.data(instances.length ? [0] : []);
@@ -41,7 +42,7 @@ export function uiCommitWarnings(context) {
containerEnter
.append('h3')
.text(type === 'warning' ? t('commit.warnings') : t('commit.errors'));
.text(severity === 'warning' ? t('commit.warnings') : t('commit.errors'));
containerEnter
.append('ul')
@@ -77,31 +78,35 @@ export function uiCommitWarnings(context) {
items = itemsEnter
.merge(items);
items
.on('mouseover', mouseover)
.on('mouseout', mouseout)
.on('click', warningClick);
function mouseover(d) {
if (d.entity) {
if (d.entities) {
context.surface().selectAll(
utilEntityOrMemberSelector([d.entity.id], context.graph())
utilEntityOrMemberSelector(
_map(d.entities, function(e) { return e.id; }),
context.graph()
)
).classed('hover', true);
}
}
function mouseout() {
context.surface().selectAll('.hover')
.classed('hover', false);
}
function warningClick(d) {
if (d.entity) {
context.map().zoomTo(d.entity);
context.enter(modeSelect(context, [d.entity.id]));
if (d.entities) {
context.map().zoomTo(d.entities[0]);
context.enter(modeSelect(
context,
_map(d.entities, function(e) { return e.id; }),
));
}
}
});

View File

@@ -1,3 +1,4 @@
import _map from 'lodash-es/map';
import {
event as d3_event,
select as d3_select
@@ -104,9 +105,9 @@ export function uiIssues(context) {
var name = 'issues_list';
var changes = context.history().changes();
var validations = context.history().validate(changes);
var issues = context.history().validate(changes);
/*validations = _reduce(validations, function(validations, val) {
/*validations = _reduce(issues, function(validations, val) {
var severity = val.severity;
if (validations.hasOwnProperty(severity)) {
validations[severity].push(val);
@@ -117,7 +118,7 @@ export function uiIssues(context) {
}, {});*/
var items = selection.selectAll('li')
.data(validations);
.data(issues);
// Exit
items.exit()
@@ -130,13 +131,18 @@ export function uiIssues(context) {
.call(tooltip()
.html(true)
.title(function(d) {
var tip = d.tooltip;
var tip = d.tooltip ? d.tooltip : '';
return uiTooltipHtml(tip);
})
.placement('bottom')
)
.on('click', function(d) {
context.enter(modeSelect(context, [d.entity.id]));
if (d.entities) {
context.enter(modeSelect(
context,
_map(d.entities, function(e) { return e.id; })
));
}
});
var label = enter

View File

@@ -2,27 +2,33 @@ import _isEmpty from 'lodash-es/isEmpty';
import { t } from '../util/locale';
import { utilTagText } from '../util/index';
import {
ValidationIssueType,
ValidationIssueSeverity,
validationIssue,
} from './validation_issue';
export function validationDeprecatedTag() {
var validation = function(changes) {
var warnings = [];
var issues = [];
for (var i = 0; i < changes.created.length; i++) {
var change = changes.created[i],
deprecatedTags = change.deprecatedTags();
if (!_isEmpty(deprecatedTags)) {
var tags = utilTagText({ tags: deprecatedTags });
warnings.push({
id: 'deprecated_tags',
issues.push(new validationIssue({
type: ValidationIssueType.deprecated_tags,
severity: ValidationIssueSeverity.warning,
message: t('validations.deprecated_tags', { tags: tags }),
entity: change
});
entities: [change],
}));
}
}
return warnings;
return issues;
};

View File

@@ -1,5 +1,10 @@
import { t } from '../util/locale';
import { utilDisplayName } from '../util';
import {
ValidationIssueType,
ValidationIssueSeverity,
validationIssue,
} from './validation_issue';
export function validationDisconnectedHighway(context) {
@@ -24,7 +29,7 @@ export function validationDisconnectedHighway(context) {
var validation = function(changes, graph) {
var warnings = [];
var issues = [];
for (var i = 0; i < changes.created.length; i++) {
var entity = changes.created[i];
@@ -38,16 +43,18 @@ export function validationDisconnectedHighway(context) {
entityLabel = utilDisplayType(entity.id)
}
}
warnings.push({
id: 'disconnected_highway',
message: t('validations.disconnected_highway', {entityLabel: entityLabel}),
issues.push(new validationIssue({
type: ValidationIssueType.disconnected_highway,
severity: ValidationIssueSeverity.error,
message: t('validations.disconnected_highway'),
tooltip: t('validations.disconnected_highway_tooltip'),
entity: entity
});
entities: [entity],
}));
}
}
return warnings;
return issues;
};

View File

@@ -1,5 +1,6 @@
export { validationDeprecatedTag } from './deprecated_tag';
export { validationDisconnectedHighway } from './disconnected_highway';
export { ValidationIssueType, ValidationIssueSeverity } from './validation_issue';
export { validationManyDeletions } from './many_deletions';
export { validationMapCSSChecks } from './mapcss_checks';
export { validationMissingTag } from './missing_tag';

View File

@@ -1,11 +1,15 @@
import { t } from '../util/locale';
import {
ValidationIssueType,
ValidationIssueSeverity,
validationIssue,
} from './validation_issue';
export function validationManyDeletions() {
var threshold = 100;
var validation = function(changes, graph) {
var warnings = [];
var issues = [];
var nodes=0, ways=0, areas=0, relations=0;
changes.deleted.forEach(function(c) {
@@ -15,14 +19,17 @@ export function validationManyDeletions() {
else if (c.type === 'relation') {relations++;}
});
if (changes.deleted.length > threshold) {
warnings.push({
id: 'many_deletions',
message: t('validations.many_deletions',
{ n: changes.deleted.length, p: nodes, l: ways, a:areas, r: relations })
});
issues.push(new validationIssue({
type: ValidationIssueType.many_deletions,
severity: ValidationIssueSeverity.warning,
message: t(
'validations.many_deletions',
{ n: changes.deleted.length, p: nodes, l: ways, a:areas, r: relations }
),
}));
}
return warnings;
return issues;
};

View File

@@ -5,7 +5,7 @@ export function validationMapCSSChecks() {
if (!services.maprules) return [];
var rules = services.maprules.validationRules();
var warnings = [];
var issues = [];
var createdModified = ['created', 'modified'];
for (var i = 0; i < rules.length; i++) {
@@ -14,12 +14,12 @@ export function validationMapCSSChecks() {
var type = createdModified[j];
var entities = changes[type];
for (var k = 0; k < entities.length; k++) {
rule.findWarnings(entities[k], graph, warnings);
rule.findIssues(entities[k], graph, issues);
}
}
}
return warnings;
return issues;
};
return validation;
}

View File

@@ -1,6 +1,10 @@
import _without from 'lodash-es/without';
import { t } from '../util/locale';
import {
ValidationIssueType,
ValidationIssueSeverity,
validationIssue,
} from './validation_issue';
export function validationMissingTag() {
@@ -12,23 +16,24 @@ export function validationMissingTag() {
var validation = function(changes, graph) {
var types = ['point', 'line', 'area', 'relation'],
warnings = [];
issues = [];
for (var i = 0; i < changes.created.length; i++) {
var change = changes.created[i],
geometry = change.geometry(graph);
if (types.indexOf(geometry) !== -1 && !hasTags(change, graph)) {
warnings.push({
id: 'missing_tag',
issues.push(new validationIssue({
type: ValidationIssueType.missing_tag,
severity: ValidationIssueSeverity.error,
message: t('validations.untagged_' + geometry),
tooltip: t('validations.untagged_' + geometry + '_tooltip'),
entity: change
});
entities: [change],
}));
}
}
return warnings;
return issues;
};

View File

@@ -1,23 +1,28 @@
import { t } from '../util/locale';
import { osmIsSimpleMultipolygonOuterMember } from '../osm';
import {
ValidationIssueType,
ValidationIssueSeverity,
validationIssue,
} from './validation_issue';
export function validationOldMultipolygon() {
return function validation(changes, graph) {
var warnings = [];
var issues = [];
for (var i = 0; i < changes.created.length; i++) {
var entity = changes.created[i];
var parent = osmIsSimpleMultipolygonOuterMember(entity, graph);
if (parent) {
warnings.push({
id: 'old_multipolygon',
issues.push(new validationIssue({
type: ValidationIssueType.old_multipolygon,
severity: ValidationIssueSeverity.warning,
message: t('validations.old_multipolygon'),
tooltip: t('validations.old_multipolygon_tooltip'),
entity: parent
});
entities: [parent],
}));
}
}
return warnings;
return issues;
};
}

View File

@@ -1,5 +1,10 @@
import _isEmpty from 'lodash-es/isEmpty';
import { t } from '../util/locale';
import {
ValidationIssueType,
ValidationIssueSeverity,
validationIssue,
} from './validation_issue';
// https://github.com/openstreetmap/josm/blob/mirror/src/org/
@@ -25,22 +30,23 @@ export function validationTagSuggestsArea() {
var validation = function(changes, graph) {
var warnings = [];
var issues = [];
for (var i = 0; i < changes.created.length; i++) {
var change = changes.created[i],
geometry = change.geometry(graph),
suggestion = (geometry === 'line' ? tagSuggestsArea(change.tags) : undefined);
if (suggestion) {
warnings.push({
id: 'tag_suggests_area',
issues.push(new validationIssue({
type: ValidationIssueType.tag_suggests_area,
severity: ValidationIssueSeverity.warning,
message: t('validations.tag_suggests_area', { tag: suggestion }),
entity: change
});
entities: [change],
}));
}
}
return warnings;
return issues;
};

View File

@@ -0,0 +1,41 @@
import _isObject from 'lodash-es/isObject';
var ValidationIssueType = Object.freeze({
deprecated_tags: 'deprecated_tags',
disconnected_highway: 'disconnected_highway',
many_deletions: 'many_deletions',
missing_tag: 'missing_tag',
old_multipolygon: 'old_multipolygon',
tag_suggests_area: 'tag_suggests_area',
map_rule_issue: 'map_rule_issue',
});
var ValidationIssueSeverity = Object.freeze({
warning: 'warning',
error: 'error',
});
export { ValidationIssueType, ValidationIssueSeverity };
export function validationIssue(attrs) {
if (!_isObject(attrs)) throw new Error('Input attrs is not an object');
if (!attrs.type || !ValidationIssueType.hasOwnProperty(attrs.type)) {
throw new Error('Invalid attrs.type: ' + attrs.type);
}
if (!attrs.severity || !ValidationIssueSeverity.hasOwnProperty(attrs.severity)) {
throw new Error('Invalid attrs.severity: ' + attr.severity);
}
if (!attrs.message) throw new Error('attrs.message is empty');
this.type = attrs.type;
this.severity = attrs.severity;
this.message = attrs.message;
this.tooltip = attrs.tooltip;
this.entities = attrs.entities; // expect an array of entities
this.coordinates = attrs.coordinates; // expect an array of [lon, lat]
this.fixes = attrs.fixes; // expect an array of functions for possible fixes
}