Skip to content

Commit

Permalink
Rename "Fix Me" Requests validation rule to more general Help Requests
Browse files Browse the repository at this point in the history
Add subtypes to almost_junction and disconnected_way validation rules
Add additional validation documentation (re: #6100)
  • Loading branch information
quincylvania committed Oct 9, 2019
1 parent d578074 commit a3d6097
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 14 deletions.
47 changes: 41 additions & 6 deletions ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -387,25 +387,44 @@ A validation rule is an object that takes an entity and a graph and returns obje

Each `validationIssue` takes its rule's `type` and may include a `subtype` that further differentiates it.

#### Issue Types

##### `almost_junction`

A way ends close to another way, indicating they should likely be connected. The current distance threshold is 5 meters.

* `highway-highway`: both the ways are roads or paths; no issue is flagged if the endpoint is an entrance or is tagged `noexit=yes`

##### `close_nodes`

Two nodes have a very small distance between them without z-axis differentiation. The threshold distance is smaller for features expected to be mapped at higher levels of detail (e.g. paths, rooms).
Two nodes have a very small distance between them. The threshold distance is smaller for features expected to be mapped at higher levels of detail (e.g. paths, rooms).

* `detached`: the nodes are not part of any ways
* `detached`: the nodes are not part of any ways; close points with differing z-axis tags like `layer` and `level` are not flagged
* `vertices`: the nodes are adjacent members of a way

##### `crossing_ways`

Two ways cross without a junction node or enough information to clarify how they cross.

##### `disconnected_way`
##### `fixme_tag`

A feature has a `fixme` tag that existed before the user's current edits. Deleting the `fixme` tag marks the issue as resolved regardless of the user's other edits.
One or more interconnected, routable feature are not connected to the rest of the routable network (i.e. they form a routing island). A way is considered connected to the network if any of its nodes are on an unloaded tile, meaning large routing islands may not always be detected.

* `highway`: the feature is a road, path, ferry route, or elevator; entrances are also considered network connections

##### `help_request`

Someone has indicated a feature needs further attention.

* `fixme_tag`: a feature has a `fixme` tag that existed before the user's current edits; deleting the `fixme` tag marks the issue as resolved regardless of the user's other edits

##### `impossible_oneway`

A one-way line does not have a valid connection at its first or last node.

* `highway`: a one-way road or path does not start or end at another highway or an entrance
* `waterway`: multiple streams, etc., start or end at the same node that's not a spring, drain, or water body

##### `incompatible_source`

The `source` tag of a feature references a data source known to have a license incompatiable with OpenStreetMap. This is very much not exhaustive and currently only flags sources containing "google".
Expand All @@ -417,7 +436,17 @@ A tag of a feature has an unexpected syntax.
* `email`: the `email` tag does not look like "user@example.com"

##### `maprules`

An issue with the active [MapRules](https://github.com/radiant-maxar/maprules) validation rules.

##### `mismatched_geometry`

A feature's tags indicate it should have a different geometry than it currently does.

* `area_as_line`: an unclosed way has tags implying it should be a closed area (e.g. `area=yes` or `building=yes`)
* `vertex_as_point`: a detached node has tags implying it should be part of a way (e.g. `highway=stop`)
* `point_as_vertex`: a vertex node has tags implying it should be detached from ways (e.g. `amenity=cafe`)

##### `missing_role`

A relation membership does not have a set `role`.
Expand Down Expand Up @@ -445,9 +474,15 @@ A feature has nonstandard tags.
An email address, phone number, or fax number is present on a residential feature that isn't also tagged as a POI.

##### `suspicious_name`

There's indication that a `name` tag doesn't contain the actual name of the feature. Multilingual names like `name:de` are also checked.

* `generic_name`: a name matches the raw key or value of the feature's defining tags (e.g. `amenity`, `cafe`) or it matches something the name-suggestion-index discards as generic when checking the most common place names in OpenStreetMap
* `not_name`: a name tag matches a value under the `not:name` tag

##### `unsquare_way`

A way has corners close to, but not quite 90°. The user can vary the "close to" degree threshold between 0° and 20°. The default is 5°.
A way has corners close to, but not quite 90°. The user can vary the "close to" degree threshold between 0° and 20°. The default is 5°. Only buildings are currently flagged.

* `building`: the feature has a `building` tag; other feature types are not currently flagged
* `building`: the feature has a `building` tag

5 changes: 3 additions & 2 deletions data/core.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1410,14 +1410,15 @@ en:
highway:
message: "{highway} is disconnected from other roads and paths"
fixme_tag:
title: '"Fix Me" Requests'
message: '{feature} has a "Fix Me" request'
tip: 'Find features with "fixme" tags'
reference: 'A "fixme" tag indicates that a mapper has requested help with a feature.'
generic_name:
message: '{feature} has the suspicious name "{name}"'
message_language: '{feature} has the suspicious name "{name}" in {language}'
reference: "Names should be the actual, on-the-ground names of features."
help_request:
title: 'Help Requests'
tip: 'Find features where others requested assistance'
incompatible_source:
title: Suspicious Sources
tip: "Find features with suspicious source tags"
Expand Down
6 changes: 4 additions & 2 deletions dist/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -1744,16 +1744,18 @@
}
},
"fixme_tag": {
"title": "\"Fix Me\" Requests",
"message": "{feature} has a \"Fix Me\" request",
"tip": "Find features with \"fixme\" tags",
"reference": "A \"fixme\" tag indicates that a mapper has requested help with a feature."
},
"generic_name": {
"message": "{feature} has the suspicious name \"{name}\"",
"message_language": "{feature} has the suspicious name \"{name}\" in {language}",
"reference": "Names should be the actual, on-the-ground names of features."
},
"help_request": {
"title": "Help Requests",
"tip": "Find features where others requested assistance"
},
"incompatible_source": {
"title": "Suspicious Sources",
"tip": "Find features with suspicious source tags",
Expand Down
1 change: 1 addition & 0 deletions modules/validations/almost_junction.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export function validationAlmostJunction(context) {

issues.push(new validationIssue({
type: type,
subtype: 'highway-highway',
severity: 'warning',
message: function(context) {
var entity1 = context.hasEntity(this.entityIds[0]);
Expand Down
1 change: 1 addition & 0 deletions modules/validations/disconnected_way.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ export function validationDisconnectedWay() {

return [new validationIssue({
type: type,
subtype: 'highway',
severity: 'warning',
message: function(context) {
if (this.entityIds.length === 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ import { utilDisplayLabel } from '../util';
import { validationIssue } from '../core/validation';


export function validationFixmeTag(context) {
var type = 'fixme_tag';

export function validationHelpRequest(context) {
var type = 'help_request';

var validation = function checkFixmeTag(entity) {

Expand All @@ -22,6 +21,7 @@ export function validationFixmeTag(context) {

return [new validationIssue({
type: type,
subtype: 'fixme_tag',
severity: 'warning',
message: function(context) {
var entity = context.hasEntity(this.entityIds[0]);
Expand Down
2 changes: 1 addition & 1 deletion modules/validations/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ export { validationAlmostJunction } from './almost_junction';
export { validationCloseNodes } from './close_nodes';
export { validationCrossingWays } from './crossing_ways';
export { validationDisconnectedWay } from './disconnected_way';
export { validationFixmeTag } from './fixme_tag';
export { validationFormatting } from './invalid_format';
export { validationHelpRequest } from './help_request';
export { validationImpossibleOneway } from './impossible_oneway';
export { validationIncompatibleSource } from './incompatible_source';
export { validationMaprules } from './maprules';
Expand Down
2 changes: 2 additions & 0 deletions test/spec/validations/almost_junction.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ describe('iD.validations.almost_junction', function () {
expect(issues).to.have.lengthOf(1);
var issue = issues[0];
expect(issue.type).to.eql('almost_junction');
expect(issue.subtype).to.eql('highway-highway');
expect(issue.entityIds).to.have.lengthOf(3);
expect(issue.entityIds[0]).to.eql('w-1');
expect(issue.entityIds[1]).to.eql('n-1');
Expand Down Expand Up @@ -177,6 +178,7 @@ describe('iD.validations.almost_junction', function () {
expect(issues).to.have.lengthOf(1);
var issue = issues[0];
expect(issue.type).to.eql('almost_junction');
expect(issue.subtype).to.eql('highway-highway');
expect(issue.entityIds).to.have.lengthOf(3);
expect(issue.entityIds[0]).to.eql('w-1');
expect(issue.entityIds[1]).to.eql('n-1');
Expand Down
2 changes: 2 additions & 0 deletions test/spec/validations/disconnected_way.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ describe('iD.validations.disconnected_way', function () {
expect(issues).to.have.lengthOf(1);
var issue = issues[0];
expect(issue.type).to.eql('disconnected_way');
expect(issue.subtype).to.eql('highway');
expect(issue.severity).to.eql('warning');
expect(issue.entityIds).to.have.lengthOf(1);
expect(issue.entityIds[0]).to.eql('w-1');
Expand All @@ -66,6 +67,7 @@ describe('iD.validations.disconnected_way', function () {
expect(issues).to.have.lengthOf(1);
var issue = issues[0];
expect(issue.type).to.eql('disconnected_way');
expect(issue.subtype).to.eql('highway');
expect(issue.severity).to.eql('warning');
expect(issue.entityIds).to.have.lengthOf(1);
expect(issue.entityIds[0]).to.eql('w-1');
Expand Down

0 comments on commit a3d6097

Please sign in to comment.