90e372d6bd4cae5273ecc2093eee1d7cbda583df
26
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 synAdminandClaude Opus 5
|
36dbf18916 |
feat(passkeys): a password reset takes the passkeys with it (#42)
The last issue in the passkeys project, and the only one that adds no capability. It is the safety net, and it exists because this is the piece most likely to be skipped and most expensive to discover missing. A reset is the recovery path, and recovery has to be complete. The reset already deletes every session on the account, on the reasoning that a reset prompted by a compromise must not leave an intruder signed in for the remaining thirty days of their cookie. A passkey an intruder registered has no expiry at all. Leaving those behind would mean a customer can recover their password and still not have their account back. The obvious objection is that this hands whoever controls the mailbox a way to strip a customer's passkeys. It does, and it costs nothing: anyone who can complete a reset already controls the email address and therefore already controls the account. The passkeys were not protecting anything by that point. Anything in flight goes too. An intruder who pressed add a passkey moments before the reset could otherwise finish that ceremony afterwards and put a credential straight back onto the account the reset had just cleared. Changing a password deliberately does not do this, and the asymmetry is the point. A change requires the current password from someone already signed in, so nothing about it suggests a lockout or a compromise, and it already spares the current session for the same reason. A customer who suspects one particular device revokes that device by name from the account page, which is a better tool than deleting everything. A reset has no idea which credential is the problem, so it takes all of them. The customer is told twice. Before, in the reset email and on the reset form, unconditionally — that form has no session and is never told whether the account has passkeys, because answering that would make the reset page an oracle for it, so the wording has to read the same to someone who has none. After, with a count, and only when the count is not zero. That moment is the only one where the count can be reported: the rows are gone by the time anyone could go and look. A customer told two were removed who only remembers registering one has just learned something they could not otherwise find out. That notice is a panel that waits to be dismissed rather than a toast, because a toast dismisses itself and this is the message a customer needs to still be looking at while they decide what to do about it. The other question this issue asks — whether a reset ends a session established by a passkey — turns out to need no code, because #39 made both paths call one createSession. But "it falls out for free" is a claim, so there is now a test that establishes a session through that exact function and watches the reset end it. docs/ops/account-recovery.md records the whole policy, including the two answers that are not code. Losing an authenticator is not a lockout: the customer signs in with their password and revokes the lost credential themselves, which is why this change implements nothing for it. Losing the email address is a lockout, and there is deliberately no self-service route out — this shop holds no second proof of identity, and anything invented to fill that gap would be a weaker credential than the one it replaced. The manual route runs through the shop owner verifying against order history, and its third step, changing the address from the admin screen, does not exist yet. That is written down as a gap with its own notification and audit questions rather than smuggled in here. Verified: backend tsc clean for src and tests, 521 unit tests pass, lint clean apart from warnings that predate this branch; frontend tsc, lint and build clean. The integration and end-to-end suites need a database this machine has no Docker for, so CI is what proves those. Closes #42 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
72e8090fd1 |
feat(passkeys): offer passkey sign-in on the login form (#41)
The point at which passkeys become visible to customers. Everything before this was reachable only by knowing the endpoints existed. Below the password form rather than above it. Passwords are how every existing customer signs in and a passkey is the alternative, so putting it first would demote the path that works for everyone. Absent entirely where WebAuthn is unavailable, rather than shown disabled. A greyed-out control invites a customer to wonder what they are missing and offers nothing they can act on, and password login is the fallback in every case regardless. The check is read once at render because it decides whether the control exists, not whether pressing it works. The passkey button has its own loading flag rather than sharing the form's. The requirement is that a dismissed prompt leaves a usable password form behind it, and a shared flag would leave that form disabled and spinning while the browser's prompt is open. Dismissing the prompt is a cancellation and shows nothing. NotAllowedError and AbortError are the two the browser raises for it, and reporting either as a failure would tell a customer something went wrong when they changed their mind — leaving a red alert sitting above a form that is working perfectly. Everything else shows a message that says what to do next rather than only that something failed. That message says nothing about whether an account exists, which costs nothing to hold to here because the server already answers every refusal identically. There is also no email on this path at all, so there is nothing to be asked about. The end-to-end test covers the half of this issue that can be proven without an authenticator. The failure is injected at the first request, before the browser prompt, so it needs no credential and cannot hang waiting for a gesture nobody will make — and then the password form behind the error is used to sign in for real. That is the requirement: not a dead end. The other half cannot be tested anywhere but production, and this issue says so itself. Credentials bind to the Relying Party ID, so a passkey registered against QA will not work against production. QA proves the flow, the fallbacks and the copy; production needs its own smoke test with a real registration afterwards, and that is a standing property of the feature rather than a gap in this change. Verified: tsc clean for src and tests, lint clean with no warnings, frontend build green. Closes #41 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
1c8763f30f |
feat(passkeys): list, add and revoke from the account page (#40)
The issue calls this the smallest one in the project and the one that makes the rest usable, and that is right: registering a passkey with no way to see or remove it is worse than not offering passkeys at all. Revocation is the row going away. #39 looks a credential up by id on every sign-in, so a deleted one is refused immediately and by construction rather than by a flag something has to remember to check. The delete is scoped to the signed-in customer in the same statement that removes the row, because a credential id is not a secret and the WHERE clause is the only thing making this safe. Reading first and deleting after would leave a window. Both "no such credential" and "not yours" answer 404. The second is the interesting case, and saying so would confirm that some other customer holds that id. The lockout check is written even though it cannot fire. password_hash is NOT NULL, so every customer has a password and removing every passkey still leaves a way in. The issue asks for the check anyway and that is the right call, because it is written against the condition rather than against today's schema — it starts holding on its own the moment the condition changes. #332 is what changes it: social sign-in makes password_hash nullable and creates the first customers with no password, and at that point this branch starts running for real. The list returns name, added and last used, and nothing else. No public key, no credential id, no counter — the customer cannot act on any of them, and the credential id is the one value that identifies an authenticator to anyone holding it. Last used is what actually tells two entries apart when the names are similar: someone about to revoke one needs to know which device they are cutting off, and a creation date does not answer that. The whole ceremony lives in customerApi rather than the component, because it is one operation: options from the server, an attestation from the browser, verification back at the server. A component holding that intermediate state could leave a challenge issued and never answered. Dismissing the browser's prompt rejects, and that is a cancellation rather than a failure. Reporting it as an error would tell a customer something went wrong when they simply changed their mind, so NotAllowedError and AbortError are swallowed and everything else is shown. The server's message on a refused revoke is shown as it arrives too — it says what to do about the last way in, and a generic message would strand the customer on a button that just does not work. The section renders nothing where WebAuthn is unavailable, rather than offering a button that cannot work. Same rule #41 applies to the login form. Verified: tsc clean for src and tests in both workspaces, backend lint at the seven pre-existing warnings with none added, frontend lint clean, 521 unit tests across 36 suites, frontend build green. Not verified: the registration ceremony needs a browser and a real authenticator, which per #41 is a standing limitation of this feature. What CI can prove is the list and revoke endpoints, which are ordinary routes. Closes #40 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
5b40ac25db |
fix(tests): regenerate the schema mirror and name the controls #56 moved (#320)
PR #317 was merged while its run was still failing, so four failures landed on main: one integration and three end-to-end. All of them are consequences of #56, and none could have been caught on the dev machine, which has no database and no browser to run those suites with. The schema mirror was never regenerated. The migration added three columns to customers and src/db-kysely/schema.ts still described the table without them, which is the drift guard from #305 doing exactly what it exists for. Hand-edited to match what kysely-codegen emits — alphabetical, and Generated on the column that has a default — because regenerating properly needs a live database. The other three are the same mistake three times: a control addressed by position, and the position moved. An unscoped getByRole('checkbox') became ambiguous once the register form had two consents. A toHaveCount(2) on the account modal's switches became three. And favoriteAlertsSwitch was getByRole('switch').last(), which did not error when a switch was appended below it — it silently retargeted, toggled analytics consent instead of favourite alerts, and then failed on a text assertion in favorites.spec.ts, naming neither the file nor the control actually at fault. The reason position was ever used is that antd's Switch renders a bare role="switch" with no accessible name; the adjacent Text is a sibling, not a label. So each one now carries an explicit aria-label and is addressed by it. That is what makes them addressable from a test, and it is what a screen reader needed regardless — the fix and the accessibility improvement are the same change. The count assertion stays, but alongside naming each switch, because a count on its own would pass if two of them were swapped for each other. Two coverage gaps closed while here, both properties the compliance work in #56 depends on and neither previously asserted anywhere a customer could see: the analytics checkbox is unchecked on the register form, and the account toggle is off for a new customer. Quebec's Law 25 s.8.1 requires profiling to start off, the integration suite asserts the server half of that, and nothing asserted the half rendered on screen. The fourth Playwright entry, the logged-out header surviving a reload, is reported flaky rather than failed and passed on retry. Left alone; it is unrelated to #56 and #257 covers flakes in this suite. Verified: backend tsc clean, frontend tsc against the test config clean, production build green, both lint suites 0 errors, 478 unit tests passing. The integration and e2e suites still cannot run here, so whether this actually clears run 875's failures is for CI to say — which is the same gap that produced them. Closes #320 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
25bec50902 |
docs(privacy): disclose every cookie and browser-storage item (#56)
Completes the consent picture the rest of this branch builds. The claim worth being able to check is that no cookie needing permission is set before it is asked for, so the policy now lists everything rather than asserting it: the rd_session sign-in cookie, which is strictly necessary and therefore exempt, the two preferences kept in localStorage and never sent anywhere, and Brevo's cookie, which cannot exist unless analytics consent was given because the script that would set it is never loaded otherwise. No cookie banner, and that is a finding rather than an omission. ePrivacy requires consent before storing anything non-essential, and this application does not store anything non-essential until the customer has asked for the feature that needs it. A banner would be asking permission for things that are either exempt or already separately consented to, which teaches people to dismiss the one consent that does matter. Described in terms of what each thing does rather than by category, since a list of cookie names tells a customer nothing they can act on. Refs #56 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
955049eac9 |
fix(privacy): separate analytics consent from email consent (#56)
The previous commit widened the marketing consent sentence to cover the Brevo tracker, so one checkbox carried both purposes. That is the specific pattern GDPR rejects: consent has to be granular, and current EDPB guidance treats bundling tracking consent with subscription consent as invalid because the customer cannot accept one purpose and refuse the other. Quebec's Law 25 s.8.1 is stricter again — profiling technology has to be off until the person switches it on, with no pre-ticked box and no consent inherited from agreeing to something else. Building to both standards was the decision, since the storefront is publicly reachable and anyone can register. So the marketing sentence is restored to exactly what it was, which leaves every existing email consent valid and untouched, and analytics gets its own column, its own sentence, its own checkbox at registration, its own toggle in the account page and its own endpoint. A customer can now hold either, both, or neither, and withdrawing one does not disturb the other. The migration defaults analytics_consent to false, which is both the honest answer — none of the existing customers was ever asked — and what Law 25 requires. Nothing about this change opts anybody in. Two details that are compliance requirements rather than wording preferences. The sentence names Brevo instead of saying "our email provider", because informed consent means the customer can tell who receives their data and a description they cannot act on is not disclosure. And the account toggle is as prominent and as easy to switch off as it is to switch on, because withdrawal has to be as easy as consenting. The analytics endpoint is separate from the marketing one rather than a second field on it, so that a single call cannot change an answer the customer did not touch — the bundling problem moved from the form into the API. The unit tests now assert the two consents stay apart in both directions, including that the marketing sentence still says nothing about tracking, because re-bundling them would otherwise pass silently and is the mistake this project already made once. Verified: backend tsc clean, both lint suites 0 errors with no new warnings, 478 unit tests passing across 33 suites, frontend production build green. Not verified: the migration has not been run against a database, and integration and e2e need a Node this machine does not have active. None of this is legal advice and the wording is worth a lawyer's eye before it ships. Refs #56 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
synAdminandClaude Opus 5
|
ac3f6e91f5 |
feat(analytics): report consenting customers' activity to Brevo (#56)
Loads Brevo's web tracker for a signed-in customer who has consented, reports route changes as page views, and tracks the three events the issue asked for: added_to_cart, favorited, and checkout_completed. The four design questions were settled on the issue in August and this implements those answers. The consent gate is the part worth reading. The decision recorded on the issue was "gate it behind consent", but the sentence customers actually agreed to named only email: "I want to receive occasional emails about new one-of-a-kind items". Gating a tracker on `marketing_consent` while that was the stored wording would have treated "email me about new items" as authorisation to send someone's browsing to a third party, which it does not say — and this project stores the wording verbatim against each customer precisely so that a record says what the customer saw. So the sentence is widened here, and the tracker is gated on `analytics_consent`, a field the server computes by comparing the wording stored against a customer with the current constant. Changing the sentence therefore does not retroactively widen anybody's consent: everyone who agreed to the old text keeps their email consent and is not tracked until they re-consent through the account page. A boolean alone could not tell those two populations apart, which is the whole reason the text is stored per customer. `analyticsConsent` is exported and has its own unit test, because "agreeing to the old wording does not authorise tracking" is the rule that silently tracks people if it regresses — their flag really is true. QA stays out of the live Brevo account by construction rather than by remembering. The key is per-environment, the tracker never loads without one, and `docker-compose.qa.yml` sets an empty literal with no stack variable behind it, so nothing can inherit a value from the host or be pasted in from production's stack. Same reasoning as QA_DB_PASSWORD and the QA_SMTP_ names beside it. Events are reported from the API layer rather than the UI call sites, so no caller can add to the cart or favorite an item without it being counted, and each fires only after the response was accepted — a refused add is not reported as one. The two checkout completions each name their processor, because a demo purchase charges nothing and counting it as a sale would overstate revenue. The privacy policy gains an analytics section in this change rather than a follow-up, since the published policy previously described none of this and would otherwise have lagged the code. It is deliberate about the limits: withdrawing consent stops further reporting, but anything already sent stays with Brevo, and a script already injected cannot be un-injected — `stopBrevoTracking` stops calls, it does not unload sa.js. That is said in the code too, because "tracking stops" reads as a stronger promise than any web tracker can make. Verified: backend tsc clean, both lint suites 0 errors with no new warnings, 474 unit tests passing across 33 suites, and the frontend production build green including the compose-environment guard. Not verified: integration and e2e, which need a database and a Node this machine does not have active, and no real Brevo key was exercised — the tracker has never been observed reporting to an actual account. Closes #56 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
 bermudalambandClaude Opus 5
|
39c82ff3a4 |
fix(orders): mark a demo order in the history rather than leaving it to read as real (#205)
#195 and #203 made the cart say a demo order is a demo order. That message is an antd toast lasting about three seconds, after which the cart empties and the card unmounts. Order history is what the customer comes back to when they wonder where their item is, and it said nothing. A demo row was a real row: item name, `$80.00`, status `completed` rendered as a neutral tag because `STATUS_COLORS` has no `completed` key, and `demo` printed raw under a heading reading "Processor". That is not an explanation — a customer has no reason to read `demo` as "this did not happen", and "processor" is not a word they have any reason to know. Two things now say it, for the same reason the cart needed two. The `demo` cell renders as a tag reading "Demo (not charged)", which marks *which* order. A notice above the table, shown only when there is one, says what that means — a tag reading "Demo" still assumes the reader knows what a demo order is, and what they actually want to know is whether to expect a parcel. Nothing changes on the backend: `orders.processor = 'demo'` was already written at checkout and already selected for this page. The row is a real row in a real table and stays visible, because hiding it would be its own kind of lie — the customer did do something, and it did have an effect on the catalogue. Written test-first against a browser: the new case drives a real demo purchase through the cart, opens `/orders`, and failed on both assertions before the change. Verified: 4 end-to-end tests in this spec pass, the 5 existing order-history tests still pass, frontend build clean, lint 0 errors (2 pre-existing warnings in `src/filters.ts`). Closes #205 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bermudalamb
|
45f1c77160 |
refactor(frontend): triage the setState-in-effect sites (#99)
Eleven warnings that looked alike and were not. This is a decision per site rather than eleven fixes, which is what the issue asked for — some of these would be made worse by "fixing" them. One was a real defect. VerifyEmail routed a fact through an effect that was already knowable during render: whether the link carries a token comes from the URL. The component therefore rendered once as a spinner in a state that was never true — a link with no token was never "verifying". Both pieces of state now derive their initial value from the token, and the effect's missing-token branch becomes an early return, so the failure is what the first render shows. Two are a defensible reset. CartProvider and FavoritesProvider clear their collection when the customer becomes null, which is synchronisation with the session rather than derived state. Deriving instead would push "signed out" onto every consumer of those contexts, and remounting on a `key` is more indirection than the problem deserves. Decided and written down rather than left for the next reader to re-investigate. Eight are legitimate and flagged conservatively. Six are a pending flag before a fetch — the rule cannot tell a spinner from a value that was already known. Cart's lapsed-item refetch is synchronisation with a server-side release the client cannot observe. useNow subscribes to the clock, which is the case the rule's own documentation names as correct. Each of the ten that stay carries the reason and a targeted disable, so lint drops from thirteen warnings to two — and the two left are the unrelated no-alphabetical-sort pair. Suppressing per site rather than switching the rule off keeps it live for new code, which is where the next VerifyEmail would be caught. The trap #60 recorded caught this, in a variant it does not describe. Placing the disable above `useEffect(` works only for a single-line effect: where the effect spans several lines the flagged line is the setState inside the body, so the directive covered nothing and produced both an unused-disable warning and the original one. Three sites were wrong that way on the first attempt. Confirmed fixed by the absence of "Unused eslint-disable directive" from the output — a disable that covers nothing reports itself, which is what makes this checkable rather than assumed. Verified: tsc clean over src and tests, and the specs covering the changed behaviour pass — verify-email, auth, favorites, orders, cart-countdown, resend-verification, 32 of 33 with the one failure passing 10/10 in a serial re-run. Closes #99 |
||
|
bermudalamb
|
8de261538b |
refactor(frontend): declare props read-only, and drop the deprecated antd prop (#100)
Seventeen components declared props the compiler was free to assume were mutable, and one antd prop had gone stale. Both mechanical, neither with any behaviour attached.
React never writes to props, and `Readonly<>` says so to the compiler rather than only to the reader. This finishes a pattern the codebase had already chosen rather than introducing one: AccountDetails and EmailTemplateEditor were already written as `type Props = Readonly<{…}>`, so the thirteen named prop interfaces are converted to that same shape and the four context providers, which annotate `{ children }` inline, get `Readonly<{ children: React.ReactNode }>`.
Cart.tsx was the last place passing `destroyOnClose`, deprecated in antd 5.20. Twelve other call sites across the admin screens, the filter drawer and four customer modals already use `destroyOnHidden`, so this one was simply stale. Deprecated props keep working until they do not, and the failure then arrives as an antd upgrade breaking something unrelated to the change being made.
Counted rather than assumed, which the issue specifically asks for, because a `Readonly<>` in the wrong position type-checks and fixes nothing: lint goes from 31 warnings to 13, a drop of exactly eighteen, and both rules disappear from the breakdown entirely rather than merely thinning out.
What that leaves is the point of doing it. The remaining thirteen are eleven `set-state-in-effect` and two `no-alphabetical-sort` — so the frontend's warnings are now only the ones that need a decision, which is what makes #99 tractable. It had grown from the eight in that issue's title to eleven, two of them added by #97's clock tick and lapsed-cart refetch.
No behaviour change intended, so the bar was the end-to-end suite. Full run: 121 passed, 8 failed; all eight pass in a 45/45 serial re-run, which is the shared-database and event-loop flakiness this suite has had throughout.
Closes #100
|
||
|
bermudalambandClaude Opus 5
|
b8549e9c72 |
feat: let a customer resend their own verification email (#110)
A verification email was sent once, at registration. If it was lost, filtered, or arrived after the 24-hour token had expired, the customer had no route back except registering again with a different address. POST /api/customers/resend-verification mints a fresh token and sends the mail, and the account page grows a "Send it again" button beside the warning that was already there. The button only exists while the address is unverified: on a verified account it would be a control whose only outcome is a refusal, and the endpoint refuses that case rather than sending a pointless email. The mint-token-and-send sequence now lives in one place. Registration and the email change already did the same three steps - supersede any outstanding token, mint a new one, send it - and this would have been a third copy. The step most likely to be dropped in a copy is the supersede, and it is the one that matters: without it an older message still sitting in the inbox goes on verifying. Anything that makes the server send mail on request is an abuse vector, so this is rate limited to three an hour, keyed on the customer id. That is tighter than either existing limiter and sidesteps #84's IPv6 problem entirely, since a signed-in caller has an identity better than an address to count against and cannot escape the bucket by moving within a delegated prefix. The refusal says the mail probably did send and to check the spam folder, which is both more useful and more honest than a bare 429. The claim that keying on customer id also solved test isolation was wrong, and the tests caught it. resetDb truncates with RESTART IDENTITY, so every integration test's first customer is id 1: three tests that each sent once left the fourth starting at its limit, and two tests failed on a 429 they never asked for. A "fresh customer per test" is a fresh row with a recycled identity. The limiter now has an explicit exported store the suite clears between tests, and the comment that claimed otherwise has been corrected rather than left to mislead the next reader. Verification: seven integration tests covering the unauthenticated refusal, the send, the new token invalidating the old one - asserted through the endpoint that would honour the stale link rather than by counting rows - the new link actually verifying, the already-verified refusal, the allowance stopping the fourth send rather than merely reporting it, and one customer's exhausted allowance leaving another's intact. Three end-to-end tests for the button, its confirmation and the message on the fourth click. The 33 integration tests across the three suites this touched all pass, as do the 199 backend unit tests. tsc clean on both sides, ESLint no errors. Closes #110 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
bermudalamb
|
9bb3cc86b6 |
feat(frontend): give order history a page of its own (#121)
The account modal had accumulated: a profile line, a name form, two collapsed panels for changing email and password, two consent switches, an order table and four controls. The table was the piece that fitted worst, being the only tabular data in a 700px dialog whose body is capped at 70vh. The scroll={{ x: 'max-content' }} already on it was a workaround for being in the wrong container rather than a layout choice.
It moves to /orders, an ordinary page in the same Routes block as /cart and /privacy, rather than another entry in MODAL_ROUTES. Order history is a list you read, like the cart, not a dialog you dismiss. A modal at /account/orders would have been the smaller change and was rejected: it inherits the same width and the same scroll cap, so it moves the table without giving it anything.
The page shell follows Cart.tsx, which is the established shape here: a Layout with a Header carrying Back to Shop and the title, and the same guard sending a signed-out visitor to /login. The account modal keeps a View order history button where the table used to be, because that is where a customer looks for it.
One thing changes rather than moves. The old effect caught a failed load with a toast and left orders as an empty array. The toast faded and the empty table did not, so from then on a customer whose request failed saw exactly what a customer with no orders saw, and the page asserted something false. Loading, failed and empty are now three distinct states, and the failed one carries a Retry: a transient failure would otherwise strand someone on a page that needs a full reload to recover.
OrdersBody sits at module level rather than nested inside Orders(). A function declared inside a component counts toward that component's cognitive complexity, which is what made Customers() hard to bring back under the threshold in #81.
The two assertions in account-modal.spec.ts that looked for the text "Order History" inside the modal are updated to look for the link, not deleted. They were the only coverage that the account view still offers any route to the orders, which is exactly what this change could have silently broken.
Verification, against a real backend and database: five new tests covering the signed-out redirect, the empty state, Back to Shop, the link from My Account, and that the page renders as a page rather than a modal over the storefront - that last one is what would catch /orders being added to MODAL_ROUTES and quietly undoing the change. The full suite goes from 100 to 105 passing with no new failures; the three that fail did so before this branch and fail identically on main. tsc and the production build are clean, ESLint reports no errors.
Closes #121
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
bermudalamb
|
84db0e7ca2 |
feat(frontend): let a customer change their own name, password and email (#111)
The three endpoints have been on main since PR #113 with nothing calling them. This adds the UI, which is what the issue is actually about: its title is that PUT /api/customers/me has no caller. The name form sits open on the account view. Changing an email address or a password does not, because both are rare and deliberate, and leaving them expanded would push order history and the account controls below the fold for everyone who never uses them. They go in a collapse instead. Both of those carry a consequence the form cannot show. A new address has to be verified before it can be used to sign in or reset a password, and the old address is told that the change happened. A password change ends every other session. Each is stated above its fields rather than reported afterwards, so the surprise arrives while there is still a chance to back out. The email form asks for the current password. A live session is not enough to move the address a password reset would be sent to, which is the whole reason the server asks for it too. Server refusals are shown as they arrive rather than replaced with something generic: the message names which of the two passwords was wrong, or which name was left blank, and that is the only useful thing to say. The forms live in their own component rather than in Account.tsx. Three forms inline would have roughly doubled that component, and nested JSX bodies count toward the parent's cognitive complexity - the same thing that made Customers() hard to bring back under the threshold in #81. Verification, all against a real backend and database rather than mocks: six new end-to-end tests covering the name surviving a reload, a blank name being refused, the old password ceasing to work while the new one starts working, a wrong current password being refused for both the password and the email change, and an email change marking the account unverified again. The password test asserts the old credential no longer opens the account rather than that the form said something reassuring. The 21 existing account and auth tests still pass, and tsc and ESLint are clean. Closes #111 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
b287c07747 |
feat: capture first and last name so emails can greet informally (#106)
Registration collected one optional Name, so every greeting had only a whole name to use: "Hi Thom Lamb," rather than "Hi Thom,". Both parts are now captured, and the cart reminder greets by first name. Both are required of anyone new, refused individually rather than as a single "name is required" so a form that filled one and not the other is told which. The columns are nullable even so, and that is deliberate. Marking them NOT NULL would mean backfilling legacy rows with empty strings, which asserts that every customer has a name — untrue of anyone who registered while the field was optional. The table records what is actually the case; the rule that new registrations must supply both lives in the route, where a missing field can produce a message naming it. The backfill splits on the first space, and it is lossy in a way no version of this avoids. "Thom Lamb" becomes Thom and Lamb; "Mary Jane Smith" gets a last name of "Jane Smith"; names that are not two parts fare worse. It was chosen over leaving the columns empty because nothing currently lets a customer correct their own name — PUT /api/customers/me exists but no frontend calls it — so empty would have meant permanently unpersonalised for every existing customer. The migration says so, so nobody later reads backfilled values as data the customer supplied in that shape. Verified against a seeded database rather than reasoned about, because this is the part that cannot be covered by the suite: migrations run in globalSetup before any test, and the old column is gone afterwards. Six representative rows through the real migration gave Thom/Lamb, Mary/"Jane Smith", Cher/null, " Padded Name " trimmed to Padded/Name, and null and whitespace-only names left as null on both. The down migration rejoins the parts and returns all six to their original strings. The old column is dropped rather than kept alongside, so there is one source of truth instead of two that drift. The admin keeps receiving a single composed display name. It only ever shows one — the list cell and the drawer title — and never edits one, so giving it both parts plus the joining logic would be work for no reader. Churn was the bulk of this: 14 backend registrations and 10 end-to-end registration forms. A first attempt at the backend fixtures also added names to login and password-reset payloads, which would still have passed since the server ignores unknown fields, but a login test implying login takes a name is a small lie; that was reverted and redone against register calls only. Verified: 172 unit, 183 integration and 95 end-to-end passing, lint unchanged at 4 backend and 27 frontend warnings. Not covered: the cart reminder itself, which runs from a cron and had no test before this either. The greeting change is a one-line substitution in that query. Refs #106 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
a700597440 |
refactor: standardise antd imports and remove the avoidable any (#65)
Stage 1 of #65: this issue's original two lists. The type-checked gate it also owns follows in later stages. Nine files imported antd from the barrel while the rest of the codebase used deep imports from antd/es. Both resolve to the same modules under antd v5 and Vite, so this is not the tree-shaking problem it would have been under v4 — the cost was that a documented convention had two spellings, and nobody reading a file could tell whether its style was deliberate or just old. Eighty-two imports converted, and every antd/es path was checked to exist before generating any of them rather than trusting a name-mangling rule. The three `client: any` parameters in cartCheckout are now PoolClient. These functions run inside a transaction, and `any` removed exactly the check that would catch a pool-versus-client mix-up — which in this codebase means a query silently running outside the transaction it was meant to be part of, on the path that takes money. publicCustomer took `any` and now takes a CustomerRow describing what it actually reads. Typed as its own shape rather than the whole table so that adding a column later — a password hash, a token, an internal note — cannot quietly start being echoed back to a customer. The three `(window as any).paypal` casts are replaced by a declared interface for the injected SDK. It is deliberately narrow: it describes the three things this app calls, not the whole SDK, because a wider guess would be fiction and a wrong shape typed confidently is worse than an honest cast. The property is optional, since the SDK is absent until its script has loaded — which is the check both call sites already make. Verified: 141 unit, 169 integration and 94 end-to-end passing. The end-to-end run is the one that matters here — an antd import migration can build cleanly and still break at runtime through styles or context, so a green tsc proves less than it appears to. Lint drops from 8 warnings to 4 in the backend and 30 to 27 in the frontend, all of them the no-explicit-any this change removed. As a side effect the no-unsafe count that later stages exist to clear falls from 259 to 216 in the backend and 73 to 67 in the frontend, measured rather than estimated. Refs #65 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
5f9172512c |
refactor: clear the 85 minutes of technical debt (#81)
Thirteen of SonarQube's fourteen code smells, with the fourteenth argued as a false positive rather than coded around. Four of these were not stylistic. The Remove button in Cart's List.Item actions array carried no key, so React could not match it across renders and rebuilt it on every cart render. The cart, customer-auth and favorites providers each passed a freshly allocated object as their context value, which re-renders every consumer whenever the provider renders, cart or session unchanged — and these three wrap the entire storefront, so the fan-out was the whole tree. Two of them also rebuilt a Set on every render for good measure. All four now memoized on the values they actually derive from. The four cognitive-complexity findings wanted real restructuring rather than suppression. parseItemFilters splits into one helper per field, leaving the function with the order those helpers run in and the single rule that spans two fields; the order is preserved exactly, because a query wrong in two ways reports the first field and rearranging the calls would change which error a caller sees. adminCategories' PUT extracts the parent resolution — the existence check and the cycle check — into a resolver that returns the refusal rather than sending it. App's four-way render chain becomes a Catalogue component, which also removes two of the nested ternaries. Customers' confirm dialog moves to module level, since a function's cognitive complexity counts everything declared inside it and that dialog branches on `disabling` five times. The rest were mechanical: two more nested ternaries — a status-colour lookup and a pluralisation helper — and one type assertion that asserted the type the expression already had. Left alone: S6478 on CategoryTreeSelect's popupRender. That is antd's render prop, called as a function and spliced in, never mounted as a component type, so the destroy-the-subtree failure the rule describes cannot happen. Marked false positive in SonarQube with that reasoning rather than contorting the component around a rule that misread it. Verified rather than assumed. Backend unit 78 pass, integration 134 pass, end-to-end 83 pass, both workspaces build clean. ESLint warnings drop from 35 to 31 with no new file warning — the same React and SonarJS rules #60 turned on are what surfaced this backlog in the first place. Refs #81 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
bermudalamb
|
c058b3ed2e |
feat(ci): add ESLint to both workspaces with a defect-only CI gate (#60)
TypeScript's strict mode checks types and nothing else, so nothing enforced the React hook rules, the SonarJS rules, or unhandled-promise detection. Adds a flat config per workspace, a lint script in each, and a lint job in tests.yml. The rule selection is the substance of this change and is measured rather than guessed. A full-strength config reports 435 violations across 50 files, but 325 of those are the no-unsafe-* family from recommendedTypeChecked, every one downstream of pool.query() returning any rows and untyped fetch responses. Typing those boundaries is the whole of #65, so enabling the rules here would ship a linter whose output is three-quarters another issue's backlog — the reliable way to teach everyone to ignore lint output. This enables recommended plus the two type-aware rules that catch defects rather than describe type debt, which leaves 110 findings. Both configs downgrade every preset to a warning and then list the error rules explicitly at the bottom, so the CI gate is readable in one place instead of inferred from four presets' defaults. Errors are no-floating-promises, no-misused-promises, rules-of-hooks, exhaustive-deps and jsx-a11y/alt-text; everything else warns. No --max-warnings flag is needed because ESLint already exits non-zero on errors and zero on warnings. no-misused-promises runs with checksVoidReturn.attributes false, since onClick={async () => ...} is idiomatic React and safe when the handler catches its own errors — at the default it flags every antd button in the admin screens, 25 of its 28 hits, and a rule that is 89% noise gets switched off within a week. The 37 errors this surfaced were mostly not the mechanical fix they looked like. The plan assumed the 30 floating promises were fire-and-forget loaders that already handled their own failures, which was true of the one sampled when writing the design and false for most of the rest: Admin, Categories, Customers, Tags, Settings, Account and CustomerAuthContext all had no rejection handling at all, so `void` on them would have hidden real failures rather than annotated deliberate ones. Each of those loaders now catches and surfaces the failure before the call site voids it. The CustomerAuthContext one was a live bug — a rejected fetchMe left loading true forever, rendering as a permanent spinner instead of a signed-out page. Admin's load became a useCallback so its effect can name it honestly rather than suppress the dependency, Categories' drop handler was split so the function antd receives returns void as its type says, and Cart's effect now names refreshCartContext, which is a useCallback with an empty dependency list and so cannot re-run it. The only disable added is in asyncRoute, where returning a promise where Express expects void is the entire point of the wrapper and the promise cannot reject. Two of the issue's premises did not survive measurement, both recorded in the spec: exhaustive-deps flags 2 cases rather than the 10 inferred from empty dependency arrays, and the backend was already clean on the defect rules because #59 wrapped every async route. Verified: lint, build, 78 unit, 134 integration and 83 e2e all pass in both workspaces, and the CI gate was confirmed to fail by introducing a deliberate violation rather than by assuming the job is wired correctly. Closes #60 |
||
|
bermudalamb
|
5ebb366074 |
feat(auth): open sign-in and registration as modals over the page behind (#50)
/login, /register and /forgot-password rendered bare cards with no site header. They linked to each other and nowhere else, so a customer who clicked Log in from the storefront and changed their mind had no way back except the browser's back button. All four auth routes are now modals over the page the customer was already on, reusing the backdrop-location arrangement from #51: a direct visit or a link from an email opens over the storefront, so closing always lands somewhere real. They remain real routes, because /reset-password links to /login and customers may have bookmarks. Signing in or registering now returns the customer to the page behind, signed in, rather than moving them to /account. Someone who signs in while browsing wants to carry on browsing, and this is already how the cart and favorites prompts behave when they resume an interrupted action. The larger half of this is removing the duplication. Signing in existed twice — as these routes and again inside the prompt shown when a signed-out visitor adds to the cart or favorites something — and the two had already drifted. There were three different wordings of the marketing consent in circulation: the register page's, a shorter one in the prompt, and the string the server actually stores. The server keeps that text verbatim so the consent record says what the customer saw, which none of the three did. Both callers now render one shared AuthForm whose checkbox is the exact string the server records, and a test asserts that wording so it cannot drift again silently. Steps within the auth flow replace rather than push, so switching between tabs or stepping to password recovery leaves the whole detour as a single history entry and closing returns to where it started instead of walking back through every tab that was looked at. The privacy policy link opens in a new tab: following it in place would discard a part-filled signup form, and /privacy still has no way back of its own until #52. Test changes follow from the destination change rather than being incidental. Nineteen assertions across seven specs waited for /account after signing in; they now assert the header shows a signed-in customer, which is the condition actually being waited for. Modal submits are scoped to their dialog, because the storefront behind now offers a Log in button of its own and an unscoped locator matched both. Assertions that follow a server round-trip were given a realistic timeout — the 5s default is too tight for a bcrypt hash plus re-rendering the storefront behind the modal. Verified with 83 end-to-end tests, all passing, and type checking clean. No backend changes. Closes #50 |
||
|
bermudalamb
|
91485b6ac1 |
feat(account): open My Account as a modal over the page behind it (#51)
/account had no header and no links of any kind, so once a customer opened it the only way back to the storefront was the browser's back button or editing the URL. It is now a modal rendered over whatever the customer was looking at, while staying a real route. Opening it from the header pushes /account and names the current page as the backdrop, so closing returns there with filters intact, and the browser's Back button does the same thing as the close control. Entering /account directly — a bookmark, the link in a verification email, or the redirect after registering — has no page behind it and falls back to the storefront, so closing always lands somewhere real. Keeping it a route means the URL still works: bookmarkable, shareable, and refreshable with the view still open, which the header link and the four post-authentication redirects already depend on. Deleting an account now clears the session as well. Previously it removed the account server-side and navigated home without touching the auth context, so the header went on offering "My Account" for an account that no longer existed until the next reload. That was always wrong, but the modal makes it visible rather than merely stale, because the storefront is rendered behind and the wrong header is on screen throughout. The modal body is capped and scrolls, and the order history table scrolls within itself, so the view survives a phone without pushing its own title and close control off-screen. Also scopes the account switch locator in the favorites spec to the modal, since the storefront now renders behind the account view and has a theme switch of its own, and gives the post-registration wait a realistic timeout — it waits on a bcrypt round-trip rather than a render, and the 5s default was surfacing as a flake on whichever test lost the race under parallel load. Closes #51 |
||
|
bermudalambandClaude Opus 5
|
f626f27e75 |
feat: favorite items and notify when a favorite is sold (#34)
Customers can favorite and unfavorite items from the storefront, opt in to being told when a favorite is sold to someone else, and manage that preference from their account page. The opt-in is a consent of its own rather than the existing marketing flag. Being told that a specific item you asked about has gone is a narrower thing than agreeing to marketing, and folding one into the other would leave marketing_consent_text no longer describing what was actually agreed to. It is recorded the same way as the marketing consent — flag, timestamp, and the exact wording shown — and accepting it does not set marketing_consent. The prompt appears only after a customer has actually favorited something, so the reason for asking is concrete rather than an abstract marketing ask, and it says plainly that it is separate from marketing email. Declining keeps the favorite. Notifications fire when an item reaches sold, either through checkout or an admin marking it sold, and never to the buyer — telling someone the item they just bought is unavailable reads as a bug. Reserved is deliberately not a trigger: reservations expire and get released, so a "gone" email would often be about an item still for sale. Disabled accounts are excluded, per #33. completeCheckout now returns the sold item ids and the buyer so its three call sites can notify after COMMIT. Sending inside the transaction would email people about a sale that then rolled back, and would hold the transaction open for SMTP. Each message is sent independently so one bad address cannot stop the rest, and the sale has already succeeded regardless. Favoriting while signed out opens the existing inline register/login modal, exactly as Add to Cart does, and completes the favorite on success. Also fixes a latent bug in the same component: while the session was still resolving, `customer` is null for a signed-in visitor too, so clicking Add to Cart or the new heart in that window prompted them to sign in again. Both now ignore clicks until the session has resolved, and the control shows as loading meanwhile. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
db7c61c89d |
feat: customer password reset via email round-trip (#32)
Adds "Forgot password?" to the login page, a request page, and a reset page reached by a one-hour, single-use token delivered by email. Reuses customer_tokens with a new password_reset kind alongside verify_email. The request endpoint always answers 200, whether or not the address has an account, so it cannot be used to test addresses for membership. Note /register still reveals existence through its 409 on a duplicate, so this protection is currently partial; closing that is its own change. Completing a reset deletes every session for that customer. A reset prompted by a compromise has to evict the intruder, and leaving a 30-day cookie alive would defeat the point. It also marks the address verified, since receiving the mail is exactly what verification proves, and supersedes any outstanding token so an older link in the inbox cannot be resurrected. Introduces the first rate limiting in the codebase, on the request endpoint only. The limiter is keyed on caller *and* submitted address: keying on IP alone would let one person lock out everyone behind the same proxy, and everything arrives via Nginx Proxy Manager. Applying that same limiter to the reset endpoint, which carries no address, collapsed every caller into one shared bucket -- so that endpoint is deliberately unlimited instead, protected by a 32-byte single-use token whose bcrypt work only runs after the token matches. The e2e tests read the issued token directly from Postgres rather than through a test-support endpoint. An endpoint returning a reset token for an arbitrary address is account takeover for every customer if it is ever reachable, and an environment gate is thin protection against that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
f537314259 |
fix(admin): theme, American English, and inventory/reservation tooling (#27)
Seven reported items, of which the first four had two root causes. The active tab was invisible in dark mode because colorPrimary was hardcoded to #1a1a1a in both themes. The accent now inverts with the theme, and colorTextLightSolid inverts with it, or a near-white accent would get antd's default white label and disappear. The Category tab, Tag tab, and item-form category selector ignored the theme entirely. antd declares main: lib/index.js and module: es/index.js, so importing from 'antd' resolves to the ES build while 'antd/lib/...' loads the CommonJS one — two copies, two React contexts, and no ConfigProvider for anything deep-imported. Switching those files to antd/es/* keeps the deep-import convention and shares the instance. This was introduced by my own use of the lib path; es is correct under Vite. Two storefront components had the same latent bug. "Colour" is now "Color". The Customers tab shows how many items each customer is holding, as a link opening the item list with a Release button. Release mirrors the customer's own cart removal — drop the cart row, return the item to available, guarded on 'reserved' so it can never resurrect a sold item — and deliberately sends no email about an action the customer did not take. The count is a subquery rather than another join, which would have multiplied rows and inflated order_count and total_spent_cents. The Inventory tab filters by category, tags, price, and status, reusing the storefront's parser and query builder so the two cannot drift. Reserved is one option in a Status filter rather than a standalone toggle. Also fixes two defects the screenshots exposed: the reserved-count link bubbled to the row handler and opened the customer drawer behind the dialog, and .admin-category-node had no CSS at all, so the tree node name, item count, and actions ran together as one string. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
dd51621956 |
fix: reset the header and return home when a customer logs out (#21)
Logging out destroyed the server session and navigated home, but never told CustomerAuthContext, so `customer` stayed in React state and the header kept offering "My Account" instead of "Log in" and "Sign up". A reload appeared to fix it, because fetchMe then returned null, which is why the symptom looked intermittent. The cart badge had the same cause: CartContext only clears its items once `customer` goes null. Logout now lives on the auth context, which clears `customer` itself rather than triggering a refetch — a refetch would leave a window where the session is gone but the UI still shows the customer signed in. logoutCustomer also ignored res.ok. A failed logout leaves the session cookie valid, so reporting success signed the customer back in on their next reload. It now rejects, and the account page reports the failure and stays put instead of pretending. Logout is not routed through handle() because the endpoint answers 204 with no body. Navigation uses replace, so Back no longer returns to the account page, which would only bounce to /login now the session is gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bermudalambandClaude Opus 5
|
b7c8c37447 |
fix: add the missing /verify-email page
Verification emails linked to /verify-email?token=..., but no such route existed in main.tsx and nothing in the frontend ever called POST /api/customers/verify-email. The SPA catch-all served index.html, no route matched, and the page rendered blank -- so the token was never redeemed and accounts stayed unverified forever. The gap was invisible until SMTP was configured, because no verification email had ever actually been delivered. Adds VerifyEmail.tsx (verifying / verified / failed states), a verifyEmail call in customerApi, and the route. The request is pinned to a single firing via a ref: the endpoint deletes the token on success, so StrictMode's double effect invocation in dev would otherwise overwrite the success state with "invalid or expired token". Verified end to end against a local stack: a real token flips customers.email_verified to true and is consumed from customer_tokens. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
bermudalamb
|
9ab689e624 | feat: add customer cart with expiry, shipping addresses with USPS validation, and multi-item PayPal checkout | ||
|
bermudalamb
|
9be4986dd3 | Initial commit: redefined-designs storefront |