Skip to content

Commit ce9bff8

Browse files
committed
feat(settings): validate the SteamGridDB key inline and drop the redundant test buttons
Sign-in already validates inline in the modal and the QAM connection row probes the server on open, so the two manual "test" buttons in Connection Settings were redundant. - Remove the "Test Connection" button from ConnectionSection (the QAM row probes automatically; the test_connection callable stays for that probe). - Remove the SteamGridDB "Verify Key" button and the section's status line. - Enter the SteamGridDB key through a dedicated SgdbApiKeyModal that verifies the key against SteamGridDB before saving it — an invalid key is rejected inline and the modal stays open, so no unchecked key is ever persisted. - Surface the specific reason on a failed QAM connection probe (Sign-in rejected / Server unreachable / No server URL / Not signed in) instead of a bare "Not connected".
1 parent 2eaa316 commit ce9bff8

13 files changed

Lines changed: 567 additions & 312 deletions

docs/user-guide/configuration.md

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,10 @@ The Connection Settings page manages your RomM server connection.
2929
[Sign out](#sign-out)).
3030
- **Allow Insecure SSL** — shown only for `https://` URLs; skips certificate verification for self-signed certs (LAN
3131
only).
32-
- **Test Connection** — available once you are signed in; verifies the plugin can reach and authenticate with your RomM
33-
server using the stored token.
32+
33+
The plugin checks the connection for you — there is no manual "Test Connection" button. The **Connection** row on the
34+
plugin's main QAM panel shows the live status whenever you open it, and names the problem when it can't connect (for
35+
example _Sign-in rejected_, _Server unreachable_, or _No server URL_).
3436

3537
### Sign out
3638

@@ -125,10 +127,11 @@ To set this up:
125127

126128
1. Create a free account at [steamgriddb.com](https://www.steamgriddb.com/)
127129
2. Go to your [API preferences](https://www.steamgriddb.com/profile/preferences/api) and copy your API key
128-
3. In Connection Settings, paste it into the **API Key** field under "SteamGridDB"
129-
4. Tap **Verify Key** to confirm it works
130+
3. In Connection Settings, tap **Edit** next to **API Key** under "SteamGridDB" and paste your key
131+
4. Tap **Save** — the key is checked against SteamGridDB first, so an invalid key is rejected inline and only a working
132+
key is stored
130133

131-
<!-- Screenshot: SteamGridDB API Key section with Edit and Verify buttons -->
134+
<!-- Screenshot: SteamGridDB API Key section with the Edit button -->
132135

133136
Without an API key, games will still have cover art from RomM but the hero banner, logo overlay, and wide grid image
134137
will be missing.

docs/user-guide/getting-started.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ After installation, you need to connect the plugin to your RomM server:
7171
2. Tap **Connection Settings**
7272
3. Enter your RomM server URL (e.g. `http://192.168.1.100:8080`) — this saves automatically
7373
4. Tap **Sign in**, enter your RomM username and password once, and confirm
74-
5. The **RomM Account** row shows **Signed in** on success. Once signed in, tap **Test Connection** to re-verify at any
75-
time
74+
5. The **RomM Account** row shows **Signed in** on success. The plugin's main QAM panel has a **Connection** row that
75+
shows the live connection status whenever you open it — there is no separate test button
7676

7777
On a controller you can confirm a text field with the on-screen keyboard's **Enter** key (or the **R2** shortcut)
7878
instead of navigating to the button — for example save-slot names and the artwork search. In the sign-in dialog, Enter

docs/user-guide/troubleshooting.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,9 @@ config file.
5454

5555
**Symptom**: Toast notifications say "RomM unreachable" or sync operations show in the Failed Syncs list.
5656

57-
**Fix**: Check your network connection and verify the RomM server is running. Go to **Connection Settings** and tap
58-
**Test Connection**. Failed syncs are queued and retried automatically when the server is reachable again.
57+
**Fix**: Check your network connection and verify the RomM server is running. The **Connection** row on the plugin's
58+
main QAM panel shows the live status (and names the problem when it can't connect). Failed syncs are queued and retried
59+
automatically when the server is reachable again.
5960

6061
### Offline detection and recovery
6162

@@ -175,8 +176,8 @@ For technical details, see [Steam Remote Play and Cross-Device Shortcuts](../arc
175176

176177
### Download shows no progress
177178

178-
**Fix**: Check your connection to the RomM server (Connection Settings > Test Connection). If the server is reachable,
179-
try cancelling and restarting the download from the game detail page.
179+
**Fix**: Check your connection to the RomM server (the **Connection** row on the plugin's main QAM panel). If the server
180+
is reachable, try cancelling and restarting the download from the game detail page.
180181

181182
### Download failed
182183

src/components/MainPage.test.tsx

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
3838
import { render, fireEvent, act } from "@testing-library/react";
3939
import { createElement, type ReactElement } from "react";
40-
import { MainPage } from "./MainPage";
40+
import { MainPage, ConnectionIndicator } from "./MainPage";
4141
import * as backend from "../api/backend";
4242
import { useVersionError } from "./VersionErrorCard";
4343
import {
@@ -656,6 +656,56 @@ describe("MainPage", () => {
656656
await flushAsync();
657657
expect(container.textContent).toContain("Not connected");
658658
});
659+
660+
// A failed probe carries the backend's {reason, message}; the row shows a
661+
// specific label instead of a bare "Not connected". version_error is not
662+
// exercised here — a version failure short-circuits the whole panel to the
663+
// VersionErrorCard (covered separately); the label mapping is covered by the
664+
// direct-render cases below.
665+
it.each([
666+
["auth_failed", "401 Unauthorized", "Sign-in rejected"],
667+
["server_unreachable", "timed out", "Server unreachable"],
668+
["config_error", "No server URL configured", "No server URL"],
669+
["config_error", "Not signed in — sign in to RomM first", "Not signed in"],
670+
["config_error", "some other config issue", "Not connected"],
671+
] as const)("a failed probe with reason=%s renders %s", async (reason, message, label) => {
672+
vi.mocked(backend.testConnection).mockResolvedValue({ success: false, reason, message });
673+
const { container } = render(<MainPage onNavigate={vi.fn()} />);
674+
await flushAsync();
675+
expect(container.textContent).toContain(label);
676+
});
677+
});
678+
679+
// ===========================================================================
680+
// D1. ConnectionIndicator — failure label mapping (direct render)
681+
// ===========================================================================
682+
describe("ConnectionIndicator failure labels", () => {
683+
it.each([
684+
["auth_failed", "", "Sign-in rejected"],
685+
["server_unreachable", "", "Server unreachable"],
686+
["version_error", "server too old", "Unsupported RomM version"],
687+
["config_error", "No server URL configured", "No server URL"],
688+
["config_error", "Not signed in — sign in to RomM first", "Not signed in"],
689+
["config_error", "unclassified config problem", "Not connected"],
690+
["unknown", "", "Not connected"],
691+
] as const)("reason=%s / message=%s → %s", (reason, message, label) => {
692+
const { container } = render(<ConnectionIndicator connected={false} failure={{ reason, message }} />);
693+
expect(container.textContent).toContain(label);
694+
});
695+
696+
it("falls back to 'Not connected' when no failure detail is present", () => {
697+
const { container } = render(<ConnectionIndicator connected={false} failure={null} />);
698+
expect(container.textContent).toContain("Not connected");
699+
});
700+
701+
it("still renders the unchanged Connected / Checking… / Backend error states", () => {
702+
const connected = render(<ConnectionIndicator connected={true} />);
703+
expect(connected.container.textContent).toContain("Connected");
704+
const checking = render(<ConnectionIndicator connected={null} />);
705+
expect(checking.container.textContent).toContain("Checking...");
706+
const failed = render(<ConnectionIndicator connected="backend_failed" />);
707+
expect(failed.container.textContent).toContain("Backend error");
708+
});
659709
});
660710

661711
// ===========================================================================

src/components/MainPage.tsx

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ import type {
7373
SessionBudgetStatus,
7474
DownloadItem,
7575
MigrationStatus,
76+
RommErrorCode,
7677
} from "../types";
7778
import { detach } from "../utils/detach";
7879
import { withTimeout } from "../utils/withTimeout";
@@ -98,6 +99,39 @@ const CONNECTION_CALLABLE_TIMEOUT = 5000;
9899
/** Backend never answered after the retry budget — distinct from `false` ("not connected"). */
99100
type BackendFailed = "backend_failed";
100101

102+
/** A resolved-but-failed `test_connection()` probe. `reason`/`message` classify
103+
* why so the connection row shows a specific label instead of a bare "Not
104+
* connected"; both are absent when the probe never resolved (an unreachable
105+
* server that hung past every deadline), which reads as the generic label. */
106+
interface ConnectionFailure {
107+
reason: RommErrorCode | undefined;
108+
message: string;
109+
}
110+
111+
/** The connection-row label for a failed probe, mapped from the backend's
112+
* `{reason, message}`. The two `config_error` sub-cases are split by message
113+
* text (the slug is shared). Anything unclassified falls back to the generic
114+
* "Not connected". `version_error` is included for completeness even though a
115+
* version failure short-circuits the whole panel to the VersionErrorCard. */
116+
function connectionFailureLabel(failure: ConnectionFailure | null | undefined): string {
117+
const reason = failure?.reason;
118+
const message = failure?.message ?? "";
119+
switch (reason) {
120+
case "auth_failed":
121+
return "Sign-in rejected";
122+
case "server_unreachable":
123+
return "Server unreachable";
124+
case "version_error":
125+
return "Unsupported RomM version";
126+
case "config_error":
127+
if (/server url/i.test(message)) return "No server URL";
128+
if (/not signed in/i.test(message)) return "Not signed in";
129+
return "Not connected";
130+
default:
131+
return "Not connected";
132+
}
133+
}
134+
101135
/** Counted segments — ``[[count, word], …]`` → ``"353 new / 800 updated"``: every
102136
* segment spells its word out, zero counts dropped, joined with `` / ``. Empty
103137
* when every count is zero. The old ``+``/``~``/``−`` sigils were a legend the
@@ -109,7 +143,10 @@ function countedSegments(pairs: [number, string][]): string {
109143
.join(" / ");
110144
}
111145

112-
const ConnectionIndicator: FC<{ connected: boolean | null | BackendFailed }> = ({ connected }) => {
146+
export const ConnectionIndicator: FC<{
147+
connected: boolean | null | BackendFailed;
148+
failure?: ConnectionFailure | null;
149+
}> = ({ connected, failure }) => {
113150
if (connected === "backend_failed") {
114151
return (
115152
<>
@@ -137,7 +174,7 @@ const ConnectionIndicator: FC<{ connected: boolean | null | BackendFailed }> = (
137174
return (
138175
<>
139176
<FaTimesCircle style={{ color: "#d4343c", fontSize: "14px" }} />
140-
<span style={{ fontSize: "12px" }}>Not connected</span>
177+
<span style={{ fontSize: "12px" }}>{connectionFailureLabel(failure)}</span>
141178
</>
142179
);
143180
};
@@ -423,6 +460,10 @@ export const MainPage: FC<MainPageProps> = ({ onNavigate }) => {
423460
const [stats, setStats] = useState<SyncStats | null>(null);
424461
const [budgetStatus, setBudgetStatus] = useState<SessionBudgetStatus | null>(null);
425462
const [connected, setConnected] = useState<boolean | null | BackendFailed>(null);
463+
// Classifies a resolved-but-failed probe so the connection row can show a
464+
// specific label (auth rejected / server unreachable / no URL / not signed
465+
// in). Null for every non-failed state and for a probe that never resolved.
466+
const [connectionFailure, setConnectionFailure] = useState<ConnectionFailure | null>(null);
426467
const versionError = useVersionError();
427468
const [syncing, setSyncing] = useState(false);
428469
// Disarmed "Cancelling…" state during the backend's RUNNING→CANCELLING→IDLE
@@ -499,6 +540,7 @@ export const MainPage: FC<MainPageProps> = ({ onNavigate }) => {
499540
const r = await withTimeout(testConnection(), CONNECTION_CALLABLE_TIMEOUT);
500541
if (isCancelled()) return;
501542
setConnected(r.success);
543+
setConnectionFailure(r.success ? null : { reason: r.reason, message: r.message });
502544
setVersionError(r.reason === "version_error" ? r.message : null);
503545
return;
504546
} catch {
@@ -1289,7 +1331,7 @@ export const MainPage: FC<MainPageProps> = ({ onNavigate }) => {
12891331
}
12901332
>
12911333
<div style={{ display: "flex", alignItems: "center", gap: "6px" }}>
1292-
<ConnectionIndicator connected={connected} />
1334+
<ConnectionIndicator connected={connected} failure={connectionFailure} />
12931335
</div>
12941336
</Field>
12951337
</PanelSectionRow>

src/components/SettingsPage.test.tsx

Lines changed: 21 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -597,7 +597,7 @@ describe("SettingsPage", () => {
597597
});
598598

599599
// The .catch sets setStatus("Failed to save settings") — assert it
600-
// surfaced via ConnectionSection.status (mirrors the handleTest throw test).
600+
// surfaced via ConnectionSection.status.
601601
const last = capturedConnection[capturedConnection.length - 1];
602602
expect(last?.status).toBe("Failed to save settings");
603603
});
@@ -940,37 +940,6 @@ describe("SettingsPage", () => {
940940
});
941941
});
942942

943-
describe("handleTest", () => {
944-
it("forwards the result message into ConnectionSection.status on success", async () => {
945-
vi.mocked(backend.testConnection).mockResolvedValue({
946-
success: true,
947-
message: "Connected!",
948-
});
949-
render(<SettingsPage onBack={vi.fn()} />);
950-
await flushAsync();
951-
952-
await act(async () => {
953-
capturedConnection[capturedConnection.length - 1]?.onTestConnection();
954-
await Promise.resolve();
955-
});
956-
957-
expect(capturedConnection[capturedConnection.length - 1]?.status).toBe("Connected!");
958-
});
959-
960-
it("sets status='Connection test failed' on throw", async () => {
961-
vi.mocked(backend.testConnection).mockRejectedValue(new Error("boom"));
962-
render(<SettingsPage onBack={vi.fn()} />);
963-
await flushAsync();
964-
965-
await act(async () => {
966-
capturedConnection[capturedConnection.length - 1]?.onTestConnection();
967-
await Promise.resolve();
968-
});
969-
970-
expect(capturedConnection[capturedConnection.length - 1]?.status).toBe("Connection test failed");
971-
});
972-
});
973-
974943
describe("handleSaveSyncSettingChange", () => {
975944
it("does nothing when saveSyncSettings is still null", async () => {
976945
// Cause getSaveSyncSettings to never resolve — saveSyncSettings stays null.
@@ -1306,81 +1275,41 @@ describe("SettingsPage", () => {
13061275
});
13071276

13081277
describe("SteamGridDB handlers", () => {
1309-
it("handleSgdbKeySubmit success with a non-empty value sets sgdbApiKey='set' and surfaces result.message", async () => {
1310-
vi.mocked(backend.saveSgdbApiKey).mockResolvedValue({
1311-
success: true,
1312-
message: "Saved!",
1313-
});
1278+
it("wires onVerifyKey directly to the verifySgdbApiKey callable (tests the key without persisting)", async () => {
1279+
vi.mocked(backend.verifySgdbApiKey).mockResolvedValue({ success: true, message: "API key is valid" });
13141280
render(<SettingsPage onBack={vi.fn()} />);
13151281
await flushAsync();
1316-
await act(async () => {
1317-
capturedSgdb[capturedSgdb.length - 1]?.onSubmitKey("apikey123");
1318-
});
13191282
const sgdb = capturedSgdb[capturedSgdb.length - 1];
1320-
expect(sgdb?.sgdbApiKey).toBe("set");
1321-
expect(sgdb?.sgdbStatus).toBe("Saved!");
1322-
});
1323-
1324-
it("handleSgdbKeySubmit success with an empty value clears the masked sgdbApiKey", async () => {
1325-
vi.mocked(backend.saveSgdbApiKey).mockResolvedValue({
1326-
success: true,
1327-
message: "Cleared",
1328-
});
1329-
render(<SettingsPage onBack={vi.fn()} />);
1330-
await flushAsync();
13311283
await act(async () => {
1332-
capturedSgdb[capturedSgdb.length - 1]?.onSubmitKey("");
1284+
await sgdb?.onVerifyKey("apikey123");
13331285
});
1334-
expect(capturedSgdb[capturedSgdb.length - 1]?.sgdbApiKey).toBe("");
1286+
expect(vi.mocked(backend.verifySgdbApiKey)).toHaveBeenCalledWith("apikey123");
1287+
// Verifying never persists — that is the modal's second step (onSaveKey).
1288+
expect(vi.mocked(backend.saveSgdbApiKey)).not.toHaveBeenCalled();
13351289
});
13361290

1337-
it("handleSgdbKeySubmit sets sgdbStatus='Failed to save API key' on throw", async () => {
1338-
vi.mocked(backend.saveSgdbApiKey).mockRejectedValue(new Error("boom"));
1291+
it("handleSaveSgdbKey persists via saveSgdbApiKey and flips the masked display to a configured key", async () => {
1292+
vi.mocked(backend.saveSgdbApiKey).mockResolvedValue({ success: true, message: "Saved!" });
13391293
render(<SettingsPage onBack={vi.fn()} />);
13401294
await flushAsync();
13411295
await act(async () => {
1342-
capturedSgdb[capturedSgdb.length - 1]?.onSubmitKey("k");
1296+
await capturedSgdb[capturedSgdb.length - 1]?.onSaveKey("apikey123");
13431297
});
1344-
expect(capturedSgdb[capturedSgdb.length - 1]?.sgdbStatus).toBe("Failed to save API key");
1298+
expect(vi.mocked(backend.saveSgdbApiKey)).toHaveBeenCalledWith("apikey123");
1299+
// A configured key renders as the masked "••••" (sgdbApiKey truthy).
1300+
expect(capturedSgdb[capturedSgdb.length - 1]?.sgdbApiKey).toBe("set");
13451301
});
13461302

1347-
it("handleSgdbVerify success=true sets sgdbStatus='Valid'", async () => {
1348-
vi.mocked(backend.verifySgdbApiKey).mockResolvedValue({
1349-
success: true,
1350-
message: "any",
1351-
});
1352-
render(<SettingsPage onBack={vi.fn()} />);
1353-
await flushAsync();
1354-
await act(async () => {
1355-
capturedSgdb[capturedSgdb.length - 1]?.onVerifyKey();
1356-
await Promise.resolve();
1357-
});
1358-
expect(capturedSgdb[capturedSgdb.length - 1]?.sgdbStatus).toBe("Valid");
1359-
});
1360-
1361-
it("handleSgdbVerify success=false surfaces result.message", async () => {
1362-
vi.mocked(backend.verifySgdbApiKey).mockResolvedValue({
1363-
success: false,
1364-
message: "Invalid key",
1365-
});
1366-
render(<SettingsPage onBack={vi.fn()} />);
1367-
await flushAsync();
1368-
await act(async () => {
1369-
capturedSgdb[capturedSgdb.length - 1]?.onVerifyKey();
1370-
await Promise.resolve();
1371-
});
1372-
expect(capturedSgdb[capturedSgdb.length - 1]?.sgdbStatus).toBe("Invalid key");
1373-
});
1374-
1375-
it("handleSgdbVerify throws → sgdbStatus='Verification failed'", async () => {
1376-
vi.mocked(backend.verifySgdbApiKey).mockRejectedValue(new Error("net"));
1303+
it("handleSaveSgdbKey lets a save rejection propagate so the modal can surface it", async () => {
1304+
vi.mocked(backend.saveSgdbApiKey).mockRejectedValue(new Error("boom"));
13771305
render(<SettingsPage onBack={vi.fn()} />);
13781306
await flushAsync();
1379-
await act(async () => {
1380-
capturedSgdb[capturedSgdb.length - 1]?.onVerifyKey();
1381-
await Promise.resolve();
1382-
});
1383-
expect(capturedSgdb[capturedSgdb.length - 1]?.sgdbStatus).toBe("Verification failed");
1307+
const sgdb = capturedSgdb[capturedSgdb.length - 1];
1308+
// The handler does not swallow — the modal's own try/catch owns the error
1309+
// UI, so onSaveKey must reject rather than resolve.
1310+
await expect(sgdb?.onSaveKey("k")).rejects.toThrow("boom");
1311+
// A failed save must not flip the masked display to configured.
1312+
expect(capturedSgdb[capturedSgdb.length - 1]?.sgdbApiKey).toBe("");
13841313
});
13851314
});
13861315

0 commit comments

Comments
 (0)