diff --git a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestSectionsTests.cs b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestSectionsTests.cs index df63ef9e..cb2430a6 100644 --- a/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestSectionsTests.cs +++ b/src/dotnet/Modgud.Api.Tests/ColdStart/RealmManifestSectionsTests.cs @@ -239,6 +239,72 @@ await InTenantAsync(factory, slug, async sp => Assert.DoesNotContain(afterPrune.ApplicationDomains, kv => kv.Value == shopAppId); } + [Fact] + public async Task Turning_off_a_per_app_settings_section_shows_in_the_plan_as_the_clear_it_is() + { + await using var host = await Fixture.CreateIsolatedHostAsync(); + var factory = host.Factory; + var ct = TestContext.Current.CancellationToken; + var applier = factory.Services.GetRequiredService(); + var planner = factory.Services.GetRequiredService(); + + const string slug = "appclear"; + var appId = new ShortGuid(Guid.NewGuid()).ToString(); + RealmManifest Manifest(ApplicationSettingsDto settings) => new() + { + Apps = + [ + new RealmManifestApp + { + Slug = "shop", Id = appId, DisplayName = "Shop", + Permissions = [new RealmManifestPermission("order", "read")], + Settings = settings, + }, + ], + }; + + Assert.False((await ProvisionRealmAsync(factory, Shell(slug), Manifest(new ApplicationSettingsDto + { + Origin = new ApplicationOriginDto { Subdomain = $"shop.{slug}.localhost" }, + Branding = new ApplicationBrandingDto { ProductName = "Shop!" }, + }), ct)).IsError); + + // What the admin UI stages when both toggles go off: the section objects are the + // COMPLETE desired override state, so a cleared subdomain is an explicit null and a + // switched-off section is a null section. Both mean "clear" to the apply — and the + // plan has to say so, or the draft looks empty and the admin applies blind. + var cleared = Manifest(new ApplicationSettingsDto + { + Origin = new ApplicationOriginDto { Subdomain = null }, + Branding = null, + }); + + var plan = await planner.PlanAsync(slug, cleared, prune: false, ct: ct); + Assert.False(plan.IsError, plan.IsError ? plan.FirstError.Description : string.Empty); + var entry = Assert.Single(plan.Value.Sections.Single(s => s.Name == "apps").Entries); + Assert.Equal("update", entry.Action); + Assert.Contains(entry.Changes, c => c.Field == "Settings.Origin.Subdomain"); + Assert.Contains(entry.Changes, c => c.Field == "Settings.Branding"); + + // …and the apply really does clear both, including the global host route. + Assert.False((await applier.UpdateRealmAsync(slug, cleared, ct: ct)).IsError); + await InTenantAsync(factory, slug, async sp => + { + var settings = await sp.GetRequiredService() + .GetAsync(new ShortGuid(appId).Guid, ct); + Assert.False(settings.IsError); + Assert.Null(settings.Value.Origin?.Subdomain); + Assert.Null(settings.Value.Branding); + }); + var realm = (await factory.Services.GetRequiredService() + .GetRealmBySlugAsync(slug, ct))!; + Assert.DoesNotContain(realm.ApplicationDomains, kv => kv.Key == $"shop.{slug}.localhost"); + + // Re-planning the cleared state is now genuinely unchanged (no phantom diff). + var again = await planner.PlanAsync(slug, cleared, prune: false, ct: ct); + Assert.Equal("unchanged", Assert.Single(again.Value.Sections.Single(s => s.Name == "apps").Entries).Action); + } + [Fact] public async Task Positions_are_feature_gated_and_import_apply_export_prune() { diff --git a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs index 5ca97547..10d0edff 100644 --- a/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs +++ b/src/dotnet/Modgud.Api/Features/Admin/Provisioning/RealmManifestPlanner.cs @@ -1,5 +1,7 @@ +using System.Runtime.CompilerServices; using System.Text.Json; using System.Text.Json.Nodes; +using System.Text.Json.Serialization; using ErrorOr; using Marten; using Microsoft.AspNetCore.Http.Json; @@ -141,7 +143,9 @@ List CanonApps(List apps) => [.. apps.Select KeyField = "Slug", PinnedId = a => a.Id, PinnedIdCheck = PinnedIdLookup(session, a => a.IsDeleted, a => a.Slug, ct), - NestedPatch = ["Settings"], + // REPLACE, not merge-patch: AppAdminService rebuilds the whole override from + // this object, so a null section clears it (→ inherit the realm). + NestedReplace = ["Settings"], DeleteNote = "Deleting fails at apply while the app is still referenced by a kept role, API or scope.", })); @@ -807,10 +811,18 @@ private sealed class SectionPolicy where T : class /// this entry (compared case-insensitively, matching the applier). public HashSet ImmutableFails { get; init; } = []; - /// Nested option-objects with their own patch semantics (app Settings, - /// position TerminalPolicy) — diffed recursively with dotted paths. + /// Nested option-objects with MERGE-PATCH semantics (a position's + /// TerminalPolicy) — diffed recursively with dotted paths; an absent property means + /// "unchanged", so only present ones are compared. public HashSet NestedPatch { get; init; } = []; + /// Nested objects with REPLACE semantics (an App's Settings): the + /// object is the COMPLETE desired override state, so a null section means "clear + /// it" rather than "leave it alone". Diffed like but off a + /// serialization that KEEPS nulls — otherwise turning a section off is invisible in + /// the plan while the apply happily clears it. + public HashSet NestedReplace { get; init; } = []; + /// Extra note appended to every delete candidate of the section. public string? DeleteNote { get; init; } @@ -1106,6 +1118,22 @@ private static RealmPlanEntry DiffEntry( ? null : JsonSerializer.SerializeToNode(baselineItem, json)!.AsObject(); + // A REPLACE section is serialized a second time with nulls KEPT: in the options + // above an explicit null is written as nothing at all, which is exactly right where + // absent means "unchanged" — and exactly wrong here, where it means "clear". + JsonObject? desiredKeepNulls = null, currentKeepNulls = null, baselineKeepNulls = null; + if (policy.NestedReplace.Count > 0) + { + var keepNulls = WithNulls(json); + desiredKeepNulls = JsonSerializer.SerializeToNode(item, keepNulls)!.AsObject(); + currentKeepNulls = existing is null + ? null + : JsonSerializer.SerializeToNode(existing, keepNulls)!.AsObject(); + baselineKeepNulls = baselineItem is null + ? null + : JsonSerializer.SerializeToNode(baselineItem, keepNulls)!.AsObject(); + } + var entry = new RealmPlanEntry { Key = key, Action = existing is null ? "create" : "update" }; var failed = false; @@ -1154,6 +1182,25 @@ private static RealmPlanEntry DiffEntry( // On create, an explicit null / empty list only restates the shipped default. if (currentNode is null && !Carries(value)) continue; + if (policy.NestedReplace.Contains(field)) + { + // The object itself still follows the outer contract: absent (or null) means + // "don't touch this App's override at all". Present means "this IS the + // override", and every section inside then carries — nulls included. + if (desiredKeepNulls?[field] is not JsonObject replace) continue; + if (currentNode is null) + { + entry.Changes.Add(new RealmPlanChange(field, null, replace.DeepClone())); + continue; + } + NestedPatchDiff(field, replace, currentKeepNulls?[field] as JsonObject ?? new JsonObject(), + conflictMode && baselineKeepNulls is not null + ? baselineKeepNulls[field] as JsonObject ?? new JsonObject() + : null, + entry.Changes, entry.Conflicts); + continue; + } + if (policy.NestedPatch.Contains(field)) { if (value is not JsonObject nested) continue; @@ -1238,6 +1285,19 @@ private static void NestedPatchDiff( } } + /// The diff options with WhenWritingNull lifted, for REPLACE sections + /// whose nulls are the payload. Cached per options instance — building a + /// is expensive and the diff runs per entity. + /// Optionals keep their absence: the resolver suppresses a None through + /// ShouldSerialize, independently of the ignore condition. + private static readonly ConditionalWeakTable KeepNullOptions = new(); + + private static JsonSerializerOptions WithNulls(JsonSerializerOptions json) + => KeepNullOptions.GetValue(json, source => new JsonSerializerOptions(source) + { + DefaultIgnoreCondition = JsonIgnoreCondition.Never, + }); + // ── Canonical JSON comparison — null == missing, arrays are order-insensitive // multisets, object properties with null values are treated as absent. ────────── diff --git a/src/frontend-vue/public/i18n/de.json b/src/frontend-vue/public/i18n/de.json index 9e2f3c05..5899cfd9 100644 --- a/src/frontend-vue/public/i18n/de.json +++ b/src/frontend-vue/public/i18n/de.json @@ -2571,7 +2571,8 @@ "admin.appSettings.inherit": "(vom Realm erben)", "admin.appSettings.posture.off": "Off — keine Selbstregistrierung", "admin.appSettings.hint": "Diese Einstellungen überschreiben die Realm-Defaults nur für diese App. Ein deaktivierter Abschnitt erbt vom Realm.", - "admin.appSettings.origin.override": "Eigene Subdomain für diese App", + "admin.appSettings.origin.subdomain": "Eigene Subdomain (Child der Realm-Primary-Domain)", + "admin.appSettings.origin.subdomainHint": "Ein Wert leitet diesen Host auf die App. Leer = keine eigene Subdomain, die App wird über die Realm-URL erreicht — das Feld zu leeren entfernt die bestehende Route.", "admin.appSettings.branding.primaryColor": "Primärfarbe (CSS)", "admin.appSettings.branding.override": "Eigenes Branding (Login/SPA)", "admin.appSettings.email.override": "Eigenes E-Mail-Branding", diff --git a/src/frontend-vue/src/views/admin/apps/AppSettingsSections.vue b/src/frontend-vue/src/views/admin/apps/AppSettingsSections.vue index a4e44845..97125e51 100644 --- a/src/frontend-vue/src/views/admin/apps/AppSettingsSections.vue +++ b/src/frontend-vue/src/views/admin/apps/AppSettingsSections.vue @@ -47,8 +47,13 @@ const loginProviderOptions = ref<{ value: string; label: string }[]>([]) // Per-section "override" toggle (on → this App overrides the realm; off → inherit). // Numbers are kept as strings (empty = inherit that field). +// +// Origin has NO toggle: the section is a single value, so a toggle would say exactly +// what an empty field already says — and two controls for one fact can contradict each +// other (ticked but empty). The subdomain IS the switch: a value routes the host, empty +// drops the route. const f = reactive({ - origin: { override: false, subdomain: '' }, + origin: { subdomain: '' }, branding: { override: false, productName: '', primaryColor: '', logoAssetId: null as string | null, logoUrl: null as string | null, @@ -93,8 +98,14 @@ const f = reactive({ reservedNames: [] as string[], perIp: '', perRealm: '', }, cimd: { override: false, enabled: false, access: '', refresh: '' }, + // ChangeFeed has no override toggle either: "Enabled" already IS the switch — a stored + // section with Enabled=false and no section at all mean the same thing to the reader + // (AppChangeFeedSubscription falls back to ApplicationChangeFeedSettings.Disabled). + // `configured` is not a control, it remembers whether this App already HAS a section, so + // switching the feed off keeps the retention numbers instead of silently dropping them. changeFeed: { enabled: false, + configured: false, retentionAgeDays: 7 as number | null, minimumEventCount: 1000 as number | null, }, @@ -139,8 +150,6 @@ const appRateLimitModeOptions = computed(() => [ const inh = computed(() => { const r = realmSettingsStore.settings return { - // No realm equivalent — inheriting means "realm primary domain / realm default". - origin: { subdomain: '' }, branding: { productName: r?.Branding?.ProductName ?? '', primaryColor: r?.Branding?.PrimaryColor ?? '', @@ -222,7 +231,7 @@ function fieldBind(section: string, field: string): any { } function resetForm() { - f.origin.override = false; f.origin.subdomain = '' + f.origin.subdomain = '' f.branding.override = false; f.branding.productName = ''; f.branding.primaryColor = '' f.branding.logoAssetId = null; f.branding.faviconAssetId = null f.branding.logoUrl = null; f.branding.faviconUrl = null @@ -247,13 +256,14 @@ function resetForm() { f.dcr.override = false; f.dcr.enabled = false; f.dcr.access = ''; f.dcr.refresh = '' f.dcr.reservedNames = []; f.dcr.perIp = ''; f.dcr.perRealm = '' f.cimd.override = false; f.cimd.enabled = false; f.cimd.access = ''; f.cimd.refresh = '' - f.changeFeed.enabled = false; f.changeFeed.retentionAgeDays = 7; f.changeFeed.minimumEventCount = 1000 + f.changeFeed.enabled = false; f.changeFeed.configured = false + f.changeFeed.retentionAgeDays = 7; f.changeFeed.minimumEventCount = 1000 } function populate(s?: ApplicationSettingsDto | null) { resetForm() if (!s) return - if (s.Origin) { f.origin.override = true; f.origin.subdomain = s.Origin.Subdomain ?? '' } + f.origin.subdomain = s.Origin?.Subdomain ?? '' if (s.Branding) { f.branding.override = true f.branding.productName = s.Branding.ProductName ?? '' @@ -343,6 +353,7 @@ function populate(s?: ApplicationSettingsDto | null) { } if (s.ChangeFeed) { f.changeFeed.enabled = s.ChangeFeed.Enabled + f.changeFeed.configured = true f.changeFeed.retentionAgeDays = s.ChangeFeed.MinimumRetentionAgeDays ?? 7 f.changeFeed.minimumEventCount = s.ChangeFeed.MinimumEventCount ?? 1000 } @@ -426,10 +437,10 @@ const emailPreviewOverlay = computed(() => ({ /** Build the override DTO as the COMPLETE desired state (the App PUT is a replace): * an overridden section sends its values, a non-overridden section sends `null` so the * backend clears that override (→ inherit the realm). Origin always sends a - * section so turning the toggle off explicitly removes any existing route. */ + * section so an emptied field explicitly removes any existing route. */ function build(): ApplicationSettingsDto { return { - Origin: { Subdomain: f.origin.override ? (f.origin.subdomain.trim() || null) : null }, + Origin: { Subdomain: f.origin.subdomain.trim() || null }, Branding: f.branding.override ? { ProductName: f.branding.productName.trim() || null, @@ -512,11 +523,17 @@ function build(): ApplicationSettingsDto { Cimd: f.cimd.override ? { Enabled: f.cimd.enabled, AccessTokenLifetimeMinutes: parseNum(f.cimd.access), RefreshTokenLifetimeDays: parseNum(f.cimd.refresh) } : null, - ChangeFeed: { - Enabled: f.changeFeed.enabled, - MinimumRetentionAgeDays: f.changeFeed.retentionAgeDays ?? 7, - MinimumEventCount: f.changeFeed.minimumEventCount ?? 1000, - }, + // An App that never had a change feed and is not getting one now sends NO section: + // these settings are REPLACE, so emitting the defaults would materialize an override + // that says exactly what no override already says — and show up in every draft plan as + // a change nobody made. A section the App already has is kept (see `configured`). + ChangeFeed: f.changeFeed.enabled || f.changeFeed.configured + ? { + Enabled: f.changeFeed.enabled, + MinimumRetentionAgeDays: f.changeFeed.retentionAgeDays ?? 7, + MinimumEventCount: f.changeFeed.minimumEventCount ?? 1000, + } + : null, } } @@ -614,9 +631,10 @@ watch(() => [activeTab.value, props.applicationId] as const, ([tab]) => {
- - - + + +