Skip to content

Commit 283134d

Browse files
authored
fix(settings): harden RomM sign-in and consolidate the connection/SteamGridDB UX (#1565)
Hardens the RomM sign-in flow and consolidates the connection / SteamGridDB settings UX after on-device testing. **Sign-in (RomM):** - Every mode's fields are required — username **and** password, the API token, or all eight pairing-code boxes. The Sign-in button stays disabled until they're filled; an empty form can only be cancelled, which leaves the existing sign-in untouched. - Enter / R2 is placed logically: in the username field it advances to the password field, and it confirms only from a completing field once every required field is filled — never submitting an incomplete form. A single-field modal no longer closes on an empty Enter/R2. - A failed sign-in keeps the dialog open and shows the error inline, closing only on success — so the message sits where you typed, and a failed re-sign-in no longer silently leaves the previous token presenting as a fresh success. - A rejected token is reported plainly ("RomM rejected this token…") instead of a generic "Server error", and a wrong username/password no longer wrongly claims the account can't create API tokens — RomM returns an indistinguishable 403 for both, so the message names both causes. **SteamGridDB key:** validated inline in its own modal — the entered key is tested against SteamGridDB before it is saved; a rejected key keeps the modal open with the reason, and only a valid key is persisted. **Fewer redundant buttons:** removed the settings-page **Test Connection** and **Verify Key** buttons — the QAM Connection row already probes on open, and both modals now validate inline. **Clearer QAM status:** the Connection row surfaces the specific reason on failure (Not signed in / Sign-in rejected / Server unreachable / No server URL) instead of a bare "Not connected". All settings text inputs (RomM URL, SteamGridDB key, default save slot) now require a non-empty value. Needs on-device verification in Game Mode — the on-screen-keyboard Enter/R2 dispatch and gamepad focus traversal can't be exercised by the unit tests. Closes #1564
1 parent ebcee07 commit 283134d

22 files changed

Lines changed: 1521 additions & 585 deletions

.size-limit.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,6 @@
33
"name": "dist/index.js (raw)",
44
"path": "dist/index.js",
55
"brotli": false,
6-
"limit": "820 KB"
6+
"limit": "830 KB"
77
}
88
]

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: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,13 @@ 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

77-
On a controller, any text field in the plugin can be confirmed with the on-screen keyboard's **Enter** key (or the
78-
**R2** shortcut) instead of navigating to the button — this applies to sign-in, save-slot names, and the artwork search.
77+
On a controller you can confirm a text field with the on-screen keyboard's **Enter** key (or the **R2** shortcut)
78+
instead of navigating to the button — for example save-slot names and the artwork search. In the sign-in dialog, Enter
79+
moves from the username to the password field and confirms only once every required field is filled; an incomplete form
80+
can't be submitted, and **Cancel** leaves your existing sign-in untouched.
7981

8082
The plugin mints a RomM Client API Token from the credentials you enter and discards the password — it is never stored.
8183
The same applies if the plugin auto-migrates an older install that still had a saved password: the password is discarded

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

py_modules/services/connection.py

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@
5050
_INVALID_URL_MESSAGE = "Enter a valid http(s):// server URL"
5151

5252
_FORBIDDEN_TOKEN_MESSAGE = (
53-
"Your RomM account cannot create API tokens — ask your admin to grant "
54-
"token permissions or use an account with a higher role."
53+
"RomM rejected the sign-in. Check your username and password — if they are correct, your account may not be "
54+
"allowed to create API tokens (ask your admin or use an account with a higher role)."
5555
)
5656

5757
# Pasted-token validation (``establish_user_token``): a 401 means the token
@@ -62,6 +62,9 @@
6262
_USER_TOKEN_SCOPE_MESSAGE = (
6363
"The API token is missing required permissions (scopes). Grant the scopes listed in the plugin docs and try again."
6464
)
65+
_USER_TOKEN_REJECTED_MESSAGE = (
66+
"RomM rejected this token. Check you pasted it correctly and that it has the required scopes (see the plugin docs)."
67+
)
6568

6669
# Pairing-code sign-in (``establish_paired_token``). The 60s single-use pairing
6770
# code is exchanged for a token over a public endpoint; each rejection carries a
@@ -276,8 +279,10 @@ async def establish_token(
276279
minted = await self._loop.run_in_executor(None, self._mint, username, password)
277280
except RommForbiddenError:
278281
# 403 on token mint: same AUTH_FAILED slug as a 401, but a distinct
279-
# message — the account lacks token-creation permission (or a
280-
# Cloudflare bot-fight 403 at the edge), not wrong credentials.
282+
# message. RomM answers 403 indistinguishably for wrong credentials
283+
# AND for an account that lacks token-creation permission (and a
284+
# Cloudflare bot-fight 403 lands here too), so the message names both
285+
# causes rather than asserting only the permission one.
281286
self._restore_auth_state(snapshot)
282287
return {"success": False, "reason": ErrorCode.AUTH_FAILED.value, "message": _FORBIDDEN_TOKEN_MESSAGE}
283288
except Exception as e:
@@ -366,6 +371,18 @@ async def establish_user_token(
366371
except Exception as e:
367372
self._restore_auth_state(snapshot)
368373
self._romm_api.set_version(None)
374+
# RomM answers the token-carrying probe with a 500 (a malformed token)
375+
# or a 403 (a token-shaped but invalid/revoked one) instead of a clean
376+
# 401, so a bad pasted token otherwise surfaces as a generic server
377+
# error. The auth state is restored now (old token, or none), so a
378+
# reachability probe that succeeds proves the server is up and the
379+
# pasted token — not the server — is at fault.
380+
if (await self.probe_reachability()).get("online"):
381+
return {
382+
"success": False,
383+
"reason": ErrorCode.AUTH_FAILED.value,
384+
"message": _USER_TOKEN_REJECTED_MESSAGE,
385+
}
369386
return error_response(e)
370387

371388
version_error = self._version_gate_error(version)

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>

0 commit comments

Comments
 (0)