revalidate entities with changed relation memberships

CHANGELOG.md

@@ -53,6 +53,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
 * Warn when a way with more than the maximum allowed number of nodes is to be uploaded and provide a way to fix it ([#7381])
 * The Suspicious Names validator warning now compares the Name field to the preset’s name in the user’s language, not just the raw tag value (typically in British English). ([#9522], thanks [@k-yle])
 * Revalidate ways that are connected to the currently edited way to also properly update/catch _disconnected way_s and _impossible oneway_ errors ([#8911], thanks [@andrewpmk])
+* Revalidate ways that are added to or removed from relations ([#10786])
 * Preserve `crossing:markings` tag when fixing missing connection of crossing path and road ([#9586], thanks [@jtracey])
 * Add a dedicated description to fix waterway-road intersections by adding a _culvert_ ([#10778], thanks [@matkoniecz])
 #### :bug: Bugfixes

modules/core/validator.js

@@ -4,7 +4,7 @@ import { prefs } from './preferences';
 import { coreDifference } from './difference';
 import { geoExtent } from '../geo/extent';
 import { modeSelect } from '../modes/select';
-import { utilArrayChunk, utilArrayGroupBy, utilEntityAndDeepMemberIDs, utilRebind } from '../util';
+import { utilArrayChunk, utilArrayDifference, utilArrayGroupBy, utilArrayIntersection, utilArrayUnion, utilEntityAndDeepMemberIDs, utilRebind } from '../util';
 import * as Validations from '../validations/index';
 
 
@@ -503,8 +503,8 @@ export function coreValidator(context) {
       return Promise.resolve();
     }
 
-    // Re-validate also connected (or previously connected) entities to the current way
-    // fix #8758
+    // revalidate also connected (or previously connected) entities to the current way
+    // https://github.com/openstreetmap/iD/issues/8758
     const addConnectedWays = graph => diff
       .filter(entityID => graph.hasEntity(entityID))
       .map(entityID    => graph.entity(entityID))
@@ -514,6 +514,22 @@ export function coreValidator(context) {
     addConnectedWays(currGraph);
     addConnectedWays(prevGraph);
 
+    // revalidate entities with changed relation memberships
+    // https://github.com/openstreetmap/iD/issues/10786
+    Object.values({...incrementalDiff.created(), ...incrementalDiff.deleted()})
+      .filter(e => e.type === 'relation')
+      .flatMap(r => r.members)
+      .forEach(m => entityIDs.add(m.id));
+    Object.values(incrementalDiff.modified())
+      .filter(e => e.type === 'relation')
+      .map(r => ({ baseEntity: prevGraph.entity(r.id), headEntity: r }))
+      .forEach(({ baseEntity, headEntity }) => {
+        const bm = baseEntity.members.map(m => m.id);
+        const hm = headEntity.members.map(m => m.id);
+        const symDiff = utilArrayDifference(utilArrayUnion(bm, hm), utilArrayIntersection(bm, hm));
+        symDiff.forEach(id => entityIDs.add(id));
+      });
+
     _headPromise = validateEntitiesAsync(entityIDs, _headCache)
       .then(() => updateResolvedIssues(entityIDs))
       .then(() => dispatch.call('validated'))

test/spec/core/validator.js

@@ -102,4 +102,67 @@ describe('iD.coreValidator', function() {
         issues = validator.getIssues();
         expect(issues).to.have.lengthOf(1);
     });
+
+    it('removes validation issue when untagged way is becomes part of a boundary relation', async () => {
+        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: [4, 6] });
+        var w1 = iD.osmWay({ id: 'w-1', nodes: ['n-1', 'n-2'], tags: { 'note': 'foo' } });
+        var w2 = iD.osmWay({ id: 'w-2', nodes: ['n-2', 'n-3'], tags: {} });
+        var r = iD.osmRelation({ id: 'r-1', members: [{ id: 'w-2' }], tags: { 'type': 'boundary' } });
+        context.perform(
+            iD.actionAddEntity(n1),
+            iD.actionAddEntity(n2),
+            iD.actionAddEntity(n3),
+            iD.actionAddEntity(w1),
+            iD.actionAddEntity(w2),
+            iD.actionAddEntity(r)
+        );
+        var validator = new iD.coreValidator(context);
+        validator.init();
+        await validator.validate();
+        // There should be a validation error about the untagged way
+        let issues = validator.getIssues();
+        expect(issues).to.have.lengthOf(1);
+
+        // add way to relation
+        context.perform(
+            iD.actionAddMember(r.id, { id: w1.id })
+        );
+
+        await validator.validate();
+        // Validation error should be fixed
+        issues = validator.getIssues();
+        expect(issues).to.have.lengthOf(0);
+    });
+
+    it('add validation issue when untagged way is removed from boundary relation', async () => {
+        // A way is "untagged", but part of a larger (boundary) relation
+        var n1 = iD.osmNode({ id: 'n-1', loc: [4, 4] });
+        var n2 = iD.osmNode({ id: 'n-2', loc: [4, 5] });
+        var w1 = iD.osmWay({ id: 'w-1', nodes: ['n-1', 'n-2'], tags: { 'note': 'foo' } });
+        var r = iD.osmRelation({ id: 'r-1', members: [{ id: 'w-1' }], tags: { 'type': 'boundary' } });
+        context.perform(
+            iD.actionAddEntity(n1),
+            iD.actionAddEntity(n2),
+            iD.actionAddEntity(w1),
+            iD.actionAddEntity(r)
+        );
+        var validator = new iD.coreValidator(context);
+        validator.init();
+        await validator.validate();
+        // Should be no errors
+        let issues = validator.getIssues();
+        expect(issues).to.have.lengthOf(0);
+
+        // delete relation
+        context.perform(
+            iD.actionDeleteRelation(r.id)
+        );
+
+        await validator.validate();
+        // Should produce untagged feature error
+        issues = validator.getIssues();
+        expect(issues).to.have.lengthOf(1);
+    });
 });