Remove 'area=yes' when selecting a preset that implies an area already

CorruptComputer · CorruptComputer · commit 77cc5f38beb8 · 2026-04-24T22:14:47.000-05:00
diff --git a/Alidade.Osm/Handlers/Tagging/MergePresetTags.cs b/Alidade.Osm/Handlers/Tagging/MergePresetTags.cs
@@ -0,0 +1,41 @@
+using Alidade.Osm.Models.Tagging;
+
+namespace Alidade.Osm.Handlers.Tagging;
+
+/// <inheritdoc />
+public sealed class MergePresetTags
+    : IRequestHandler<MergePresetTags.Query, QueryResult<Dictionary<string, string>>>
+{
+    /// <summary>
+    ///   Merges a preset's tags over an existing tag dictionary and removes
+    ///   <c>area=yes</c> when it is redundant because an area-implying key
+    ///   (e.g. <c>landuse</c>, <c>building</c>, <c>natural</c>) is already
+    ///   present in the merged result.
+    /// </summary>
+    /// <param name="CurrentTags">The element's current tags, used as the merge base.</param>
+    /// <param name="Preset">The preset whose tags are applied on top.</param>
+    public record Query( IReadOnlyDictionary<string, string> CurrentTags, Preset Preset)
+        : IRequest<QueryResult<Dictionary<string, string>>>;
+
+    /// <inheritdoc />
+    public Task<QueryResult<Dictionary<string, string>>> Handle( Query request, CancellationToken cancellationToken)
+    {
+        Dictionary<string, string> merged = new(request.CurrentTags);
+        foreach ((string k, string v) in request.Preset.Tags)
+        {
+            if (v != "*")
+            {
+                merged[k] = v;
+            }
+        }
+
+        if (merged.TryGetValue("area", out string? areaVal)
+            && areaVal == "yes"
+            && merged.Keys.Any(OsmWay.AreaImplyingKeys.Contains))
+        {
+            merged.Remove("area");
+        }
+
+        return Task.FromResult<QueryResult<Dictionary<string, string>>>(merged);
+    }
+}
diff --git a/Alidade.Osm/Models/OsmWay.cs b/Alidade.Osm/Models/OsmWay.cs
@@ -43,7 +43,7 @@ public sealed record OsmWay(
     public bool IsClosed => NodeIds.Count >= 2 && NodeIds[0] == NodeIds[^1];
 
     // Tag keys that make a closed way implicitly an area even without area=yes.
-    private static readonly HashSet<string> AreaImplyingKeys =
+    internal static readonly HashSet<string> AreaImplyingKeys =
     [
         "building", "building:part", "landuse", "leisure", "natural",
         "amenity", "shop", "place", "man_made", "military", "aeroway",
diff --git a/Alidade/Components/Dialogs/ConflictResolutionDialog.razor.cs b/Alidade/Components/Dialogs/ConflictResolutionDialog.razor.cs
@@ -1,3 +1,5 @@
+using Alidade.Osm.Handlers.Editing;
+
 namespace Alidade.Components.Dialogs;
 
 /// <summary>
diff --git a/Alidade/Components/Dialogs/GridifyDialog.razor.cs b/Alidade/Components/Dialogs/GridifyDialog.razor.cs
@@ -1,5 +1,8 @@
 using System.Text.Json;
+using Alidade.Handlers.Map;
+using Alidade.Handlers.Selection;
 using Alidade.Map.Handlers;
+using Alidade.Osm.Handlers.Editing;
 using Microsoft.AspNetCore.Components;
 using Microsoft.JSInterop;
 using NetTopologySuite.Features;
@@ -10,7 +13,7 @@ namespace Alidade.Components.Dialogs;
 /// <summary>
 ///   Floating, draggable panel for splitting a selected closed way into a grid of equal
 ///   rectangular sub-areas. Shows a live orange dashed preview overlay while open.
-///   Follows the same always-in-DOM, CSS-visibility pattern as <see cref="Alidade.Components.Panels.InspectorPanel"/>.
+///   Follows the same always-in-DOM, CSS-visibility pattern as <see cref="Panels.InspectorPanel"/>.
 /// </summary>
 public partial class GridifyDialog(
     IMediator mediator,
@@ -153,7 +156,7 @@ private async Task PushPreviewAsync()
             return;
         }
 
-        FeatureCollection fc = new();
+        FeatureCollection fc = [];
 
         foreach (IReadOnlyList<GridifyNodeRef> cellRefs in result.CellNodeRefs)
         {
@@ -190,7 +193,7 @@ private async Task PushPreviewAsync()
 
     private async Task ClearPreviewAsync()
     {
-        FeatureCollection empty = new();
+        FeatureCollection empty = [];
         string json = JsonSerializer.Serialize(empty, geoJsonOptions);
         await mediator.Send(new SetSourceData.Command("osm-gridify-preview", json));
     }
@@ -204,14 +207,14 @@ private async Task ApplyAsync()
 
         await ClearPreviewAsync();
         await mediator.Send(new GridifyWay.Command(_wayId.Value, _rows, _cols, _rotation));
-        await mediator.Send(new Handlers.Map.ToggleGridifyDialog.Command());
-        await mediator.Send(new Handlers.Selection.ClearSelection.Command());
+        await mediator.Send(new ToggleGridifyDialog.Command());
+        await mediator.Send(new ClearSelection.Command());
     }
 
     private void Cancel()
     {
         _ = ClearPreviewAsync();
-        _ = mediator.Send(new Handlers.Map.ToggleGridifyDialog.Command());
+        _ = mediator.Send(new ToggleGridifyDialog.Command());
     }
 
     /// <inheritdoc />
diff --git a/Alidade/Components/Panels/InspectorPanel.razor.cs b/Alidade/Components/Panels/InspectorPanel.razor.cs
@@ -3,6 +3,10 @@
 using Alidade.Osm.Models.Nsi;
 using Alidade.Osm.Models.Tagging;
 using Microsoft.AspNetCore.Components.Web;
+using Alidade.Handlers.Map;
+using Alidade.Handlers.Selection;
+using Alidade.Osm.Handlers.Editing;
+using Alidade.Osm.Handlers.Tagging;
 
 namespace Alidade.Components.Panels;
 
@@ -22,7 +26,8 @@ public partial class InspectorPanel(
     /// <summary>
     ///   When set, the panel is pinned to this element and does not follow selection changes.
     /// </summary>
-    [Parameter] public OsmElementRef? PinnedRef { get; set; }
+    [Parameter]
+    public OsmElementRef? PinnedRef { get; set; }
 
     private bool IsPinned => PinnedRef is not null;
 
@@ -259,9 +264,12 @@ OsmElementTypes.Relation when buf.Relations.TryGetValue(_targetRef.Id, out OsmRe
         RefreshPreset();
     }
 
-    private void ApplyPreset(Preset preset)
+    private async Task ApplyPreset(Preset preset)
     {
-        if (_targetRef is null) return;
+        if (_targetRef is null)
+        {
+            return;
+        }
 
         EditBufferState buf = editBufferState.State;
         IReadOnlyDictionary<string, string>? currentTags = _targetRef.Type switch
@@ -272,15 +280,18 @@ OsmElementTypes.Relation when buf.Relations.TryGetValue(_targetRef.Id, out OsmRe
             _ => null
         };
 
-        if (currentTags is null) return;
+        if (currentTags is null)
+        {
+            return;
+        }
 
-        Dictionary<string, string> merged = new(currentTags);
-        foreach ((string k, string v) in preset.Tags)
+        Dictionary<string, string>? merged = await mediator.Send(new MergePresetTags.Query(currentTags, preset));
+        if (merged is null)
         {
-            if (v != "*") merged[k] = v;
+            return;
         }
 
-        _ = mediator.Send(new UpdateTags.Command(_targetRef, currentTags, merged));
+        await mediator.Send(new UpdateTags.Command(_targetRef, currentTags, merged));
         _activePreset = preset;
         _query = string.Empty;
         _isSearching = false;
@@ -292,19 +303,19 @@ private void PinCurrent()
     {
         if (_targetRef is not null)
         {
-            _ = mediator.Send(new Handlers.Map.PinInspector.Command(_targetRef));
+            _ = mediator.Send(new PinInspector.Command(_targetRef));
         }
     }
 
     private void OnClose()
     {
         if (IsPinned)
         {
-            _ = mediator.Send(new Handlers.Map.UnpinInspector.Command(PinnedRef!));
+            _ = mediator.Send(new UnpinInspector.Command(PinnedRef!));
         }
         else
         {
-            _ = mediator.Send(new Handlers.Selection.ClearSelection.Command());
+            _ = mediator.Send(new ClearSelection.Command());
         }
     }
 
diff --git a/Alidade/Components/Panels/PresetField.razor.cs b/Alidade/Components/Panels/PresetField.razor.cs
@@ -1,5 +1,6 @@
 using Microsoft.AspNetCore.Components;
 using Alidade.Osm.Models.Tagging;
+using Alidade.Osm.Handlers.Editing;
 
 namespace Alidade.Components.Panels;
 
diff --git a/Alidade/Components/Panels/PresetPanel.razor.cs b/Alidade/Components/Panels/PresetPanel.razor.cs
@@ -1,3 +1,5 @@
+using Alidade.Osm.Handlers.Editing;
+using Alidade.Osm.Handlers.Tagging;
 using Alidade.Osm.Models.Tagging;
 using Microsoft.AspNetCore.Components;
 
@@ -106,14 +108,10 @@ OsmElementTypes.Relation when buf.Relations.TryGetValue(selected.Id, out OsmRela
             return;
         }
 
-        // Merge preset tags over existing tags and preserve unrelated tags
-        Dictionary<string, string> merged = new(currentTags);
-        foreach ((string k, string v) in preset.Tags)
+        Dictionary<string, string>? merged = await Mediator.Send(new MergePresetTags.Query(currentTags, preset));
+        if (merged is null)
         {
-            if (v != "*")
-            {
-                merged[k] = v;
-            }
+            return;
         }
 
         await Mediator.Send(new UpdateTags.Command(selected, currentTags, merged));
diff --git a/Alidade/Components/Panels/RelationsSection.razor.cs b/Alidade/Components/Panels/RelationsSection.razor.cs
@@ -1,3 +1,4 @@
+using Alidade.Osm.Handlers.Editing;
 using Microsoft.AspNetCore.Components;
 
 namespace Alidade.Components.Panels;
diff --git a/Alidade/Components/Panels/TagEditor.razor.cs b/Alidade/Components/Panels/TagEditor.razor.cs
@@ -1,5 +1,6 @@
 using Microsoft.AspNetCore.Components;
 using Alidade.Osm.Models.Nsi;
+using Alidade.Osm.Handlers.Editing;
 
 namespace Alidade.Components.Panels;
 
diff --git a/Alidade/GlobalUsings.cs b/Alidade/GlobalUsings.cs
@@ -1,31 +1,34 @@
 global using System.Collections.Immutable;
 global using Questy;
+
+global using Alidade.Handlers.EditBuffer;
+global using Alidade.Models;
+global using Alidade.Models.EditBuffer.Draft;
+global using Alidade.Models.Map;
+global using Alidade.Models.Selection;
+global using Alidade.Models.Tool;
+global using Alidade.Models.Validation;
+global using Alidade.PipelineBehaviors;
+global using Alidade.Services;
+global using Alidade.Services.State;
+
 global using Alidade.Core.Consts;
 global using Alidade.Core.Enums;
 global using Alidade.Core.Models.CQRS;
-global using Alidade.Handlers.EditBuffer;
-global using Alidade.Osm.Handlers.Editing;
 global using Alidade.Core.Models.CQRS.Request;
 global using Alidade.Core.Models.CQRS.Response;
 global using Alidade.Core.Models.Settings;
 global using Alidade.Core.ServiceInterface;
 global using Alidade.Core.Services;
+
 global using Alidade.Map.Models;
 global using Alidade.Map.Notifications;
-global using Alidade.Models;
-global using Alidade.Models.EditBuffer.Draft;
-global using Alidade.Osm.Models.EditBuffer;
-global using Alidade.Models.Map;
-global using Alidade.Models.Selection;
-global using Alidade.Models.Tool;
-global using Alidade.Models.Validation;
-global using Alidade.Osm.Services;
-global using Alidade.Osm.Services.State;
-global using Alidade.Osm.Models.Api.Auth;
+
 global using Alidade.Osm.Enums;
 global using Alidade.Osm.Models;
+global using Alidade.Osm.Models.Api.Auth;
+global using Alidade.Osm.Models.EditBuffer;
 global using Alidade.Osm.Models.Validation;
+global using Alidade.Osm.Services;
+global using Alidade.Osm.Services.State;
 global using Alidade.Osm.Validators;
-global using Alidade.PipelineBehaviors;
-global using Alidade.Services;
-global using Alidade.Services.State;
diff --git a/Alidade/Handlers/Selection/DeleteSelected.cs b/Alidade/Handlers/Selection/DeleteSelected.cs
@@ -1,3 +1,5 @@
+using Alidade.Osm.Handlers.Editing;
+
 namespace Alidade.Handlers.Selection;
 
 /// <inheritdoc />
diff --git a/Alidade/Handlers/Tool/Circularize.cs b/Alidade/Handlers/Tool/Circularize.cs
@@ -1,3 +1,5 @@
+using Alidade.Osm.Handlers.Editing;
+
 namespace Alidade.Handlers.Tool;
 
 /// <inheritdoc />
diff --git a/Alidade/Handlers/Tool/Orthogonalize.cs b/Alidade/Handlers/Tool/Orthogonalize.cs
@@ -1,3 +1,5 @@
+using Alidade.Osm.Handlers.Editing;
+
 namespace Alidade.Handlers.Tool;
 
 /// <inheritdoc />
diff --git a/Alidade/Services/DrawingToolService.cs b/Alidade/Services/DrawingToolService.cs
@@ -1,3 +1,5 @@
+using Alidade.Osm.Handlers.Editing;
+
 namespace Alidade.Services;
 
 /// <summary>
diff --git a/Alidade/Services/EditBufferService.cs b/Alidade/Services/EditBufferService.cs
@@ -1,5 +1,6 @@
 using System.Text.Json;
 using Alidade.Map.Handlers;
+using Alidade.Osm.Handlers.Editing;
 using Alidade.Osm.Models.Editing;
 using NetTopologySuite.Features;
 using NetTopologySuite.Geometries;
diff --git a/Tests/Alidade.Osm.Tests/Handlers/Tagging/MergePresetTagsTests.cs b/Tests/Alidade.Osm.Tests/Handlers/Tagging/MergePresetTagsTests.cs
@@ -0,0 +1,72 @@
+using Alidade.Core.Models.CQRS.Response;
+using Alidade.Osm.Handlers.Tagging;
+using Alidade.Osm.Models.Tagging;
+using Autofac;
+using Questy;
+using Xunit;
+
+namespace Alidade.Osm.Tests.Handlers.Tagging;
+
+public class MergePresetTagsTests(OsmMediatorFixture fixture) : IClassFixture<OsmMediatorFixture>
+{
+    private static Preset MakePreset(params (string k, string v)[] tags)
+        => new(
+            Id: "test/preset", Icon: null,
+            Fields: [], MoreFields: [],
+            Geometry: ["area"],
+            Tags: tags.ToDictionary(t => t.k, t => t.v),
+            MatchScore: 1.0, Name: null,
+            Terms: [], Aliases: [],
+            Searchable: true);
+
+    private Task<QueryResult<Dictionary<string, string>>> Send(
+        IReadOnlyDictionary<string, string> currentTags, Preset preset)
+    {
+        using ILifetimeScope scope = fixture.Container.BeginLifetimeScope();
+        ISender sender = scope.Resolve<ISender>();
+        return sender.Send(new MergePresetTags.Query(currentTags, preset));
+    }
+
+    [Fact]
+    public async Task AreaImplyingKeyAdded_RemovesAreaYes()
+    {
+        Dictionary<string, string> current = new() { ["area"] = "yes" };
+        QueryResult<Dictionary<string, string>> result = await Send(current, MakePreset(("landuse", "grass")));
+        Assert.Equal("grass", result.Result!["landuse"]);
+        Assert.False(result.Result!.ContainsKey("area"));
+    }
+
+    [Fact]
+    public async Task NoAreaYes_MergesCleanly()
+    {
+        Dictionary<string, string> current = new() { ["name"] = "Park" };
+        QueryResult<Dictionary<string, string>> result = await Send(current, MakePreset(("landuse", "grass")));
+        Assert.Equal("grass", result.Result!["landuse"]);
+        Assert.Equal("Park", result.Result!["name"]);
+    }
+
+    [Fact]
+    public async Task AreaNo_IsNotRemoved()
+    {
+        Dictionary<string, string> current = new() { ["area"] = "no" };
+        QueryResult<Dictionary<string, string>> result = await Send(current, MakePreset(("landuse", "grass")));
+        Assert.Equal("no", result.Result!["area"]);
+    }
+
+    [Fact]
+    public async Task WildcardValue_IsNotApplied()
+    {
+        Dictionary<string, string> current = new() { ["shop"] = "bakery" };
+        QueryResult<Dictionary<string, string>> result = await Send(current, MakePreset(("shop", "*")));
+        Assert.Equal("bakery", result.Result!["shop"]);
+    }
+
+    [Fact]
+    public async Task AreaImplyingKeyInCurrentTags_RemovesAreaYes()
+    {
+        // area=yes is redundant even when the implying key comes from currentTags, not the preset
+        Dictionary<string, string> current = new() { ["building"] = "yes", ["area"] = "yes" };
+        QueryResult<Dictionary<string, string>> result = await Send(current, MakePreset(("name", "Town Hall")));
+        Assert.False(result.Result!.ContainsKey("area"));
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ public sealed record OsmWay(`
`43`	`43`	`public bool IsClosed => NodeIds.Count >= 2 && NodeIds[0] == NodeIds[^1];`
`44`	`44`
`45`	`45`	`// Tag keys that make a closed way implicitly an area even without area=yes.`
`46`		`- private static readonly HashSet<string> AreaImplyingKeys =`
	`46`	`+ internal static readonly HashSet<string> AreaImplyingKeys =`
`47`	`47`	`[`
`48`	`48`	`"building", "building:part", "landuse", "leisure", "natural",`
`49`	`49`	`"amenity", "shop", "place", "man_made", "military", "aeroway",`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+using Alidade.Osm.Handlers.Editing;`
	`2`	`+`
`1`	`3`	`namespace Alidade.Components.Dialogs;`
`2`	`4`
`3`	`5`	`/// <summary>`
Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,10 @@`
`3`	`3`	`using Alidade.Osm.Models.Nsi;`
`4`	`4`	`using Alidade.Osm.Models.Tagging;`
`5`	`5`	`using Microsoft.AspNetCore.Components.Web;`
	`6`	`+using Alidade.Handlers.Map;`
	`7`	`+using Alidade.Handlers.Selection;`
	`8`	`+using Alidade.Osm.Handlers.Editing;`
	`9`	`+using Alidade.Osm.Handlers.Tagging;`
`6`	`10`
`7`	`11`	`namespace Alidade.Components.Panels;`
`8`	`12`
`@@ -22,7 +26,8 @@ public partial class InspectorPanel(`
`22`	`26`	`/// <summary>`
`23`	`27`	`/// When set, the panel is pinned to this element and does not follow selection changes.`
`24`	`28`	`/// </summary>`
`25`		`- [Parameter] public OsmElementRef? PinnedRef { get; set; }`
	`29`	`+ [Parameter]`
	`30`	`+ public OsmElementRef? PinnedRef { get; set; }`
`26`	`31`
`27`	`32`	`private bool IsPinned => PinnedRef is not null;`
`28`	`33`
`@@ -259,9 +264,12 @@ OsmElementTypes.Relation when buf.Relations.TryGetValue(_targetRef.Id, out OsmRe`
`259`	`264`	`RefreshPreset();`
`260`	`265`	`}`
`261`	`266`
`262`		`- private void ApplyPreset(Preset preset)`
	`267`	`+ private async Task ApplyPreset(Preset preset)`
`263`	`268`	`{`
`264`		`- if (_targetRef is null) return;`
	`269`	`+ if (_targetRef is null)`
	`270`	`+ {`
	`271`	`+ return;`
	`272`	`+ }`
`265`	`273`
`266`	`274`	`EditBufferState buf = editBufferState.State;`
`267`	`275`	`IReadOnlyDictionary<string, string>? currentTags = _targetRef.Type switch`
`@@ -272,15 +280,18 @@ OsmElementTypes.Relation when buf.Relations.TryGetValue(_targetRef.Id, out OsmRe`
`272`	`280`	`_ => null`
`273`	`281`	`};`
`274`	`282`
`275`		`- if (currentTags is null) return;`
	`283`	`+ if (currentTags is null)`
	`284`	`+ {`
	`285`	`+ return;`
	`286`	`+ }`
`276`	`287`
`277`		`- Dictionary<string, string> merged = new(currentTags);`
`278`		`- foreach ((string k, string v) in preset.Tags)`
	`288`	`+ Dictionary<string, string>? merged = await mediator.Send(new MergePresetTags.Query(currentTags, preset));`
	`289`	`+ if (merged is null)`
`279`	`290`	`{`
`280`		`- if (v != "*") merged[k] = v;`
	`291`	`+ return;`
`281`	`292`	`}`
`282`	`293`
`283`		`- _ = mediator.Send(new UpdateTags.Command(_targetRef, currentTags, merged));`
	`294`	`+ await mediator.Send(new UpdateTags.Command(_targetRef, currentTags, merged));`
`284`	`295`	`_activePreset = preset;`
`285`	`296`	`_query = string.Empty;`
`286`	`297`	`_isSearching = false;`
`@@ -292,19 +303,19 @@ private void PinCurrent()`
`292`	`303`	`{`
`293`	`304`	`if (_targetRef is not null)`
`294`	`305`	`{`
`295`		`- _ = mediator.Send(new Handlers.Map.PinInspector.Command(_targetRef));`
	`306`	`+ _ = mediator.Send(new PinInspector.Command(_targetRef));`
`296`	`307`	`}`
`297`	`308`	`}`
`298`	`309`
`299`	`310`	`private void OnClose()`
`300`	`311`	`{`
`301`	`312`	`if (IsPinned)`
`302`	`313`	`{`
`303`		`- _ = mediator.Send(new Handlers.Map.UnpinInspector.Command(PinnedRef!));`
	`314`	`+ _ = mediator.Send(new UnpinInspector.Command(PinnedRef!));`
`304`	`315`	`}`
`305`	`316`	`else`
`306`	`317`	`{`
`307`		`- _ = mediator.Send(new Handlers.Selection.ClearSelection.Command());`
	`318`	`+ _ = mediator.Send(new ClearSelection.Command());`
`308`	`319`	`}`
`309`	`320`	`}`
`310`	`321`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,4 @@`
	`1`	`+using Alidade.Osm.Handlers.Editing;`
`1`	`2`	`using Microsoft.AspNetCore.Components;`
`2`	`3`
`3`	`4`	`namespace Alidade.Components.Panels;`