chore(ci): clear the lint debt and make lint blocking (#79)
CI ran lint with continue-on-error because 23 errors predated it. All are fixed, so the flag is gone and a typecheck step is added beside it — nothing else in CI compiles the test files, so a type error in a fixture could only ever be found on a developer's machine. Real fixes, no blanket disables and no suppressions: - react-hooks/set-state-in-effect (6): each was "reset or seed state when an input changes", so each becomes a state adjustment during render behind a previous-value guard — the documented React pattern for it. Two were also latent bugs. transactions/page.tsx depended on `others`, rebuilt by .filter() every render, so the effect already ran every render, one commit late. shared/page.tsx depended on the `tagIds` array identity, so the row window was reset more often than "changing any filter" claims — including after "Show more", the one interaction it must not undo; the key now joins by value. csv-import-modal reads localStorage once at mount, which is a lazy useState initializer, not an effect (SSR-guarded, though the modal is mounted from client state and never prerenders). - react-hooks/static-components (3): SortIcon was declared inside SharedPage, making it a new component type on every render. Hoisted to module scope with the sort state passed as props. - @typescript-eslint/no-explicit-any (9): the dynamically imported route handlers in two integration tests. Typed as their real signatures with one `as unknown as NextRequest` cast at the fixture boundary, so the call sites and response bodies get checked instead of opted out. - react/no-unescaped-entities (5): exact-equivalent entities (', "), so no rendered text changes. Also fixes the one pre-existing tsc error (unit/order-ingestion.test.ts:511): `options` is optional, and `?? 0` keeps the assertion failing on an absent list rather than silencing it with a non-null bang. 9 warnings remain (no-unused-vars, exhaustive-deps). eslint exits 0 on warnings, so lint blocks on errors only — deliberately, since none of the remaining warnings should stop a push.
This commit is contained in:
+12
-4
@@ -22,10 +22,18 @@ jobs:
|
||||
# the normal colour.
|
||||
- name: Generate Prisma client
|
||||
run: npx prisma generate
|
||||
# Advisory until the pre-existing lint debt is cleared (2026-07-19:
|
||||
# ~20 errors across budget/insights/shared pages) — then make blocking.
|
||||
- name: Lint (advisory)
|
||||
# Blocking since #79. It was advisory from 2026-07-19 while 23 errors sat
|
||||
# across the pages and the integration tests; those are now fixed, so a
|
||||
# new one must not be able to slip in behind a green tick. Warnings are
|
||||
# still allowed (eslint exits 0 on them) — there are 9 left, all
|
||||
# `no-unused-vars` or `exhaustive-deps` advisories, none of which should
|
||||
# stop a push.
|
||||
- name: Lint
|
||||
run: npm run lint
|
||||
continue-on-error: true
|
||||
# Typecheck is separate from `next build`: nothing else in CI compiles the
|
||||
# test files, so a type error in a fixture could only ever be found on a
|
||||
# developer's machine.
|
||||
- name: Typecheck
|
||||
run: npx tsc --noEmit
|
||||
- name: Unit tests
|
||||
run: npm test
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { describe, it, expect, beforeAll } from "vitest";
|
||||
import { readFileSync } from "fs";
|
||||
import { resolve } from "path";
|
||||
import type { NextRequest } from "next/server";
|
||||
import { queryRaw } from "../../lib/db";
|
||||
|
||||
/**
|
||||
@@ -13,13 +14,22 @@ const dir = resolve(__dirname, "../fixtures/orders/real");
|
||||
const html = (f: string) => readFileSync(resolve(dir, `${f}.html`), "utf-8");
|
||||
|
||||
const TOKEN = "test-ingest-token";
|
||||
let POST: any;
|
||||
|
||||
// The handler is imported dynamically (after the env var is set), so its type
|
||||
// is declared here rather than inferred. `unknown` for the request is
|
||||
// deliberate: these fixtures are the two properties the route actually reads,
|
||||
// not a whole NextRequest, and widening to `any` would stop the compiler
|
||||
// checking the call sites too.
|
||||
let POST: (req: NextRequest) => Promise<Response>;
|
||||
|
||||
// One cast, at the boundary, instead of `any` everywhere downstream: the
|
||||
// fixture is the two properties the route actually reads, and everything past
|
||||
// this line stays type-checked.
|
||||
const req = (body: unknown, token: string | null = TOKEN) =>
|
||||
({
|
||||
headers: { get: (h: string) => (h === "x-ingest-token" ? token : null) },
|
||||
json: async () => body,
|
||||
}) as any;
|
||||
}) as unknown as NextRequest;
|
||||
|
||||
beforeAll(async () => {
|
||||
process.env.ORDER_INGEST_TOKEN = TOKEN;
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { describe, it, expect, beforeAll, beforeEach, vi } from "vitest";
|
||||
import type { NextRequest } from "next/server";
|
||||
import { createPool, mockDbWithPool, resetDB } from "./helpers";
|
||||
|
||||
/**
|
||||
@@ -12,15 +13,41 @@ import { createPool, mockDbWithPool, resetDB } from "./helpers";
|
||||
const pool = createPool();
|
||||
mockDbWithPool(pool);
|
||||
|
||||
let GET: any;
|
||||
let PUT: any;
|
||||
/** What the review route returns, to the extent these tests read it. */
|
||||
type ItemVerdict = { item: string; verdict: string };
|
||||
type Review = {
|
||||
participant_id: number;
|
||||
rating: string | null;
|
||||
note: string | null;
|
||||
order_again: boolean | null;
|
||||
item_verdicts: ItemVerdict[];
|
||||
};
|
||||
type ReviewBody = {
|
||||
reviews: Review[];
|
||||
splits: { participant_id: number; share_percent: number }[];
|
||||
merchant: {
|
||||
warn: boolean;
|
||||
note: string | null;
|
||||
counts: Record<string, number>;
|
||||
history: { transaction_id: number }[];
|
||||
items: { item: string; loved: number; never: number }[];
|
||||
} | null;
|
||||
};
|
||||
type Ctx = { params: Promise<{ id: string }> };
|
||||
|
||||
// Declared rather than inferred because the route is imported dynamically,
|
||||
// after vi.doMock. `unknown` for the request keeps the fixture honest — it is
|
||||
// the two properties the handler reads, not a NextRequest.
|
||||
let GET: (req: NextRequest, ctx: Ctx) => Promise<Response>;
|
||||
let PUT: (req: NextRequest, ctx: Ctx) => Promise<Response>;
|
||||
let ownerId: number;
|
||||
let otherId: number;
|
||||
let txnId: number;
|
||||
|
||||
const req = (body?: unknown) =>
|
||||
({ headers: { get: () => null }, json: async () => body }) as any;
|
||||
const params = (id: number) => ({ params: Promise.resolve({ id: String(id) }) });
|
||||
({ headers: { get: () => null }, json: async () => body }) as unknown as NextRequest;
|
||||
const params = (id: number): Ctx => ({ params: Promise.resolve({ id: String(id) }) });
|
||||
const bodyOf = async (res: Response): Promise<ReviewBody> => (await res.json()) as ReviewBody;
|
||||
|
||||
beforeAll(async () => {
|
||||
vi.doMock("@/lib/auth", () => ({
|
||||
@@ -59,25 +86,25 @@ describe("order verdicts — one per person", () => {
|
||||
it("keeps both people's verdicts on the same order", async () => {
|
||||
await PUT(req({ participant_id: ownerId, rating: "loved" }), params(txnId));
|
||||
const res = await PUT(req({ participant_id: otherId, rating: "never" }), params(txnId));
|
||||
const body = await res.json();
|
||||
const body = await bodyOf(res);
|
||||
|
||||
// The bug this guards: a UNIQUE on transaction_id alone made the second
|
||||
// save overwrite the first, and the disagreement is the useful part.
|
||||
expect(body.reviews).toHaveLength(2);
|
||||
expect(body.reviews.find((r: any) => r.participant_id === ownerId).rating).toBe("loved");
|
||||
expect(body.reviews.find((r: any) => r.participant_id === otherId).rating).toBe("never");
|
||||
expect(body.reviews.find((r) => r.participant_id === ownerId)?.rating).toBe("loved");
|
||||
expect(body.reviews.find((r) => r.participant_id === otherId)?.rating).toBe("never");
|
||||
});
|
||||
|
||||
it("defaults the verdict to the signed-in user, not the owner", async () => {
|
||||
const res = await PUT(req({ rating: "ok" }), params(txnId));
|
||||
const body = await res.json();
|
||||
const body = await bodyOf(res);
|
||||
expect(body.reviews[0].participant_id).toBe(ownerId);
|
||||
});
|
||||
|
||||
it("revising a verdict updates rather than duplicating", async () => {
|
||||
await PUT(req({ participant_id: ownerId, rating: "loved" }), params(txnId));
|
||||
const res = await PUT(req({ participant_id: ownerId, rating: "never" }), params(txnId));
|
||||
const body = await res.json();
|
||||
const body = await bodyOf(res);
|
||||
expect(body.reviews).toHaveLength(1);
|
||||
expect(body.reviews[0].rating).toBe("never");
|
||||
});
|
||||
@@ -88,12 +115,12 @@ describe("order verdicts — one per person", () => {
|
||||
});
|
||||
|
||||
it("derives order_again from the rating", async () => {
|
||||
let body = await (await PUT(req({ rating: "never" }), params(txnId))).json();
|
||||
let body = await bodyOf(await PUT(req({ rating: "never" }), params(txnId)));
|
||||
expect(body.reviews[0].order_again).toBe(false);
|
||||
body = await (await PUT(req({ rating: "ok" }), params(txnId))).json();
|
||||
body = await bodyOf(await PUT(req({ rating: "ok" }), params(txnId)));
|
||||
expect(body.reviews[0].order_again).toBe(true);
|
||||
// "bad" is also a no — you would not choose it again.
|
||||
body = await (await PUT(req({ rating: "bad" }), params(txnId))).json();
|
||||
body = await bodyOf(await PUT(req({ rating: "bad" }), params(txnId)));
|
||||
expect(body.reviews[0].order_again).toBe(false);
|
||||
});
|
||||
|
||||
@@ -104,13 +131,13 @@ describe("order verdicts — one per person", () => {
|
||||
const older = await seedOrder("Thai Palace", "2026-06-01");
|
||||
await PUT(req({ participant_id: ownerId, rating: "bad" }), params(older));
|
||||
|
||||
let body = await (await GET(req(), params(txnId))).json();
|
||||
expect(body.merchant.counts.bad).toBe(1);
|
||||
expect(body.merchant.warn).toBe(false);
|
||||
let body = await bodyOf(await GET(req(), params(txnId)));
|
||||
expect(body.merchant!.counts.bad).toBe(1);
|
||||
expect(body.merchant!.warn).toBe(false);
|
||||
|
||||
await PUT(req({ participant_id: ownerId, rating: "never" }), params(older));
|
||||
body = await (await GET(req(), params(txnId))).json();
|
||||
expect(body.merchant.warn).toBe(true);
|
||||
body = await bodyOf(await GET(req(), params(txnId)));
|
||||
expect(body.merchant!.warn).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -125,7 +152,7 @@ describe("item verdicts", () => {
|
||||
);
|
||||
// No item_verdicts key at all — the shape a note-only form sends.
|
||||
const res = await PUT(req({ rating: "liked", note: "slow delivery" }), params(txnId));
|
||||
const body = await res.json();
|
||||
const body = await bodyOf(res);
|
||||
expect(body.reviews[0].item_verdicts).toEqual([
|
||||
{ item: "Pad Thai", verdict: "loved" },
|
||||
]);
|
||||
@@ -138,7 +165,7 @@ describe("item verdicts", () => {
|
||||
params(txnId)
|
||||
);
|
||||
const res = await PUT(req({ rating: "liked", item_verdicts: [] }), params(txnId));
|
||||
const body = await res.json();
|
||||
const body = await bodyOf(res);
|
||||
expect(body.reviews[0].item_verdicts).toEqual([]);
|
||||
});
|
||||
|
||||
@@ -154,7 +181,7 @@ describe("item verdicts", () => {
|
||||
}),
|
||||
params(txnId)
|
||||
);
|
||||
const body = await res.json();
|
||||
const body = await bodyOf(res);
|
||||
expect(body.reviews[0].rating).toBe("ok");
|
||||
expect(body.reviews[0].item_verdicts).toEqual([
|
||||
{ item: "Pad Thai", verdict: "loved" },
|
||||
@@ -167,10 +194,10 @@ describe("merchant history", () => {
|
||||
const older = await seedOrder("Thai Palace", "2026-06-01");
|
||||
await PUT(req({ participant_id: ownerId, rating: "never", note: "cold" }), params(older));
|
||||
|
||||
const body = await (await GET(req(), params(txnId))).json();
|
||||
expect(body.merchant.warn).toBe(true);
|
||||
expect(body.merchant.counts.never).toBe(1);
|
||||
expect(body.merchant.history.map((h: any) => h.transaction_id)).toEqual([older]);
|
||||
const body = await bodyOf(await GET(req(), params(txnId)));
|
||||
expect(body.merchant!.warn).toBe(true);
|
||||
expect(body.merchant!.counts.never).toBe(1);
|
||||
expect(body.merchant!.history.map((h) => h.transaction_id)).toEqual([older]);
|
||||
});
|
||||
|
||||
it("treats the same restaurant as one merchant across platforms", async () => {
|
||||
@@ -181,17 +208,17 @@ describe("merchant history", () => {
|
||||
const shouty = await seedOrder("THAI PALACE", "2026-06-01");
|
||||
await PUT(req({ participant_id: ownerId, rating: "never" }), params(shouty));
|
||||
|
||||
const body = await (await GET(req(), params(txnId))).json();
|
||||
expect(body.merchant.warn).toBe(true);
|
||||
expect(body.merchant.history.map((h: any) => h.transaction_id)).toContain(shouty);
|
||||
const body = await bodyOf(await GET(req(), params(txnId)));
|
||||
expect(body.merchant!.warn).toBe(true);
|
||||
expect(body.merchant!.history.map((h) => h.transaction_id)).toContain(shouty);
|
||||
});
|
||||
|
||||
it("does not carry a verdict across different merchants", async () => {
|
||||
const other = await seedOrder("Pizza Place", "2026-06-01");
|
||||
await PUT(req({ participant_id: ownerId, rating: "never" }), params(other));
|
||||
|
||||
const body = await (await GET(req(), params(txnId))).json();
|
||||
expect(body.merchant.warn).toBe(false);
|
||||
const body = await bodyOf(await GET(req(), params(txnId)));
|
||||
expect(body.merchant!.warn).toBe(false);
|
||||
});
|
||||
|
||||
it("pools item opinions across the merchant's orders", async () => {
|
||||
@@ -206,10 +233,10 @@ describe("merchant history", () => {
|
||||
params(older2)
|
||||
);
|
||||
|
||||
const body = await (await GET(req(), params(txnId))).json();
|
||||
const body = await bodyOf(await GET(req(), params(txnId)));
|
||||
// Case-folded: the same dish comes back capitalised differently between
|
||||
// receipts, and two entries for one dish is not a track record.
|
||||
expect(body.merchant.items).toEqual([{ item: "Pad Thai", loved: 2, never: 0 }]);
|
||||
expect(body.merchant!.items).toEqual([{ item: "Pad Thai", loved: 2, never: 0 }]);
|
||||
});
|
||||
|
||||
it("keeps item opinions from reviews that have no overall rating", async () => {
|
||||
@@ -218,14 +245,14 @@ describe("merchant history", () => {
|
||||
req({ item_verdicts: [{ item: "Satay", verdict: "never" }] }),
|
||||
params(older)
|
||||
);
|
||||
const body = await (await GET(req(), params(txnId))).json();
|
||||
expect(body.merchant.items).toEqual([{ item: "Satay", loved: 0, never: 1 }]);
|
||||
const body = await bodyOf(await GET(req(), params(txnId)));
|
||||
expect(body.merchant!.items).toEqual([{ item: "Satay", loved: 0, never: 1 }]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("share state", () => {
|
||||
it("reports splits so the panel can show shared vs just me", async () => {
|
||||
let body = await (await GET(req(), params(txnId))).json();
|
||||
let body = await bodyOf(await GET(req(), params(txnId)));
|
||||
expect(body.splits).toEqual([]);
|
||||
|
||||
await pool.query(
|
||||
@@ -233,7 +260,7 @@ describe("share state", () => {
|
||||
VALUES ($1, $2, 50)`,
|
||||
[txnId, otherId]
|
||||
);
|
||||
body = await (await GET(req(), params(txnId))).json();
|
||||
body = await bodyOf(await GET(req(), params(txnId)));
|
||||
expect(body.splits).toHaveLength(1);
|
||||
expect(body.splits[0].participant_id).toBe(otherId);
|
||||
});
|
||||
|
||||
@@ -508,7 +508,9 @@ describe("HTML entities in line items", () => {
|
||||
// The ampersand decodes, and the bullet becomes a boundary, not text.
|
||||
expect(item!.description).toContain("Sweet & Sour Crunch");
|
||||
expect(item!.description).not.toContain("•");
|
||||
expect(item!.options.length).toBeGreaterThan(0);
|
||||
// `options` is optional on the parsed item; `?? 0` keeps the assertion honest
|
||||
// (an absent list fails) instead of silencing the check with a non-null bang.
|
||||
expect(item!.options?.length ?? 0).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("leaves a literal &bull; alone rather than turning it into a bullet", () => {
|
||||
|
||||
@@ -141,7 +141,7 @@ export default function ReconcilePage() {
|
||||
<div className="border border-zinc-800 rounded-xl overflow-hidden">
|
||||
<div className="px-4 py-3 bg-zinc-900/60 border-b border-zinc-800">
|
||||
<p className="text-sm font-medium text-zinc-400">No statement matches found ({noMatches.length})</p>
|
||||
<p className="text-xs text-zinc-600 mt-0.5">These may not have hit a statement yet, or the statement hasn't been imported.</p>
|
||||
<p className="text-xs text-zinc-600 mt-0.5">These may not have hit a statement yet, or the statement hasn't been imported.</p>
|
||||
</div>
|
||||
<div className="divide-y divide-zinc-800">
|
||||
{noMatches.map((tx) => (
|
||||
|
||||
+26
-9
@@ -432,6 +432,19 @@ const SharedTxRow = memo(function SharedTxRow({
|
||||
// ── Main page ─────────────────────────────────────────────────────────────────
|
||||
type SortCol = "transaction_date" | "created_at" | "amount";
|
||||
|
||||
/**
|
||||
* Sort direction marker for a column header.
|
||||
*
|
||||
* Declared at module scope and given the sort state as props, rather than
|
||||
* closing over it inside SharedPage: a component defined during render is a
|
||||
* new type on every render, so React unmounts and remounts it each time
|
||||
* instead of updating it.
|
||||
*/
|
||||
function SortIcon({ col, sortCol, sortDir }: { col: SortCol; sortCol: SortCol; sortDir: "asc" | "desc" }) {
|
||||
if (sortCol !== col) return <span className="text-zinc-600 ml-0.5">↕</span>;
|
||||
return <span className="ml-0.5">{sortDir === "desc" ? "↓" : "↑"}</span>;
|
||||
}
|
||||
|
||||
export default function SharedPage() {
|
||||
const [tagIds, setTagIds] = useState<string[]>([]);
|
||||
const [participantId, setParticipantId] = useState<number | undefined>(undefined);
|
||||
@@ -478,10 +491,6 @@ export default function SharedPage() {
|
||||
else { setSortCol(col); setSortDir("desc"); }
|
||||
}
|
||||
|
||||
function SortIcon({ col }: { col: SortCol }) {
|
||||
if (sortCol !== col) return <span className="text-zinc-600 ml-0.5">↕</span>;
|
||||
return <span className="ml-0.5">{sortDir === "desc" ? "↓" : "↑"}</span>;
|
||||
}
|
||||
const { data: balances = [], isLoading: balLoading } = useParticipantBalances(realTagIds);
|
||||
const { data: allTags = [] } = useTags();
|
||||
const { data: me } = useCurrentUser();
|
||||
@@ -504,9 +513,17 @@ export default function SharedPage() {
|
||||
// table was what made expanding a receipt freeze the browser. "Show more"
|
||||
// extends the window; changing any filter resets it.
|
||||
const [visibleCount, setVisibleCount] = useState(100);
|
||||
useEffect(() => {
|
||||
// Reset during render behind a previous-value guard rather than in an effect.
|
||||
// Note the key joins `tagIds` by value: as a raw dependency it was a fresh
|
||||
// array on many renders, so the window was being reset more often than
|
||||
// "changing any filter" claims — including after "Show more", which is the
|
||||
// one interaction this must not undo.
|
||||
const windowKey = `${search}|${sortCol}|${sortDir}|${tagIds.join(",")}|${participantId ?? ""}`;
|
||||
const [lastWindowKey, setLastWindowKey] = useState(windowKey);
|
||||
if (windowKey !== lastWindowKey) {
|
||||
setLastWindowKey(windowKey);
|
||||
setVisibleCount(100);
|
||||
}, [search, sortCol, sortDir, tagIds, participantId]);
|
||||
}
|
||||
const visible = (transactions as SharedTransactionRow[]).slice(0, visibleCount);
|
||||
|
||||
// Receipt expansion, same pattern as the transactions page. The shared
|
||||
@@ -667,13 +684,13 @@ export default function SharedPage() {
|
||||
className="text-left px-4 py-2 text-xs text-zinc-500 font-medium cursor-pointer hover:text-white whitespace-nowrap"
|
||||
onClick={() => toggleSort("transaction_date")}
|
||||
>
|
||||
Date <SortIcon col="transaction_date" />
|
||||
Date <SortIcon col="transaction_date" sortCol={sortCol} sortDir={sortDir} />
|
||||
</th>
|
||||
<th
|
||||
className="text-left px-4 py-2 text-xs text-zinc-500 font-medium cursor-pointer hover:text-white whitespace-nowrap"
|
||||
onClick={() => toggleSort("created_at")}
|
||||
>
|
||||
Imported <SortIcon col="created_at" />
|
||||
Imported <SortIcon col="created_at" sortCol={sortCol} sortDir={sortDir} />
|
||||
</th>
|
||||
<th className="text-left px-4 py-2 text-xs text-zinc-500 font-medium sticky left-0 z-10 bg-zinc-900 border-r border-zinc-800/80">Description</th>
|
||||
<th className="text-left px-4 py-2 text-xs text-zinc-500 font-medium">Category</th>
|
||||
@@ -681,7 +698,7 @@ export default function SharedPage() {
|
||||
className="text-right px-4 py-2 text-xs text-zinc-500 font-medium cursor-pointer hover:text-white"
|
||||
onClick={() => toggleSort("amount")}
|
||||
>
|
||||
Amount <SortIcon col="amount" />
|
||||
Amount <SortIcon col="amount" sortCol={sortCol} sortDir={sortDir} />
|
||||
</th>
|
||||
{/* Paid by sits next to Splits deliberately: together they are the
|
||||
two halves of the question this page exists to answer — whose
|
||||
|
||||
@@ -59,7 +59,7 @@ function ConvertModal({
|
||||
<div className="px-6 pt-5 pb-4 border-b border-zinc-800">
|
||||
<div className="flex items-center gap-2">
|
||||
<span className="w-3 h-3 rounded-full flex-shrink-0" style={{ backgroundColor: tag.color }} />
|
||||
<h3 className="font-semibold text-sm text-zinc-300">Convert "{tag.name}" to Trip</h3>
|
||||
<h3 className="font-semibold text-sm text-zinc-300">Convert "{tag.name}" to Trip</h3>
|
||||
</div>
|
||||
<p className="text-xs text-zinc-500 mt-1">
|
||||
Creates a trip with the same name and color, and assigns all {tag.transaction_count} tagged transaction{tag.transaction_count !== 1 ? "s" : ""} to it.
|
||||
|
||||
@@ -148,7 +148,7 @@ function SaveAsRulePrompt({
|
||||
Automatically apply this {field === "category" ? "category" : "merchant name"} to future matching transactions.
|
||||
</p>
|
||||
<div className="bg-zinc-900 rounded-lg px-3 py-2 text-xs text-zinc-300 mb-3 space-y-1">
|
||||
<p><span className="text-zinc-500">If</span> {conditionField === "merchant_normalized" ? "merchant" : "description"} {conditionOperator} <span className="text-white">"{conditionValue}"</span></p>
|
||||
<p><span className="text-zinc-500">If</span> {conditionField === "merchant_normalized" ? "merchant" : "description"} {conditionOperator} <span className="text-white">"{conditionValue}"</span></p>
|
||||
<p>
|
||||
<span className="text-zinc-500">Then</span>{" "}
|
||||
{field === "category"
|
||||
@@ -250,11 +250,17 @@ function MarkAsPaymentModal({
|
||||
|
||||
const [participantId, setParticipantId] = useState<number | "">(others[0]?.id ?? "");
|
||||
|
||||
useEffect(() => {
|
||||
// Seed the picker once the participant list arrives — the initial state is
|
||||
// computed before the query resolves, so it starts empty.
|
||||
//
|
||||
// Adjusted during render rather than in an effect, which is also strictly
|
||||
// more correct: `others` is rebuilt by .filter() on every render, so as a
|
||||
// dependency it never compared equal and the effect ran every render anyway
|
||||
// — just one commit later, painting an empty select first. The condition
|
||||
// clears itself on the first non-empty list, so this cannot loop.
|
||||
if (participantId === "" && others.length > 0) {
|
||||
setParticipantId(others[0].id);
|
||||
}
|
||||
}, [others]);
|
||||
// For credits/refunds the default direction is "they paid me"
|
||||
const [direction, setDirection] = useState<"received" | "sent">(
|
||||
SPEND_TYPES.has(transaction.transaction_type) ? "sent" : "received"
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
"use client";
|
||||
|
||||
import { useState, useRef, useEffect } from "react";
|
||||
import { useState, useRef } from "react";
|
||||
import { CATEGORIES, formatCategory } from "@/lib/categories";
|
||||
import { useImportCSV, useStatementCoverage } from "@/lib/hooks";
|
||||
import {
|
||||
@@ -54,13 +54,23 @@ export function CsvImportModal({ onClose }: { onClose: () => void }) {
|
||||
const [savePreset, setSavePreset] = useState(false);
|
||||
const [editedRows, setEditedRows] = useState<ParsedTransaction[]>([]);
|
||||
const [error, setError] = useState("");
|
||||
const [presets, setPresets] = useState<BankPreset[]>([]);
|
||||
// Read once, at mount, from localStorage. A lazy initializer rather than an
|
||||
// effect: it runs exactly once per mount — the same semantics as
|
||||
// `useEffect(…, [])` — but before the first paint, so the preset dropdown
|
||||
// does not appear empty and then populate. Nothing in this component writes
|
||||
// `presets` afterwards, so there is no state to keep in sync.
|
||||
//
|
||||
// The `typeof window` guard is for safety only: this modal is mounted from
|
||||
// client state and so is never server-rendered. If that ever changes, it
|
||||
// degrades to an empty list instead of throwing on `localStorage`.
|
||||
const [presets] = useState<BankPreset[]>(() =>
|
||||
typeof window === "undefined" ? [] : loadBankPresets()
|
||||
);
|
||||
const [insertedCount, setInsertedCount] = useState(0);
|
||||
const [coveredRows, setCoveredRows] = useState<ParsedTransaction[]>([]);
|
||||
const [showCovered, setShowCovered] = useState(false);
|
||||
const coverage = useStatementCoverage();
|
||||
|
||||
useEffect(() => { setPresets(loadBankPresets()); }, []);
|
||||
|
||||
function handleFile(file: File) {
|
||||
setError("");
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
"use client";
|
||||
|
||||
import { useEffect, useMemo, useState } from "react";
|
||||
import { useMemo, useState } from "react";
|
||||
import { useRuleMatches, useApplyRuleToSelection, type RuleMatchChange } from "@/lib/hooks";
|
||||
import { formatCategory } from "@/lib/categories";
|
||||
|
||||
@@ -39,9 +39,17 @@ export function RulePreviewModal({
|
||||
|
||||
// Default to everything the rule would change — the common case is "yes, all
|
||||
// of these", with deselection as the exception.
|
||||
useEffect(() => {
|
||||
if (data?.transactions) setSelected(new Set(data.transactions.map((t) => t.id)));
|
||||
}, [data]);
|
||||
//
|
||||
// Adjusted during render behind a previous-value guard rather than in an
|
||||
// effect. In an effect the modal painted once with nothing ticked and then
|
||||
// again with everything ticked; here the seeded selection is part of the
|
||||
// first commit. The guard is on `data` identity, so a user deselecting a row
|
||||
// is not undone on the next render — only a genuinely new preview reseeds.
|
||||
const [seededFrom, setSeededFrom] = useState<typeof data>(undefined);
|
||||
if (data !== seededFrom) {
|
||||
setSeededFrom(data);
|
||||
setSelected(new Set(data?.transactions?.map((t) => t.id) ?? []));
|
||||
}
|
||||
|
||||
const rows = data?.transactions ?? [];
|
||||
const allSelected = rows.length > 0 && selected.size === rows.length;
|
||||
|
||||
@@ -82,10 +82,19 @@ export function Sidebar() {
|
||||
const pathname = usePathname();
|
||||
const [open, setOpen] = useState(false);
|
||||
|
||||
// Close drawer on route change
|
||||
useEffect(() => {
|
||||
// Close the drawer on route change.
|
||||
//
|
||||
// Adjusting state during render behind a previous-value guard — the
|
||||
// documented React pattern for "reset state when a prop changes" — rather
|
||||
// than in an effect: React re-runs this component immediately and never
|
||||
// commits the intermediate DOM, so there is no cascading render. Putting it
|
||||
// in each Link's onClick is the other candidate fix and is worse here: it
|
||||
// misses browser back/forward, where the route changes with no click.
|
||||
const [lastPathname, setLastPathname] = useState(pathname);
|
||||
if (pathname !== lastPathname) {
|
||||
setLastPathname(pathname);
|
||||
setOpen(false);
|
||||
}, [pathname]);
|
||||
}
|
||||
|
||||
// Prevent body scroll when drawer is open
|
||||
useEffect(() => {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
"use client";
|
||||
|
||||
import { useState, useEffect } from "react";
|
||||
import { useState } from "react";
|
||||
import { useParticipants, useSetSplits, useTransactionSplits, useBulkAction, useCreateRule, useCurrentUser } from "@/lib/hooks";
|
||||
|
||||
interface Split {
|
||||
@@ -33,22 +33,32 @@ export function SplitModal({ transactionId, transactionIds, amount, description,
|
||||
const [saveAsRule, setSaveAsRule] = useState(false);
|
||||
const [ruleSaved, setRuleSaved] = useState(false);
|
||||
|
||||
// Initialise: bulk always defaults to 100% Me; single loads existing splits
|
||||
useEffect(() => {
|
||||
if (!participants || participants.length === 0 || !currentUser) return;
|
||||
if (isBulk) {
|
||||
setSplitsState([{ participant_id: currentUser.id, share_percent: 100 }]);
|
||||
} else if (existingSplits && existingSplits.length > 0) {
|
||||
// Initialise: bulk always defaults to 100% Me; single loads existing splits.
|
||||
//
|
||||
// Seeded during render behind a previous-value guard rather than in an
|
||||
// effect. Both queries resolve after the first paint, so as an effect this
|
||||
// rendered an empty split table and then replaced it — and, worse, it re-ran
|
||||
// on every identity change of `participants`/`existingSplits`, which would
|
||||
// discard edits in progress if either query refetched while the modal was
|
||||
// open. Keying on the inputs that actually define the seed fixes both.
|
||||
const seedKey =
|
||||
!participants || participants.length === 0 || !currentUser
|
||||
? null
|
||||
: `${isBulk ? "bulk" : singleId}:${currentUser.id}:${
|
||||
existingSplits ? JSON.stringify(existingSplits) : "none"
|
||||
}`;
|
||||
const [seededFrom, setSeededFrom] = useState<string | null>(null);
|
||||
if (seedKey !== null && seedKey !== seededFrom) {
|
||||
setSeededFrom(seedKey);
|
||||
setSplitsState(
|
||||
existingSplits.map((s: { participant_id: number; share_percent: number }) => ({
|
||||
!isBulk && existingSplits && existingSplits.length > 0
|
||||
? existingSplits.map((s: { participant_id: number; share_percent: number }) => ({
|
||||
participant_id: s.participant_id,
|
||||
share_percent: Number(s.share_percent),
|
||||
}))
|
||||
: [{ participant_id: currentUser!.id, share_percent: 100 }]
|
||||
);
|
||||
} else {
|
||||
setSplitsState([{ participant_id: currentUser.id, share_percent: 100 }]);
|
||||
}
|
||||
}, [existingSplits, participants, isBulk, currentUser]);
|
||||
|
||||
const total = splits.reduce((sum, s) => sum + s.share_percent, 0);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user