Skip to content

Commit 1d64b32

Browse files
ToolboxAidToolboxAid
committed
Fix platform banner save to display contract mismatch - PR_26167_188-platform-banner-display-fix
1 parent 2e6c22d commit 1d64b32

11 files changed

Lines changed: 1069 additions & 2441 deletions

‎admin/platform-settings.html‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ <h2>Admin</h2>
4343
<summary>Banner Controls</summary>
4444
<div class="accordion-body content-stack">
4545
<label class="field-inline">
46-
<input type="checkbox" data-platform-banner-enabled>
46+
<input type="checkbox" data-platform-banner-active>
4747
<span>Show banner</span>
4848
</label>
4949
<label class="field-group">Notice kind
@@ -88,6 +88,7 @@ <h2>Inspector</h2>
8888
<summary>Status</summary>
8989
<div class="accordion-body content-stack">
9090
<p class="status" role="status" data-platform-settings-status>Loading platform banner settings...</p>
91+
<p class="status" role="status" data-platform-banner-diagnostics>Banner diagnostics unavailable.</p>
9192
</div>
9293
</details>
9394
<details class="vertical-accordion" open>

‎assets/theme-v2/js/admin-platform-settings.js‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ import {
66
class AdminPlatformSettingsController {
77
constructor(root) {
88
this.root = root;
9-
this.enabledInput = root.querySelector("[data-platform-banner-enabled]");
9+
this.activeInput = root.querySelector("[data-platform-banner-active]");
1010
this.kindInput = root.querySelector("[data-platform-banner-kind]");
11+
this.diagnostics = root.querySelector("[data-platform-banner-diagnostics]");
1112
this.messageInput = root.querySelector("[data-platform-banner-message]");
1213
this.preview = root.querySelector("[data-platform-banner-preview]");
1314
this.status = root.querySelector("[data-platform-settings-status]");
@@ -16,11 +17,11 @@ class AdminPlatformSettingsController {
1617
}
1718

1819
init() {
19-
if (!this.enabledInput || !this.kindInput || !this.messageInput || !this.preview || !this.status || !this.toneInput || !this.saveButton) {
20+
if (!this.activeInput || !this.kindInput || !this.diagnostics || !this.messageInput || !this.preview || !this.status || !this.toneInput || !this.saveButton) {
2021
return;
2122
}
2223
this.saveButton.addEventListener("click", () => this.save());
23-
[this.enabledInput, this.kindInput, this.messageInput, this.toneInput].forEach((control) => {
24+
[this.activeInput, this.kindInput, this.messageInput, this.toneInput].forEach((control) => {
2425
control.addEventListener("input", () => this.renderPreview(this.currentBanner()));
2526
control.addEventListener("change", () => this.renderPreview(this.currentBanner()));
2627
});
@@ -33,24 +34,35 @@ class AdminPlatformSettingsController {
3334

3435
currentBanner() {
3536
return {
36-
enabled: this.enabledInput.checked,
37+
active: this.activeInput.checked,
3738
kind: this.kindInput.value,
3839
message: this.messageInput.value.trim(),
3940
tone: this.toneInput.value
4041
};
4142
}
4243

4344
applyBanner(banner) {
44-
this.enabledInput.checked = banner.enabled === true;
45+
this.activeInput.checked = banner.active === true;
4546
this.kindInput.value = banner.kind || "general";
4647
this.messageInput.value = banner.message || "";
4748
this.toneInput.value = banner.tone || "info";
4849
this.renderPreview(this.currentBanner());
4950
}
5051

52+
setDiagnostics(diagnostics = {}) {
53+
const active = diagnostics.active === true;
54+
const message = typeof diagnostics.message === "string" && diagnostics.message
55+
? diagnostics.message
56+
: "(empty)";
57+
const sourceTableRowKey = typeof diagnostics.sourceTableRowKey === "string" && diagnostics.sourceTableRowKey
58+
? diagnostics.sourceTableRowKey
59+
: "(missing)";
60+
this.diagnostics.textContent = `Active: ${active}. Message: ${message}. Source row: ${sourceTableRowKey}.`;
61+
}
62+
5163
renderPreview(banner) {
5264
this.preview.dataset.platformBannerPreviewTone = banner.tone || "info";
53-
this.preview.textContent = banner.enabled && banner.message
65+
this.preview.textContent = banner.active && banner.message
5466
? banner.message
5567
: "No active banner.";
5668
}
@@ -59,6 +71,7 @@ class AdminPlatformSettingsController {
5971
try {
6072
const payload = readAdminPlatformBanner();
6173
this.applyBanner(payload.banner || {});
74+
this.setDiagnostics(payload.diagnostics || payload.banner || {});
6275
this.setStatus("Platform banner settings loaded.");
6376
} catch (error) {
6477
this.setStatus(error instanceof Error ? error.message : "Platform banner settings are unavailable.");
@@ -69,6 +82,7 @@ class AdminPlatformSettingsController {
6982
try {
7083
const payload = updateAdminPlatformBanner(this.currentBanner());
7184
this.applyBanner(payload.banner || {});
85+
this.setDiagnostics(payload.diagnostics || payload.banner || {});
7286
this.setStatus("Platform banner settings saved.");
7387
window.dispatchEvent(new CustomEvent("gamefoundry:platform-settings-changed"));
7488
} catch (error) {

‎assets/theme-v2/js/gamefoundry-partials.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,8 @@
388388
return {
389389
active: banner.active === true,
390390
message: typeof banner.message === "string" ? banner.message.trim() : "",
391+
sourceTable: typeof banner.sourceTable === "string" ? banner.sourceTable : data?.sourceTable || "",
392+
sourceTableRowKey: typeof banner.sourceTableRowKey === "string" ? banner.sourceTableRowKey : "",
391393
tone
392394
};
393395
}
@@ -430,6 +432,12 @@
430432
async function renderPlatformBanner() {
431433
try {
432434
const banner = await requestPlatformBanner();
435+
window.GameFoundryPlatformBannerDiagnostics = {
436+
active: banner.active,
437+
message: banner.message,
438+
sourceTable: banner.sourceTable,
439+
sourceTableRowKey: banner.sourceTableRowKey
440+
};
433441
removePlatformBanner();
434442
if (!banner.active || !banner.message) {
435443
return;
@@ -446,6 +454,12 @@
446454
}
447455
} catch (error) {
448456
removePlatformBanner();
457+
window.GameFoundryPlatformBannerDiagnostics = {
458+
active: false,
459+
message: "",
460+
sourceTable: "",
461+
sourceTableRowKey: ""
462+
};
449463
console.warn("[platform-settings/operator] Platform banner unavailable:", error instanceof Error ? error.message : String(error || ""));
450464
}
451465
}

‎docs_build/dev/reports/PR_26167_188-platform-banner-display-fix.md‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
# PR_26167_188-platform-banner-display-fix
2+
3+
## Root Cause
4+
5+
The platform banner contract mixed `enabled` and `active` names. The live table had the banner message saved, but the active setting row was still `false`, so `/api/platform-settings/banner` correctly returned inactive data and both the Admin preview and header banner displayed no active banner.
6+
7+
Fix: make `active` the single browser/API contract name, keep the existing `platform.banner.enabled` table setting key for row continuity, and add diagnostics that expose active state, message, and source table row key.
8+
9+
## Branch Validation
10+
11+
PASS - current branch is `main`.
12+
13+
## Requirement Checklist
14+
15+
| Requirement | Status | Evidence |
16+
| --- | --- | --- |
17+
| Read `docs_build/dev/PROJECT_INSTRUCTIONS.md` first | PASS | Read before implementation. |
18+
| Hard stop unless current branch is `main` | PASS | `git branch --show-current` returned `main`. |
19+
| Investigate Admin Preferences save | PASS | Found Admin save used an `enabled` payload name instead of the public `active` banner contract. |
20+
| Investigate platform settings table write | PASS | Live DB row evidence confirmed message row existed while active row value was `false` before the fix. |
21+
| Investigate `/api/platform-settings/banner` response | PASS | Before payload had `active: false`, populated message, and source row key; after save had `active: true`. |
22+
| Investigate `requestPlatformBanner()` | PASS | Confirmed it reads `/api/platform-settings/banner` and normalizes the public banner payload. |
23+
| Investigate `renderPlatformBanner()` | PASS | Confirmed it removes the banner when `active` is false or message is empty, and renders under header when active. |
24+
| Determine if active flag was not being saved | PASS | Live before state showed active row `false`; fixed Admin save sends `active: true/false`. |
25+
| Determine if API returns inactive banner | PASS | Before payload returned inactive; after fixed save returned active. |
26+
| Determine if API shape mismatched `normalizedPlatformBanner()` | PASS | Fixed contract to use `active` consistently; tests now use active-only mocked API payloads. |
27+
| Determine if banner message stored in wrong field | PASS | Message was stored in `platform.banner.message`; not the root cause. |
28+
| Determine if read path differs from save path | PASS | Both read and save use `platform_settings`; diagnostics now expose the source row key. |
29+
| Fix root cause | PASS | Admin payload, Admin UI hook, server update normalization, public normalization, and tests now use `active`. |
30+
| Add diagnostics showing active state, message, source table row key | PASS | API returns `diagnostics`; Admin page shows visible diagnostics; renderer exposes `window.GameFoundryPlatformBannerDiagnostics`. |
31+
| Keep platform-settings as SSoT | PASS | Reads/writes remain through `platform_settings` API/service contract only. |
32+
| No browser-owned banner data | PASS | Browser code calls server API; no local persistence or page-owned banner records. |
33+
| No silent fallback | PASS | API errors throw/report status; inactive banners are explicit `active: false`, not fallback data. |
34+
| Save banner `banner is active!@!!` | PASS | Live validation saved exact message with active true. |
35+
| Verify row exists in DB | PASS | Live DB row evidence listed active/message/tone/kind `platform_settings` rows. |
36+
| Verify GET returns active=true and message populated | PASS | Live public API after active save returned `active: true` and exact message. |
37+
| Verify banner renders under header | PASS | Live browser validation found header before banner and banner before main with exact message. |
38+
| Verify banner removed when active=false | PASS | Live public API returned `active: false`; browser validation found rendered banner count 0. |
39+
| Run targeted platform-settings Playwright | PASS | 2/2 targeted Playwright tests passed. |
40+
| Do not run full samples smoke | PASS | Full samples smoke was not run. |
41+
42+
## Before API Payload
43+
44+
`GET /api/platform-settings/banner` before the fix returned:
45+
46+
```json
47+
{
48+
"status": 200,
49+
"banner": {
50+
"active": false,
51+
"kind": "general",
52+
"message": "banner is active!@!!",
53+
"sourceTableRowKey": "01KV8W32ZERK6CD5W270QPBC18",
54+
"sourceTable": "platform_settings",
55+
"tone": "info"
56+
},
57+
"diagnostics": {
58+
"active": false,
59+
"message": "banner is active!@!!",
60+
"sourceTable": "platform_settings",
61+
"sourceTableRowKey": "01KV8W32ZERK6CD5W270QPBC18"
62+
}
63+
}
64+
```
65+
66+
## After API Payload
67+
68+
After saving `active: true` with message `banner is active!@!!`, `GET /api/platform-settings/banner` returned:
69+
70+
```json
71+
{
72+
"status": 200,
73+
"banner": {
74+
"active": true,
75+
"kind": "temporary-data",
76+
"message": "banner is active!@!!",
77+
"sourceTableRowKey": "01KV8W32ZERK6CD5W270QPBC18",
78+
"sourceTable": "platform_settings",
79+
"tone": "warning"
80+
},
81+
"diagnostics": {
82+
"active": true,
83+
"message": "banner is active!@!!",
84+
"sourceTable": "platform_settings",
85+
"sourceTableRowKey": "01KV8W32ZERK6CD5W270QPBC18"
86+
}
87+
}
88+
```
89+
90+
After saving `active: false`, the public API returned `active: false` with the same message and source row key.
91+
92+
## DB Row Evidence
93+
94+
After active save:
95+
96+
| Setting key | Row key | Value | Active row |
97+
| --- | --- | --- | --- |
98+
| `platform.banner.enabled` | `01KV8W32ZERK6CD5W270QPBC18` | `true` | true |
99+
| `platform.banner.message` | `01KV8W32ZE2XZMG1HY8XJM09Q7` | `banner is active!@!!` | true |
100+
| `platform.banner.tone` | `01KV8W32ZE3ZC91PRWECZX16H4` | `warning` | true |
101+
| `platform.banner.kind` | `01KV8W32ZE870AYSVT62X4JTY5` | `temporary-data` | true |
102+
103+
After inactive save:
104+
105+
| Setting key | Row key | Value | Active row |
106+
| --- | --- | --- | --- |
107+
| `platform.banner.enabled` | `01KV8W32ZERK6CD5W270QPBC18` | `false` | true |
108+
| `platform.banner.message` | `01KV8W32ZE2XZMG1HY8XJM09Q7` | `banner is active!@!!` | true |
109+
110+
## Rendered Banner Evidence
111+
112+
Live browser validation after active save:
113+
114+
```json
115+
{
116+
"message": "banner is active!@!!",
117+
"headerBeforeBanner": true,
118+
"bannerBeforeMain": true,
119+
"diagnostics": {
120+
"active": true,
121+
"message": "banner is active!@!!",
122+
"sourceTable": "platform_settings",
123+
"sourceTableRowKey": "01KV8W32ZERK6CD5W270QPBC18"
124+
}
125+
}
126+
```
127+
128+
Live browser validation after inactive save:
129+
130+
```json
131+
{
132+
"renderedInactiveBannerCount": 0
133+
}
134+
```
135+
136+
## Validation Lane Report
137+
138+
| Lane | Result | Notes |
139+
| --- | --- | --- |
140+
| Static syntax | PASS | `node --check` for changed JS/MJS files passed. |
141+
| HTML restriction scan | PASS | No inline script/style/event handlers found in `admin/platform-settings.html`. |
142+
| Contract/API test | PASS | `node --test --test-name-pattern="Platform banner reads and writes through platform settings service routes" tests/dev-runtime/SupabaseProductDataCutover.test.mjs`. |
143+
| Targeted Playwright | PASS | `npx playwright test tests/playwright/tools/AdminPlatformToolsWireframes.spec.mjs --grep "Platform banner renders|Platform Settings Admin controls" --workers=1` passed 2/2. |
144+
| Live manual validation | PASS | Saved exact banner message, verified DB rows, verified public API active true, verified render, then verified active false removes banner. |
145+
| Playwright V8 coverage | PASS | PR188-scoped report written to `docs_build/dev/reports/playwright_v8_coverage_report.txt`; server-side route module reported as browser V8 WARN. |
146+
| Full samples smoke | SKIP | Explicitly excluded by request. |
147+
148+
## Manual Validation Notes
149+
150+
- DavidQ sign-in for the live validation returned creator/admin roles; the password value was not printed or written to files.
151+
- Final live banner state after validation is inactive (`active: false`) with the message retained in `platform_settings`; the header banner does not render while inactive.
152+
- No `.log` or `.txt` files were created under repo `tmp`.
153+

‎docs_build/dev/reports/codex_changed_files.txt‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,11 @@
1-
admin/platform-settings.html
2-
assets/theme-v2/css/status.css
1+
admin/platform-settings.html
32
assets/theme-v2/js/admin-platform-settings.js
43
assets/theme-v2/js/gamefoundry-partials.js
5-
docs_build/database/dml/admin.sql
6-
docs_build/database/seed/admin.json
7-
docs_build/dev/reports/PR_26167_187-live-db-cleanup-and-platform-banner.md
4+
docs_build/dev/reports/PR_26167_188-platform-banner-display-fix.md
85
docs_build/dev/reports/codex_changed_files.txt
96
docs_build/dev/reports/codex_review.diff
107
docs_build/dev/reports/coverage_changed_js_guardrail.txt
11-
docs_build/dev/reports/environment_agnostic_browser_gate_report.md
128
docs_build/dev/reports/playwright_v8_coverage_report.txt
13-
scripts/sync-supabase-dev-creator-identities.mjs
14-
src/dev-runtime/auth/provider-contract-stubs.mjs
15-
src/dev-runtime/persistence/mock-db-store.js
16-
src/dev-runtime/seed/seed-db-keys.mjs
17-
src/dev-runtime/seed/server-seed-loader.mjs
189
src/dev-runtime/server/local-api-router.mjs
19-
src/dev-runtime/server/local-api-server.mjs
20-
src/dev-runtime/server/mock-api-router.mjs
21-
src/dev-runtime/testing/supabase-dev-auth-test-user-cleanup.mjs
22-
src/dev-runtime/testing/supabase-dev-creator-identity-seed-sync.mjs
23-
src/engine/api/platform-settings-api-client.js
24-
tests/dev-runtime/SupabaseDevCreatorIdentitySeedSync.test.mjs
2510
tests/dev-runtime/SupabaseProductDataCutover.test.mjs
26-
tests/dev-runtime/SupabaseProviderContractStub.test.mjs
2711
tests/playwright/tools/AdminPlatformToolsWireframes.spec.mjs

0 commit comments

Comments
 (0)