fix(e12): close what round one broke, and revert two fixes that overreached
Round two of the review loop, on round one's own diff. Two of its findings were regressions the fixes themselves introduced, which is exactly what the loop is for: **A per-user refusal was writing onto a shared row.** MediaAssets are deduped across users, and the generic failure handler puts the message on the asset — right for an upstream failure, wrong for "your storage is full". Two users queuing the same video share one asset, so the first user hitting their cap failed the second user's job with the first one's private reason, telling someone with an empty library to free up space. The refusal now raises `_OwnerRefused`, caught separately: the job fails, the asset goes back to `pending` for whoever else holds it. Pinned by a test that drives `_download` directly — the refusal happens before extraction, so nothing needs stubbing. **The restore scripts restarted the app, undoing the restore.** `docker compose start` re-runs the container command, and that runs `alembic upgrade head` — so on the rollback path this documents as recommended (restore the dump, THEN pin the older tag) the newer image immediately re-applied the migrations the restore had just undone, and on a FAILED restore it put the app back on a half-restored schema. Both are the opposite of why the stop was added. Nothing is restarted automatically now; the scripts say what to run and when, and the docs' snippets say the same. **Reverted rather than patched:** the OnboardingWizard toast (the wizard already renders that failure inline, next to a Retry button — three recoveries for one failure), and `useVerifyEntry`'s "each new link clears the other". The latter blanked the signed-out card's only banner, where nothing renders the reset flag to replace it. The two states are independent again; what changes is that `dismiss` peels ONE layer, which is what the original finding was actually about. Also: `Stats` uses the YouTube helper rather than the bare one, so the route's English 403 detail no longer beats the translated fallback under a Hungarian UI; `urlparse` guarded alongside `.port` (an unclosed bracket raised just as late), the reason renamed `malformed` since "bad port" misdescribes it; three docstrings narrowed to what the code actually does (the worker's byte check is per-job, not an aggregate ledger, and not on the edit path; the redirect residual leaks a reachability oracle, which the note had denied; the invalidation helper does not cover suspend); the pasted Windows recipe wrapped so `throw` can stop it; the bash one-liner's failure branch no longer exits 0; and the tests that were missing for the round-one fixes — the account panel's error paths, the invalidation set, and the shared-asset case above. Gate green: vitest 685, pytest 950.
This commit is contained in:
@@ -0,0 +1,144 @@
|
||||
// @vitest-environment jsdom
|
||||
//
|
||||
// The admin account panel's RECOVERY actions, and specifically that a refusal is visible.
|
||||
//
|
||||
// All three call `quiet: true` endpoints, which suppress the global error dialog on the promise that
|
||||
// the caller reports the failure inline. Without an `onError` that promise is broken and a refusal
|
||||
// renders as nothing at all — and on the broken-SMTP instance this panel exists for, an admin reads
|
||||
// silence as "the mail went out". These pin that each failure says something, and that the resend
|
||||
// distinguishes "this instance has no working mail" from a genuine send failure.
|
||||
//
|
||||
// i18n is a key-passthrough so assertions read stable keys; the API and the notify singleton are
|
||||
// stubbed so the panel can be driven without a server.
|
||||
import { describe, it, expect, afterEach, beforeEach, vi } from "vitest";
|
||||
import { render, screen, cleanup, waitFor, fireEvent } from "@testing-library/react";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
|
||||
vi.mock("react-i18next", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("react-i18next")>();
|
||||
return {
|
||||
...actual,
|
||||
useTranslation: () => ({ t: (key: string) => key, i18n: { language: "en" } }),
|
||||
};
|
||||
});
|
||||
|
||||
const notifyMock = vi.fn();
|
||||
vi.mock("../lib/notifications", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../lib/notifications")>();
|
||||
return { ...actual, notify: (...a: unknown[]) => notifyMock(...a) };
|
||||
});
|
||||
|
||||
const access = {
|
||||
email_verified: false,
|
||||
email_configured: true,
|
||||
is_active: false,
|
||||
is_allowed: true,
|
||||
can_activate: true,
|
||||
has_password: false,
|
||||
google_email: null,
|
||||
invite_status: null,
|
||||
};
|
||||
|
||||
const apiMock = {
|
||||
adminUserAccess: vi.fn(async () => access),
|
||||
createSetPasswordLink: vi.fn(),
|
||||
resendUserVerification: vi.fn(),
|
||||
activateUser: vi.fn(),
|
||||
};
|
||||
vi.mock("../lib/api", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../lib/api")>();
|
||||
return { ...actual, api: apiMock };
|
||||
});
|
||||
|
||||
const { HttpError } = await import("../lib/api");
|
||||
const AccountPanel = (await import("./AccountPanel")).default;
|
||||
|
||||
const user = {
|
||||
id: 7,
|
||||
email: "stuck@example.com",
|
||||
display_name: null,
|
||||
role: "user",
|
||||
is_demo: false,
|
||||
is_active: false,
|
||||
is_suspended: false,
|
||||
};
|
||||
|
||||
function mount() {
|
||||
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||
render(
|
||||
<QueryClientProvider client={qc}>
|
||||
{/* eslint-disable-next-line @typescript-eslint/no-explicit-any */}
|
||||
<AccountPanel user={user as any} onClose={() => {}} />
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
/** Click the recovery button whose label is `key`, once the readout has loaded. */
|
||||
async function clickAction(key: string) {
|
||||
mount();
|
||||
await screen.findByText("users.account.recovery");
|
||||
fireEvent.click(screen.getByText(key));
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
notifyMock.mockClear();
|
||||
Object.values(apiMock).forEach((m) => m.mockClear());
|
||||
apiMock.adminUserAccess.mockResolvedValue(access);
|
||||
});
|
||||
afterEach(cleanup);
|
||||
|
||||
const errorMessages = () =>
|
||||
notifyMock.mock.calls
|
||||
.map(([a]) => a as { level?: string; message?: string })
|
||||
.filter((a) => a.level === "error")
|
||||
.map((a) => a.message);
|
||||
|
||||
describe("the account panel reports a refused recovery action", () => {
|
||||
it("says the link could not be minted instead of leaving the button inert", async () => {
|
||||
apiMock.createSetPasswordLink.mockRejectedValue(new HttpError(404, "gone", null));
|
||||
|
||||
await clickAction("users.account.setPassword");
|
||||
|
||||
await waitFor(() => expect(errorMessages()).toContain("users.account.linkFailed"));
|
||||
// …and no link block was rendered, so the toast is the only thing that happened.
|
||||
expect(screen.queryByText("users.account.copy")).toBeNull();
|
||||
});
|
||||
|
||||
it("names the no-mail case, so a no-SMTP instance does not read as a bug", async () => {
|
||||
apiMock.resendUserVerification.mockRejectedValue(
|
||||
new HttpError(409, "email off", { code: "resend", reason: "email_off" }),
|
||||
);
|
||||
|
||||
await clickAction("users.account.resend");
|
||||
|
||||
await waitFor(() =>
|
||||
expect(errorMessages()).toContain("users.pendingAccess.resendEmailOff"),
|
||||
);
|
||||
});
|
||||
|
||||
it("reports a genuine send failure as a failure, not as a sent mail", async () => {
|
||||
apiMock.resendUserVerification.mockRejectedValue(
|
||||
new HttpError(422, "send failed", { code: "resend", reason: "send_failed" }),
|
||||
);
|
||||
|
||||
await clickAction("users.account.resend");
|
||||
|
||||
await waitFor(() => expect(errorMessages()).toContain("users.pendingAccess.resendFailed"));
|
||||
// The success toast must NOT also fire — that is the exact lie the backend guards against.
|
||||
expect(notifyMock.mock.calls.map(([a]) => (a as { message?: string }).message)).not.toContain(
|
||||
"users.account.resendSent",
|
||||
);
|
||||
});
|
||||
|
||||
it("reports a refused activation", async () => {
|
||||
apiMock.activateUser.mockRejectedValue(
|
||||
new HttpError(409, "not allowed", { code: "activate", reason: "not_allowed" }),
|
||||
);
|
||||
|
||||
await clickAction("users.account.activate");
|
||||
|
||||
await waitFor(() =>
|
||||
expect(errorMessages()).toContain("users.pendingAccess.activateFailed"),
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user