Skip to content

Commit be98a11

Browse files
mtmailmtmail
committed
Move logic from suspicious name to mututally_exclusive_tags
1 parent 4097c71 commit be98a11

File tree

5 files changed

+38
-116
lines changed

5 files changed

+38
-116
lines changed

‎data/core.yaml‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1833,9 +1833,6 @@ en:
18331833
amap: "Amap products are proprietary and must not be used as references."
18341834
baidu: "Baidu products are proprietary and must not be used as references."
18351835
google: "Google products are proprietary and must not be used as references."
1836-
incorrect_name:
1837-
message: '{feature} has the mistaken name "{name}"'
1838-
message_language: '{feature} has the mistaken name "{name}" in {language}'
18391836
invalid_format:
18401837
title: Invalid Formatting
18411838
tip: Find tags with unexpected formats
@@ -1870,8 +1867,12 @@ en:
18701867
mutually_exclusive_tags:
18711868
title: Contradictory Tags
18721869
tip: "Find features that have contradictory tags."
1873-
message: "{feature} has both {tag1} and {tag2}"
1874-
reference: 'The keys "{tag1}" and "{tag2}" contradict each other. Only one should be set.'
1870+
default:
1871+
message: "{feature} has both {tag1} and {tag2}"
1872+
reference: 'The tags "{tag1}" and "{tag2}" contradict each other. Only one tag should be set.'
1873+
same_value:
1874+
message: "{feature}: {tag1} and {tag2} have same value"
1875+
reference: 'The tags "{tag1}" and "{tag2}" have the same value. Only one tag should be set.'
18751876
old_multipolygon:
18761877
message: "{multipolygon} has misplaced tags"
18771878
reference: "Multipolygons should be tagged on their relation, not their outer way."

‎modules/validations/mutually_exclusive_tags.js‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,36 +29,39 @@ export function validationMutuallyExclusiveTags(/* context */) {
2929

3030
// Addtional:
3131
// Check if name and not:name (and similar) are set and both have the same value
32+
// not:name can actually have multiple values, separate by ;
3233
// https://taginfo.openstreetmap.org/search?q=not%3A#keys
3334
Object.keys(entity.tags).forEach((key) => {
3435
let negative_key = 'not:' + key;
35-
if (negative_key in entity.tags && entity.tags[key] === entity.tags[negative_key]) {
36-
pairsFounds.push([negative_key, key]);
36+
if (negative_key in entity.tags && entity.tags[negative_key].split(';').includes(entity.tags[key])) {
37+
pairsFounds.push([negative_key, key, 'same_value']);
3738
}
3839
// For name:xx we also compare against the not:name tag
3940
if (key.match(/^name:[a-z]+/)) {
4041
negative_key = 'not:name';
41-
if (negative_key in entity.tags && entity.tags[key] === entity.tags[negative_key]) {
42-
pairsFounds.push([negative_key, key]);
42+
if (negative_key in entity.tags && entity.tags[negative_key].split(';').includes(entity.tags[key])) {
43+
pairsFounds.push([negative_key, key, 'same_value']);
4344
}
4445
}
4546
});
4647

4748
let issues = pairsFounds.map((pair) => {
49+
const subtype = pair[2] || 'default';
4850
return new validationIssue({
4951
type: type,
52+
subtype: subtype,
5053
severity: 'warning',
5154
message: function(context) {
5255
let entity = context.hasEntity(this.entityIds[0]);
53-
return entity ? t.append('issues.' + type + '.message', {
56+
return entity ? t.append(`issues.${type}.${subtype}.message`, {
5457
feature: utilDisplayLabel(entity, context.graph()),
5558
tag1: pair[0],
5659
tag2: pair[1]
5760
}) : '';
5861
},
59-
reference: (selection) => showReference(selection, pair),
62+
reference: (selection) => showReference(selection, pair, subtype),
6063
entityIds: [entity.id],
61-
dynamicFixes: () => pair.map((tagToRemove) => createIssueFix(tagToRemove))
64+
dynamicFixes: () => pair.slice(0,2).map((tagToRemove) => createIssueFix(tagToRemove))
6265
});
6366
});
6467

@@ -79,13 +82,13 @@ export function validationMutuallyExclusiveTags(/* context */) {
7982
});
8083
}
8184

82-
function showReference(selection, pair) {
85+
function showReference(selection, pair, subtype) {
8386
selection.selectAll('.issue-reference')
8487
.data([0])
8588
.enter()
8689
.append('div')
8790
.attr('class', 'issue-reference')
88-
.call(t.append('issues.' + type + '.reference', { tag1: pair[0], tag2: pair[1] }));
91+
.call(t.append(`issues.${type}.${subtype}.reference`, { tag1: pair[0], tag2: pair[1] }));
8992
}
9093

9194
return issues;

‎modules/validations/suspicious_name.js‎

Lines changed: 10 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -96,53 +96,6 @@ export function validationSuspiciousName() {
9696
}
9797
}
9898

99-
function makeIncorrectNameIssue(entityId, nameKey, incorrectName, langCode) {
100-
return new validationIssue({
101-
type: type,
102-
subtype: 'not_name',
103-
severity: 'warning',
104-
message: function(context) {
105-
const entity = context.hasEntity(this.entityIds[0]);
106-
if (!entity) return '';
107-
const preset = presetManager.match(entity, context.graph());
108-
const langName = langCode && localizer.languageName(langCode);
109-
return t.append('issues.incorrect_name.message' + (langName ? '_language' : ''),
110-
{ feature: preset.name(), name: incorrectName, language: langName }
111-
);
112-
},
113-
reference: showReference,
114-
entityIds: [entityId],
115-
hash: `${nameKey}=${incorrectName}`,
116-
dynamicFixes: function() {
117-
return [
118-
new validationIssueFix({
119-
icon: 'iD-operation-delete',
120-
title: t.append('issues.fix.remove_the_name.title'),
121-
onClick: function(context) {
122-
const entityId = this.issue.entityIds[0];
123-
const entity = context.entity(entityId);
124-
let tags = Object.assign({}, entity.tags); // shallow copy
125-
delete tags[nameKey];
126-
context.perform(
127-
actionChangeTags(entityId, tags), t('issues.fix.remove_mistaken_name.annotation')
128-
);
129-
}
130-
})
131-
];
132-
}
133-
});
134-
135-
function showReference(selection) {
136-
selection.selectAll('.issue-reference')
137-
.data([0])
138-
.enter()
139-
.append('div')
140-
.attr('class', 'issue-reference')
141-
.call(t.append('issues.generic_name.reference'));
142-
}
143-
}
144-
145-
14699
let validation = function checkGenericName(entity) {
147100
const tags = entity.tags;
148101

@@ -151,23 +104,23 @@ export function validationSuspiciousName() {
151104
if (hasWikidata) return [];
152105

153106
let issues = [];
154-
const notNames = (tags['not:name'] || '').split(';');
107+
// const notNames = (tags['not:name'] || '').split(';');
155108

156109
for (let key in tags) {
157110
const m = key.match(/^name(?:(?::)([a-zA-Z_-]+))?$/);
158111
if (!m) continue;
159112

160113
const langCode = m.length >= 2 ? m[1] : null;
161114
const value = tags[key];
162-
if (notNames.length) {
163-
for (let i in notNames) {
164-
const notName = notNames[i];
165-
if (notName && value === notName) {
166-
issues.push(makeIncorrectNameIssue(entity.id, key, value, langCode));
167-
continue;
168-
}
169-
}
170-
}
115+
// if (notNames.length) {
116+
// for (let i in notNames) {
117+
// const notName = notNames[i];
118+
// if (notName && value === notName) {
119+
// issues.push(makeIncorrectNameIssue(entity.id, key, value, langCode));
120+
// continue;
121+
// }
122+
// }
123+
// }
171124
if (isGenericName(value, tags)) {
172125
issues.provisional = _waitingForNsi; // retry later if we are waiting on NSI to finish loading
173126
issues.push(makeGenericNameIssue(entity.id, key, value, langCode));

‎test/spec/validations/mutually_exclusive_tags.js‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,36 +49,42 @@ describe('iD.validations.mutually_exclusive_tags', function () {
4949
expect(issues).to.have.lengthOf(1);
5050
var issue = issues[0];
5151
expect(issue.type).to.eql('mutually_exclusive_tags');
52+
expect(issue.subtype).to.eql('default');
5253
expect(issue.severity).to.eql('warning');
5354
expect(issue.entityIds).to.have.lengthOf(1);
5455
expect(issue.entityIds[0]).to.eql('n-1');
5556
done();
5657
}, 20);
5758
});
5859

59-
it('flags mutually exclusive tags containing not:', function(done) {
60-
createNode({'name': 'Trader Joe', 'not:name': 'Trader Joe'});
60+
it('flags feature with a mutually exclusive `not:name` value', function(done) {
61+
createNode({ shop: 'supermarket', name: 'Lous', 'not:name': 'Lous' });
62+
// createNode({'name': 'Trader Joe', 'not:name': 'Trader Joe'});
6163
var validator = iD.validationMutuallyExclusiveTags(context);
6264
window.setTimeout(function() { // async, so data will be available
6365
var issues = validate(validator);
6466
expect(issues).to.have.lengthOf(1);
6567
var issue = issues[0];
6668
expect(issue.type).to.eql('mutually_exclusive_tags');
69+
expect(issue.subtype).to.eql('same_value');
6770
expect(issue.severity).to.eql('warning');
6871
expect(issue.entityIds).to.have.lengthOf(1);
6972
expect(issue.entityIds[0]).to.eql('n-1');
7073
done();
7174
}, 20);
7275
});
7376

74-
it('flags mutually exclusive tags containing not: for languages', function(done) {
75-
createNode({'name': 'Trader Jane', 'name:en': 'Trader Joe', 'not:name': 'Trader Joe'});
77+
78+
it('flags feature with a mutually exclusive semicolon-separated `not:name` value', function(done) {
79+
createNode({ shop: 'supermarket', name: 'Lous', 'not:name': 'Louis\';Lous;Louis\'s' });
80+
// createNode({'name': 'Trader Jane', 'name:en': 'Trader Joe', 'not:name': 'Trader Joe'});
7681
var validator = iD.validationMutuallyExclusiveTags(context);
7782
window.setTimeout(function() { // async, so data will be available
7883
var issues = validate(validator);
7984
expect(issues).to.have.lengthOf(1);
8085
var issue = issues[0];
8186
expect(issue.type).to.eql('mutually_exclusive_tags');
87+
expect(issue.subtype).to.eql('same_value');
8288
expect(issue.severity).to.eql('warning');
8389
expect(issue.entityIds).to.have.lengthOf(1);
8490
expect(issue.entityIds[0]).to.eql('n-1');

‎test/spec/validations/suspicious_name.js‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -183,45 +183,4 @@ describe('iD.validations.suspicious_name', function () {
183183
done();
184184
}, 20);
185185
});
186-
187-
it('ignores feature with a non-matching `not:name` tag', function(done) {
188-
createWay({ shop: 'supermarket', name: 'Lou\'s', 'not:name': 'Lous' });
189-
var validator = iD.validationSuspiciousName(context);
190-
window.setTimeout(function() { // async, so data will be available
191-
var issues = validate(validator);
192-
expect(issues).to.have.lengthOf(0);
193-
done();
194-
}, 20);
195-
});
196-
197-
it('flags feature with a matching `not:name` tag', function(done) {
198-
createWay({ shop: 'supermarket', name: 'Lous', 'not:name': 'Lous' });
199-
var validator = iD.validationSuspiciousName(context);
200-
window.setTimeout(function() { // async, so data will be available
201-
var issues = validate(validator);
202-
expect(issues).to.have.lengthOf(1);
203-
var issue = issues[0];
204-
expect(issue.type).to.eql('suspicious_name');
205-
expect(issue.subtype).to.eql('not_name');
206-
expect(issue.entityIds).to.have.lengthOf(1);
207-
expect(issue.entityIds[0]).to.eql('w-1');
208-
done();
209-
}, 20);
210-
});
211-
212-
it('flags feature with a matching a semicolon-separated `not:name` tag', function(done) {
213-
createWay({ shop: 'supermarket', name: 'Lous', 'not:name': 'Louis\';Lous;Louis\'s' });
214-
window.setTimeout(function() { // async, so data will be available
215-
var validator = iD.validationSuspiciousName(context);
216-
var issues = validate(validator);
217-
expect(issues).to.have.lengthOf(1);
218-
var issue = issues[0];
219-
expect(issue.type).to.eql('suspicious_name');
220-
expect(issue.subtype).to.eql('not_name');
221-
expect(issue.entityIds).to.have.lengthOf(1);
222-
expect(issue.entityIds[0]).to.eql('w-1');
223-
done();
224-
}, 20);
225-
});
226-
227186
});

0 commit comments

Comments
 (0)
⚡