Skip to content

Commit

Permalink
Undo 5b8db55 and instead get latest entity at time of fix
Browse files Browse the repository at this point in the history
Per conversation w @quincylvania we need the issue ids to remain
stable so that users can ignore them.

So instead of forcing a refresh by changing the `id` we will grab current
versions of entities at fix time so that tags aren't replaced wrongly.
  • Loading branch information
bhousel committed May 23, 2019
1 parent 5b8db55 commit 5440467
Showing 1 changed file with 44 additions and 15 deletions.
59 changes: 44 additions & 15 deletions modules/validations/outdated_tags.js
Expand Up @@ -4,7 +4,6 @@ import { matcher, brands } from 'name-suggestion-index';
import { actionChangePreset } from '../actions/change_preset';
import { actionChangeTags } from '../actions/change_tags';
import { actionUpgradeTags } from '../actions/upgrade_tags';
import { osmEntity } from '../osm/entity';
import { osmIsOldMultipolygonOuterMember, osmOldMultipolygonOuterMemberOfRelation } from '../osm/multipolygon';
import { utilDisplayLabel, utilTagDiff } from '../util';
import { validationIssue, validationIssueFix } from '../core/validation';
Expand Down Expand Up @@ -87,13 +86,10 @@ export function validationOutdatedTags() {
type: type,
subtype: subtype,
severity: 'warning',
message: function() {
var entity = context.hasEntity(this.entityIds[0]);
return entity ? t('issues.outdated_tags.' + prefix + 'message', { feature: utilDisplayLabel(entity, context) }) : '';
},
message: showMessage,
reference: showReference,
entityIds: [entity.id],
hash: osmEntity.key(entity),
hash: JSON.stringify(tagDiff),
fixes: [
new validationIssueFix({
autoArgs: [doUpgrade, t('issues.fix.upgrade_tags.annotation')],
Expand All @@ -107,7 +103,29 @@ export function validationOutdatedTags() {


function doUpgrade(graph) {
return actionChangeTags(entity.id, newTags)(graph);
var currEntity = context.hasEntity(entity.id);
if (!currEntity) return graph;

var newTags = Object.assign({}, currEntity.tags); // shallow copy
tagDiff.forEach(function(diff) {
if (diff.type === '-') {
delete newTags[diff.key];
} else if (diff.type === '+') {
newTags[diff.key] = diff.newVal;
}
});

return actionChangeTags(currEntity.id, newTags)(graph);
}


function showMessage() {
var currEntity = context.hasEntity(entity.id);
if (!currEntity) return '';

return t('issues.outdated_tags.' + prefix + 'message',
{ feature: utilDisplayLabel(currEntity, context) }
);
}


Expand Down Expand Up @@ -142,8 +160,8 @@ export function validationOutdatedTags() {
}
}

function oldMultipolygonIssues(entity, context) {

function oldMultipolygonIssues(entity, context) {
var graph = context.graph();

var multipolygon, outerWay;
Expand All @@ -163,10 +181,7 @@ export function validationOutdatedTags() {
type: type,
subtype: 'old_multipolygon',
severity: 'warning',
message: function() {
var entity = context.hasEntity(this.issue.entityIds[1]);
return entity ? t('issues.old_multipolygon.message', { multipolygon: utilDisplayLabel(entity, context) }) : '';
},
message: showMessage,
reference: showReference,
entityIds: [outerWay.id, multipolygon.id],
fixes: [
Expand All @@ -182,9 +197,23 @@ export function validationOutdatedTags() {


function doUpgrade(graph) {
multipolygon = multipolygon.mergeTags(outerWay.tags);
graph = graph.replace(multipolygon);
return actionChangeTags(outerWay.id, {})(graph);
var currMultipolygon = context.hasEntity(multipolygon.id);
var currOuterWay = context.hasEntity(outerWay.id);
if (!currMultipolygon || !currOuterWay) return graph;

currMultipolygon = currMultipolygon.mergeTags(currOuterWay.tags);
graph = graph.replace(currMultipolygon);
return actionChangeTags(currOuterWay.id, {})(graph);
}


function showMessage() {
var currMultipolygon = context.hasEntity(multipolygon.id);
if (!currMultipolygon) return '';

return t('issues.old_multipolygon.message',
{ multipolygon: utilDisplayLabel(currMultipolygon, context) }
);
}


Expand Down

0 comments on commit 5440467

Please sign in to comment.