docs/google-sign-in-plan
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 synAdminandClaude Opus 5
|
5bba28bfda |
fix(tests): the error-handling suite would not compile, and nothing local said so (#307)
The trigger added in #322 does not typecheck. pg declares query with several overloads and jest.spyOn resolves the mock argument against the last of them, whose parameter list is empty, so the inferred type of a rejection value is never and an Error cannot be assigned to it. ts-jest compiles each suite as it runs, so this surfaced as a suite that failed to run rather than as a test that failed — which is why CI reported 446 tests passing, zero failing, and the step red anyway. The cast is the type system rather than a shortcut, and says so in the file: every overload rejects on failure, and the cast only chooses which one to check against. The reason this reached main is the part worth keeping. The backend has a tsconfig.test.json covering scripts and tests, and nothing was running it — build compiles src alone, and I had been reporting tsc clean on that basis while touching test files it never looked at. The frontend build has run its equivalent all along, so the gap was one workspace wide and invisible from the other. It is now a script, typecheck:tests, and it reproduces this failure in seconds against no database. That matters beyond this fix: the standing excuse for integration regressions here has been that the suite needs Postgres and a Node this machine cannot run, and a type error in a test was never actually in that category — it only looked like it because the check that finds it was never invoked. Not added to build. The Dockerfile runs that, and a production image should not fail to build because a test file has a type error. Verified: typecheck:tests clean, build tsc clean, lint 0 errors with no new warnings, 485 unit tests passing. The suite itself still needs CI to run. Refs #307 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
8956b9f122 |
fix(items): read the id strictly, and stop the error test needing a route that does not (#307)
Item 5. GET /api/items/:id was the last route reading its id with a bare Number(), so an unreadable id reached Postgres and came back to the caller as a 500 for an item that cannot exist. It answers 404 now, like every other id-taking route since #207. It could not be fixed on its own, which is why it stayed. errorHandling.integration.test.ts used this route's looseness as its way of making a handler reject: tightening the parse would have left that test green while removing the thing it tests. So the test now fails a database call directly, with a spy on pool.query against a route that makes one. That is the failure the error middleware actually exists for, and it does not depend on any route declining to validate — the previous comment's own conclusion, that moving the trigger to cart.ts would only move the wart. Two things fixed along the way that the issue asked about but that switching to readId would not have delivered on its own. readId was not as strict as its name suggests. Number reads 5.0, 1e2, 0x10 and +5 as 5, 100, 16 and 5 — every one a positive integer, so every check readId made passed and the route fetched a real row for a URL nobody wrote. /items/5.0 answered with item 5. This never raised an error and so never announced itself; the issue noticed it only because #308 converted the comparison to a real integer. An id is a string of digits, so it is matched against digits before being parsed. readId is also now bounded at the top of a 32-bit serial. Above that Postgres raises 22003 rather than returning nothing, which is the same wrong answer to the caller as the 22P02 the function was written to prevent — a 500 for an id that identifies nothing. The leak assertion was passing for the wrong reason. It checks the response does not contain "syntax" or "items", and the error it was checking against happened to contain both only by accident of which route was used. The injected failure now contains both words deliberately, and the whole message is asserted against as well, so a future error format cannot slip through by wording itself differently. Verified: tsc clean, lint 0 errors with no new warnings, 485 unit tests passing across 33 suites — seven of them new, covering the inputs above. The integration suite cannot run on this machine, so whether the rewritten error test passes is for CI to say. Item 4, coverage, is not closed by this and cannot be closed yet: the SonarQube scan step has been skipped on every recent run because it is gated on the earlier steps succeeding, and those steps were failing. The dashboard is therefore stale. Reported on the issue rather than guessed at. Refs #307 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
 bermudalambandClaude Opus 5
|
c77fdad2b9 |
fix: run migrations on boot and stop failures rendering as empty (#23)
The storefront showed no inventory after deploying the categories/tags release. No data was lost: the code queried categories/item_tags/ items.category_id against a database where the migration had not been run, and that failure was invisible at every layer. Three changes, each addressing one layer: Migrations now run at container start, so deployed code cannot be ahead of the schema and the easily-forgotten manual `docker exec migrate.js up` step disappears. migrate.js waits for Postgres to accept connections first, since the NAS brings the DB container up slower than the app, and still exits non-zero so a bad migration stops the container rather than serving a half-migrated schema. Express 4 does not forward a rejected async handler, and no error middleware was mounted, so a failing query never responded at all. Async routes are now wrapped and an error middleware guarantees a 500. A hung request is indistinguishable from an empty result in the UI, which is how a schema mismatch came to read as "the store has no items". The storefront now separates "request failed" from "no items" and offers a retry. fetchItems/fetchFilterOptions throw on a non-OK response rather than returning the parsed error body, which would have been set as the item list and crashed the grid on .map. Also restores the project-context update from 7fb5764, which was left out of PR #24 and ended up dangling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |