diff --git a/data/core.yaml b/data/core.yaml index 984b2f74e..c5d3080f1 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -1377,9 +1377,7 @@ en: specific: message: '{feature} has no "{tag}" tag' old_multipolygon: - title: Misplaced Multipolygon Tags message: "{multipolygon} has misplaced tags" - tip: "Find old multipolygons with tags assigned to the outer way" reference: "Multipolygons should be tagged on their relation, not their outer way." outdated_tags: title: Outdated Tags diff --git a/dist/locales/en.json b/dist/locales/en.json index cbab926e2..c911091b4 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1704,9 +1704,7 @@ } }, "old_multipolygon": { - "title": "Misplaced Multipolygon Tags", "message": "{multipolygon} has misplaced tags", - "tip": "Find old multipolygons with tags assigned to the outer way", "reference": "Multipolygons should be tagged on their relation, not their outer way." }, "outdated_tags": { diff --git a/modules/core/validator.js b/modules/core/validator.js index 4083bfe04..382484080 100644 --- a/modules/core/validator.js +++ b/modules/core/validator.js @@ -204,7 +204,7 @@ export function coreValidator(context) { runValidation('missing_role'); if (entity.type === 'relation') { - if (!runValidation('old_multipolygon')) { + if (!runValidation('outdated_tags')) { // don't flag missing tags if they are on the outer way ran.missing_tag = true; } diff --git a/modules/validations/index.js b/modules/validations/index.js index 60d9acd36..5265b2e33 100644 --- a/modules/validations/index.js +++ b/modules/validations/index.js @@ -6,7 +6,6 @@ export { validationIncompatibleSource } from './incompatible_source'; export { validationMaprules } from './maprules'; export { validationMissingRole } from './missing_role'; export { validationMissingTag } from './missing_tag'; -export { validationOldMultipolygon } from './old_multipolygon'; export { validationOutdatedTags } from './outdated_tags'; export { validationPrivateData } from './private_data'; export { validationTagSuggestsArea } from './tag_suggests_area'; diff --git a/modules/validations/old_multipolygon.js b/modules/validations/old_multipolygon.js deleted file mode 100644 index 773305c71..000000000 --- a/modules/validations/old_multipolygon.js +++ /dev/null @@ -1,68 +0,0 @@ -import { t } from '../util/locale'; -import { actionChangeTags } from '../actions/change_tags'; -import { osmIsOldMultipolygonOuterMember, osmOldMultipolygonOuterMemberOfRelation } from '../osm/multipolygon'; -import { utilDisplayLabel } from '../util'; -import { validationIssue, validationIssueFix } from '../core/validation'; - - -export function validationOldMultipolygon() { - var type = 'old_multipolygon'; - - - var validation = function checkOldMultipolygon(entity, context) { - var graph = context.graph(); - - var multipolygon, outerWay; - if (entity.type === 'relation') { - outerWay = osmOldMultipolygonOuterMemberOfRelation(entity, graph); - multipolygon = entity; - } else if (entity.type === 'way') { - multipolygon = osmIsOldMultipolygonOuterMember(entity, graph); - outerWay = entity; - } else { - return []; - } - - if (!multipolygon || !outerWay) return []; - - var multipolygonLabel = utilDisplayLabel(multipolygon, context); - return [new validationIssue({ - type: type, - severity: 'warning', - message: t('issues.old_multipolygon.message', { multipolygon: multipolygonLabel }), - reference: showReference, - entities: [outerWay, multipolygon], - fixes: [ - new validationIssueFix({ - autoArgs: [doUpgrade, t('issues.fix.move_tags.annotation')], - title: t('issues.fix.move_tags.title'), - onClick: function() { - context.perform(doUpgrade, t('issues.fix.move_tags.annotation')); - } - }) - ] - })]; - - - function doUpgrade(graph) { - multipolygon = multipolygon.mergeTags(outerWay.tags); - graph = graph.replace(multipolygon); - return actionChangeTags(outerWay.id, {})(graph); - } - - - function showReference(selection) { - selection.selectAll('.issue-reference') - .data([0]) - .enter() - .append('div') - .attr('class', 'issue-reference') - .text(t('issues.old_multipolygon.reference')); - } - }; - - - validation.type = type; - - return validation; -} diff --git a/modules/validations/outdated_tags.js b/modules/validations/outdated_tags.js index 37f73636e..4ceaecf88 100644 --- a/modules/validations/outdated_tags.js +++ b/modules/validations/outdated_tags.js @@ -2,6 +2,7 @@ import { t } from '../util/locale'; import { actionChangePreset } from '../actions/change_preset'; import { actionChangeTags } from '../actions/change_tags'; import { actionUpgradeTags } from '../actions/upgrade_tags'; +import { osmIsOldMultipolygonOuterMember, osmOldMultipolygonOuterMemberOfRelation } from '../osm/multipolygon'; import { utilArrayUnion, utilDisplayLabel } from '../util'; import { validationIssue, validationIssueFix } from '../core/validation'; @@ -9,8 +10,7 @@ import { validationIssue, validationIssueFix } from '../core/validation'; export function validationOutdatedTags() { var type = 'outdated_tags'; - - var validation = function checkOutdatedTags(entity, context) { + function oldTagIssues(entity, context) { var graph = context.graph(); var oldTags = Object.assign({}, entity.tags); // shallow copy var preset = context.presets().match(entity, graph); @@ -111,6 +111,66 @@ export function validationOutdatedTags() { }) .text(function(d) { return d; }); } + } + + function oldMultipolygonIssues(entity, context) { + + var graph = context.graph(); + + var multipolygon, outerWay; + if (entity.type === 'relation') { + outerWay = osmOldMultipolygonOuterMemberOfRelation(entity, graph); + multipolygon = entity; + } else if (entity.type === 'way') { + multipolygon = osmIsOldMultipolygonOuterMember(entity, graph); + outerWay = entity; + } else { + return []; + } + + if (!multipolygon || !outerWay) return []; + + var multipolygonLabel = utilDisplayLabel(multipolygon, context); + return [new validationIssue({ + type: type, + severity: 'warning', + message: t('issues.old_multipolygon.message', { multipolygon: multipolygonLabel }), + reference: showReference, + entities: [outerWay, multipolygon], + fixes: [ + new validationIssueFix({ + autoArgs: [doUpgrade, t('issues.fix.move_tags.annotation')], + title: t('issues.fix.move_tags.title'), + onClick: function() { + context.perform(doUpgrade, t('issues.fix.move_tags.annotation')); + } + }) + ] + })]; + + + function doUpgrade(graph) { + multipolygon = multipolygon.mergeTags(outerWay.tags); + graph = graph.replace(multipolygon); + return actionChangeTags(outerWay.id, {})(graph); + } + + + function showReference(selection) { + selection.selectAll('.issue-reference') + .data([0]) + .enter() + .append('div') + .attr('class', 'issue-reference') + .text(t('issues.old_multipolygon.reference')); + } + } + + + var validation = function checkOutdatedTags(entity, context) { + var issues = oldMultipolygonIssues(entity, context); + if (!issues.length) issues = oldTagIssues(entity, context); + return issues; }; diff --git a/test/spec/validations/old_multipolygon.js b/test/spec/validations/old_multipolygon.js deleted file mode 100644 index e7069c4e5..000000000 --- a/test/spec/validations/old_multipolygon.js +++ /dev/null @@ -1,75 +0,0 @@ -describe('iD.validations.old_multipolygon', function () { - var context; - - beforeEach(function() { - context = iD.coreContext(); - }); - - function createWay(tags) { - var n1 = iD.osmNode({id: 'n-1', loc: [4,4]}); - var n2 = iD.osmNode({id: 'n-2', loc: [4,5]}); - var w = iD.osmWay({id: 'w-1', nodes: ['n-1', 'n-2'], tags: tags}); - - context.perform( - iD.actionAddEntity(n1), - iD.actionAddEntity(n2), - iD.actionAddEntity(w) - ); - } - - function createRelation(wayTags, relationTags) { - var n1 = iD.osmNode({id: 'n-1', loc: [4,4]}); - var n2 = iD.osmNode({id: 'n-2', loc: [4,5]}); - var n3 = iD.osmNode({id: 'n-3', loc: [5,5]}); - var w = iD.osmWay({id: 'w-1', nodes: ['n-1', 'n-2', 'n-3', 'n-1'], tags: wayTags}); - var r = iD.osmRelation({id: 'r-1', members: [{id: 'w-1'}], tags: relationTags}); - - context.perform( - iD.actionAddEntity(n1), - iD.actionAddEntity(n2), - iD.actionAddEntity(n3), - iD.actionAddEntity(w), - iD.actionAddEntity(r) - ); - } - - function validate() { - var validator = iD.validationOldMultipolygon(); - var changes = context.history().changes(); - var entities = changes.modified.concat(changes.created); - var issues = []; - entities.forEach(function(entity) { - issues = issues.concat(validator(entity, context)); - }); - return issues; - } - - it('has no errors on init', function() { - var issues = validate(); - expect(issues).to.have.lengthOf(0); - }); - - it('ignores way with no relations', function() { - createWay({}); - var issues = validate(); - expect(issues).to.have.lengthOf(0); - }); - - it('ignores multipolygon tagged on the relation', function() { - createRelation({}, { type: 'multipolygon', building: 'yes' }); - var issues = validate(); - expect(issues).to.have.lengthOf(0); - }); - - it('flags multipolygon tagged on the outer way', function() { - createRelation({ building: 'yes' }, { type: 'multipolygon' }); - var issues = validate(); - expect(issues).to.not.have.lengthOf(0); - var issue = issues[0]; - expect(issue.type).to.eql('old_multipolygon'); - expect(issue.entities).to.have.lengthOf(2); - expect(issue.entities[0].id).to.eql('w-1'); - expect(issue.entities[1].id).to.eql('r-1'); - }); - -}); diff --git a/test/spec/validations/outdated_tags.js b/test/spec/validations/outdated_tags.js index 3c78f6da9..69414b050 100644 --- a/test/spec/validations/outdated_tags.js +++ b/test/spec/validations/outdated_tags.js @@ -17,6 +17,22 @@ describe('iD.validations.outdated_tags', function () { ); } + function createRelation(wayTags, relationTags) { + var n1 = iD.osmNode({id: 'n-1', loc: [4,4]}); + var n2 = iD.osmNode({id: 'n-2', loc: [4,5]}); + var n3 = iD.osmNode({id: 'n-3', loc: [5,5]}); + var w = iD.osmWay({id: 'w-1', nodes: ['n-1', 'n-2', 'n-3', 'n-1'], tags: wayTags}); + var r = iD.osmRelation({id: 'r-1', members: [{id: 'w-1'}], tags: relationTags}); + + context.perform( + iD.actionAddEntity(n1), + iD.actionAddEntity(n2), + iD.actionAddEntity(n3), + iD.actionAddEntity(w), + iD.actionAddEntity(r) + ); + } + function validate() { var validator = iD.validationOutdatedTags(); var changes = context.history().changes(); @@ -50,4 +66,28 @@ describe('iD.validations.outdated_tags', function () { expect(issue.entities[0].id).to.eql('w-1'); }); + + it('ignores way with no relations', function() { + createWay({}); + var issues = validate(); + expect(issues).to.have.lengthOf(0); + }); + + it('ignores multipolygon tagged on the relation', function() { + createRelation({}, { type: 'multipolygon', building: 'yes' }); + var issues = validate(); + expect(issues).to.have.lengthOf(0); + }); + + it('flags multipolygon tagged on the outer way', function() { + createRelation({ building: 'yes' }, { type: 'multipolygon' }); + var issues = validate(); + expect(issues).to.not.have.lengthOf(0); + var issue = issues[0]; + expect(issue.type).to.eql('outdated_tags'); + expect(issue.entities).to.have.lengthOf(2); + expect(issue.entities[0].id).to.eql('w-1'); + expect(issue.entities[1].id).to.eql('r-1'); + }); + });