Fix erroneous mismatched_geometry warning for preset-less tags

re: #1626 re: openstreetmap/iD#10522 re: openstreetmap/iD#10523
facebook · Dec 12, 2024 · 7b115b8 · 7b115b8
1 parent 8843b93
commit 7b115b8
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 9 deletions.
diff --git a/modules/validations/mismatched_geometry.js b/modules/validations/mismatched_geometry.js
@@ -36,6 +36,12 @@ export function validationMismatchedGeometry(context) {
             // If the presets match something like '*', (e.g. attraction), ignore
             const key = Object.keys(tagSuggestingArea)[0];
             if (linePreset.tags[key] === '*' || areaPreset.tags[key === '*']) return null;
+
+            // If the entity matches the fallback preset, regardless of the
+            // geometry, then changing the geometry will not help.  iD#10523
+            if (linePreset.isFallback() && areaPreset.isFallback() && !deepEqual(tagSuggestingArea, { area: 'yes' })) {
+              return null;
+            }
         }
 
         return tagSuggestingArea;

diff --git a/test/browser/validations/mismatched_geometry.js b/test/browser/validations/mismatched_geometry.js
@@ -1,5 +1,6 @@
 describe('validationMismatchedGeometry', () => {
   let graph;
+  let _savedAreaKeys;
 
   class MockLocalizationSystem {
     constructor() {}
@@ -11,12 +12,28 @@ describe('validationMismatchedGeometry', () => {
     constructor() {}
   }
 
+  class MockStorageSystem {
+    constructor() { }
+    getItem() { return ''; }
+  }
+
+  class MockUrlSystem {
+    constructor() {
+      this.initialHashParams = new Map();
+    }
+    initAsync()   { return Promise.resolve(); }
+    on()          { return this; }
+  }
+
   class MockContext {
     constructor() {
       this.systems = {
+        assets:     new Rapid.AssetSystem(this),
         l10n:       new MockLocalizationSystem(),
         locations:  new MockLocationSystem(),
-        presets:    new Rapid.PresetSystem(this)
+        presets:    new Rapid.PresetSystem(this),
+        storage:    new MockStorageSystem(),
+        urlhash:    new MockUrlSystem()
       };
     }
   }
@@ -26,9 +43,25 @@ describe('validationMismatchedGeometry', () => {
 
   beforeEach(() => {
     graph = new Rapid.Graph();     // reset
+    _savedAreaKeys = Rapid.osmAreaKeys;
+
+    // For the tests that need a building preset
+    const testPresets = {
+      building: {
+        tags: { building: '*' },
+        geometry: ['area']
+      }
+    };
+    context.systems.assets._cache.tagging_preset_presets = testPresets;
+  });
+
+  afterEach(() => {
+    Rapid.osmSetAreaKeys(_savedAreaKeys);
+    context.systems.assets._cache.tagging_preset_presets = {};
   });
 
 
+
   function validate() {
     const entities = [ ...graph.base.entities.values() ];
 
@@ -110,16 +143,32 @@ describe('validationMismatchedGeometry', () => {
   });
 
   it('flags open way with area tag', () => {
+    // In this test case, `building=yes` suggests area, and we should match the 'building' preset.
     Rapid.osmSetAreaKeys({ building: {} });
-    createOpenWay({ building: 'yes' });
-    const issues = validate();
-    expect(issues).to.have.lengthOf(1);
+    const presets = context.systems.presets;
+    return presets.initAsync().then(() => {
+      createOpenWay({ building: 'yes' });
+      const issues = validate();
+      expect(issues).to.have.lengthOf(1);
+
+      const issue = issues[0];
+      expect(issue.type).to.eql('mismatched_geometry');
+      expect(issue.subtype).to.eql('area_as_line');
+      expect(issue.severity).to.eql('warning');
+      expect(issue.entityIds).to.eql(['w-1']);
+    });
+  });
 
-    const issue = issues[0];
-    expect(issue.type).to.eql('mismatched_geometry');
-    expect(issue.subtype).to.eql('area_as_line');
-    expect(issue.severity).to.eql('warning');
-    expect(issue.entityIds).to.eql(['w-1']);
+  it('does not flag cases whether the entity matches the generic preset, regardless of geometry', () => {
+    // In this test case, `waterway=yes` suggests area, but we won't match any presets.
+    // There is no preset for `waterway=security_lock`, so it matches fallback presets for both line and area.
+    Rapid.osmSetAreaKeys({ waterway: { dam: true } });
+    const presets = context.systems.presets;
+    return presets.initAsync().then(() => {
+      createOpenWay({ 'disused:waterway': 'security_lock' });
+      const issues = validate();
+      expect(issues).to.have.lengthOf(0);
+    });
   });
 
   it('flags open way with both area and line tags', () => {