Feature/81 technical debt #82

2026-08-20T15:07:34-05:00

bermudalamb commented

2026-08-20 15:07:34 -05:00

No description provided.

bermudalamb added 2 commits 2026-08-20 15:07:35 -05:00

refactor: clear the 85 minutes of technical debt (#81 ) 5f9172512c

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>

refactor(frontend): actually reduce Customers' complexity rather than relocate it (#81 )

SonarQube Analysis / sonarqube (pull_request) Successful in 14m7s

Details

Tests / lint (pull_request) Successful in 1m52s

Details

Tests / backend-unit (pull_request) Successful in 39s

Details

Tests / frontend-e2e (pull_request) Failing after 8m52s

Details

4bad40868c

The previous commit claimed this one fixed. It was not: hoisting the confirm dialog to module level moved the reported line from 12 to 60, and I read that as the finding having moved to the hoisted function. It had not — line 60 was Customers() itself, still scoring exactly 16. The scan is what caught it, which is the argument for scanning rather than reasoning about what a rule will say.

Two further attempts also failed to move the number, and both are worth recording because they were wrong about what cognitive complexity counts. Collapsing five branches on `disabling` into one copy object fixed the hoisted function but left Customers() at 16. Extracting ten ternaries out of the table's cell renderers into module-level components left it at 16 as well — the ternaries inside a render callback were never the weight.

What actually carried the score was the drawer and the reserved-items dialog: two JSX bodies whose loading, empty and populated states are each a branch nested several levels inside the component. Extracting them as CustomerDetailPanel and ReservedItemsBody takes Customers() under the limit.

The cell-renderer extraction is kept even though it did not move the metric. NameAndEmail, BooleanTag, ReservedCell and ToggleDisabledButton read better than the inline callbacks did, and BooleanTag removes a repetition the Status column was open-coding differently from Verified and Subscribed.

Verified on a scan rather than by argument: technical debt 85 minutes to 5, code smells 14 to 1, and the one that remains is the S6478 false positive. ESLint holds at 31 warnings against a baseline of 35. End-to-end 83 pass.

One thing found on the way: the e2e suite is not idempotent against a persistent database. Two runs against a database that had already served three produced two different pairs of failures; recreating it produced a clean 83. CI is unaffected because its Postgres is fresh per run, but locally the suite needs a new database rather than a repeated one.

Refs #81
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

bermudalamb self-assigned this 2026-08-20 15:07:48 -05:00

bermudalamb added this to the Code Quality and Hardening project 2026-08-20 15:08:04 -05:00

bermudalamb moved this to In Progress in Code Quality and Hardening on 2026-08-20 15:08:08 -05:00

bermudalamb merged commit 8fef669358 into main

2026-08-20 15:08:20 -05:00

bermudalamb deleted branch feature/81-technical-debt

2026-08-20 15:08:23 -05:00

bermudalamb moved this to Ready for Release in Code Quality and Hardening on 2026-08-20 15:44:41 -05:00

bermudalamb moved this to Released in Code Quality and Hardening on 2026-08-21 12:12:03 -05:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: bermudalamb/redefined-designs#82