Suggest adding an entrance=* node to fix 'disconnected highway' errors

CHANGELOG.md

@@ -45,6 +45,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
 #### :white_check_mark: Validation
 * Add warning if aeroways cross each other, buildings or highways ([#9315], thanks [@k-yle])
 * 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])
+* Suggest adding an `entrance=*` node to fix 'disconnected highway' errors ([#10729], thanks [@k-yle])
 #### :bug: Bugfixes
 * Prevent degenerate ways caused by deleting a corner of a triangle ([#10003], thanks [@k-yle])
 * Fix briefly disappearing data layer during background layer tile layer switching transition ([#10748])
@@ -60,6 +61,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
 [#7381]: https://github.com/openstreetmap/iD/issues/7381
 [#10003]: https://github.com/openstreetmap/iD/pull/10003
 [#10720]: https://github.com/openstreetmap/iD/issues/10720
+[#10729]: https://github.com/openstreetmap/iD/pull/10729
 [#10747]: https://github.com/openstreetmap/iD/issues/10747
 [#10748]: https://github.com/openstreetmap/iD/issues/10748
 [#10755]: https://github.com/openstreetmap/iD/issues/10755
@@ -92,9 +94,9 @@ _Breaking developer changes, which may affect downstream projects or sites that
 #### :white_check_mark: Validation
 * Include wikidata errors from osmose QA service ([#9998], thanks [@k-yle])
 #### :bug: Bugfixes
-* Fix unsolvable validator error triggered by regional presets ([#10459])
+* Fix unsolvable validator error triggered by regional presets ([#10459], thanks [@k-yle])
 * Render highway direction cones only on matching parent ways ([#9013])
-* Prevent edit menu from being covered up by street level imagery or other map overlay panels ([#10495])
+* Prevent edit menu from being covered up by street level imagery or other map overlay panels ([#10495], thanks [@fahnestd])
 * Fix grid lines from showing up on background map tiles in certain situations (semi-transparent tiles or fractional browser zoom level) ([#10594], thanks [@Nekzuris])
 * Prevent search results from sometimes getting stuck in the highlighted state when mouse-hovering the list of search results while typing ([#10661])
 * Allow tiles in minimap to be slightly underzoomed, preventing them from blacking out on low map zoom levels ([#10653])
@@ -144,6 +146,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
 [@winstonsung]: https://github.com/winstonsung/
 [@Nekzuris]: https://github.com/Nekzuris
 [@michaelabon]: https://github.com/michaelabon
+[@fahnestd]: https://github.com/fahnestd
 
 
 # 2.30.4

data/core.yaml

@@ -1988,6 +1988,8 @@ en:
         title: Continue drawing from start
       continue_from_end:
         title: Continue drawing from end
+      tag_as_entrance:
+        title: Mark the endpoint as “{presetName}”
       convert_to_line:
         title: Convert this to a line
         annotation: Converted an area to a line.

modules/validations/disconnected_way.js

@@ -2,9 +2,11 @@ import { t, localizer } from '../core/localizer';
 import { modeDrawLine } from '../modes/draw_line';
 import { operationDelete } from '../operations/delete';
 import { utilDisplayLabel } from '../util/utilDisplayLabel';
-import { osmRoutableHighwayTagValues } from '../osm/tags';
+import { osmPathHighwayTagValues, osmRoutableHighwayTagValues } from '../osm/tags';
 import { validationIssue, validationIssueFix } from '../core/validation';
 import { services } from '../services';
+import { presetManager } from '../presets';
+import { actionChangeTags } from '../actions';
 
 export function validationDisconnectedWay() {
     var type = 'disconnected_way';
@@ -45,6 +47,8 @@ export function validationDisconnectedWay() {
 
                     var textDirection = localizer.textDirection();
 
+                    fixes.push(...makeTagAsEntranceFixIfAllowed(singleEntity));
+
                     var startFix = makeContinueDrawingFixIfAllowed(textDirection, singleEntity.first(), 'start');
                     if (startFix) fixes.push(startFix);
 
@@ -207,6 +211,52 @@ export function validationDisconnectedWay() {
             });
         }
 
+        /** @param {osmWay} highway */
+        function makeTagAsEntranceFixIfAllowed(highway) {
+            const fixes = [];
+            for (const vertexId of [highway.first(), highway.last()]) {
+                const vertex = graph.hasEntity(vertexId);
+                if (!vertex) continue; // not downloaded
+
+                const parents = graph.parentWays(vertex);
+
+                const isConnectedToBuilding = parents.some(way =>
+                    way.id !== highway.id &&
+                    way.tags.building &&
+                    way.tags.building !== 'no'
+                );
+
+                // ineligible for this fix
+                if (!isConnectedToBuilding) continue;
+
+                // this autofix is only supported for paths
+                if (!(highway.tags.highway in osmPathHighwayTagValues)) continue;
+
+                const entranceTags = { entrance: 'yes' };
+
+                const presetName = presetManager.matchTags(entranceTags, 'vertex').name();
+
+                fixes.push(
+                    new validationIssueFix({
+                        icon: 'iD-icon-wrench',
+                        title: t.append('issues.fix.tag_as_entrance.title', { presetName }),
+                        entityIds: [vertexId],
+                        onClick: function(context) {
+                            const newTags = {
+                                ...graph.entity(vertexId).tags,
+                                ...entranceTags,
+                            };
+                            context.perform(
+                                actionChangeTags(vertexId, newTags),
+                                t('operations.change_tags.annotation')
+                            );
+                        }
+                    })
+                );
+            }
+
+            return fixes;
+        }
     };
 
     validation.type = type;

test/spec/validations/disconnected_way.js

@@ -59,6 +59,13 @@ describe('iD.validations.disconnected_way', function () {
         expect(issue.severity).to.eql('warning');
         expect(issue.entityIds).to.have.lengthOf(1);
         expect(issue.entityIds[0]).to.eql('w-1');
+
+        // dynamicFixes don't have IDs, but the icons can identify the dynamicFix
+        expect(issue.dynamicFixes(context).map(fix => fix.icon)).toStrictEqual([
+            'iD-operation-continue-left',
+            'iD-operation-continue',
+            'iD-operation-delete',
+        ]);
     });
 
     it('flags highway connected only to service area', function() {
@@ -93,4 +100,42 @@ describe('iD.validations.disconnected_way', function () {
         expect(issues).to.have.lengthOf(0);
     });
 
+    describe.each([
+        // ['unclassified', { amenity: 'parking_entrance' }],
+        ['path', { entrance: 'yes' }]
+    ])('highway=%s -> %s', (highway, entranceTags) => {
+        it('suggests adding an entrance node if the isolated highway network touches a building outline', () => {
+            const n1 = iD.osmNode({ id: 'n-1', loc: [1, 1] });
+            const n2 = iD.osmNode({ id: 'n-2', loc: [2, 2] });
+            const n3 = iD.osmNode({ id: 'n-3', loc: [3, 3] });
+            const n4 = iD.osmNode({ id: 'n-4', loc: [4, 4] });
+            const w1 = iD.osmWay({ id: 'w-1', nodes: ['n-1', 'n-2'], tags: { highway } });
+            const w2 = iD.osmWay({ id: 'w-2', nodes: ['n-1', 'n-3', 'n-4', 'n-1'], tags: { building: 'yes'} });
+
+            context.perform(
+                iD.actionAddEntity(n1),
+                iD.actionAddEntity(n2),
+                iD.actionAddEntity(n3),
+                iD.actionAddEntity(n4),
+                iD.actionAddEntity(w1),
+                iD.actionAddEntity(w2),
+            );
+
+            const issues = validate();
+            expect(issues).toHaveLength(1);
+
+            // dynamicFixes don't have IDs, but the icons can identify the dynamicFix
+            expect(issues[0].dynamicFixes(context).map(fix => fix.icon)).toStrictEqual([
+                'iD-icon-wrench',
+                'iD-operation-continue-left',
+                'iD-operation-continue',
+                'iD-operation-delete',
+            ]);
+
+            // run the 'Tag as Entrance' autofix
+            issues[0].dynamicFixes(context)[0].onClick(context);
+
+            expect(context.graph().entity('n-1').tags).toStrictEqual(entranceTags);
+        });
+    });
 });