Remove the four duplicated blocks, one of which #139 introduced #182

New Issue

2026-08-25T11:34:48-05:00

bermudalamb commented

2026-08-25 11:34:48 -05:00

SonarQube reports 4 duplicated blocks across 8,200 lines. Searching for them turns up four candidates, and the first is the clearest because it was created three days ago.

1. The category tree adapter, duplicated by #139

frontend/src/admin/CategoryTreeSelect.tsx and frontend/src/components/FilterDrawer.tsx now hold the same interface and the same function, verbatim:

interface CategoryTreeOption {
  value: number;
  title: string;
  children?: CategoryTreeOption[];
}

function toTreeData(nodes: CategoryNode[]): CategoryTreeOption[] { ... }

This is mine. #139 moved the storefront's category filter from an antd Tree to a TreeSelect, and rather than reusing the admin's adapter I copied it — the commit even says the shape "matches the admin's CategoryTreeSelect so the two stay comparable", which is an argument for sharing one and instead produced two.

frontend/src/admin/Categories.tsx has a third toTreeData returning antd's DataNode[]. That one is genuinely a different shape for a different control, so it is not the same duplication, but it is worth deciding whether all three want one adapter with two output shapes or whether the third stays separate.

buildCategoryTree in filters.ts is already shared by all of them, and is the right precedent: the nesting logic lives in one place because it has one meaning. The adapter has one meaning too.

2. `TAG_COLORS`

Present in four files across both workspaces:

frontend/src/admin/Admin.tsx
frontend/src/admin/Tags.tsx
backend/src/routes/adminTags.ts
backend/src/utils.ts

This was noted as worth filing earlier and never was. The cross-workspace half cannot be deduplicated by an import — there is no shared package, and creating one for a colour list would cost more than it saves — so the honest fix is one definition per workspace with a comment on each pointing at the other, in the same spirit as ALLOWED_IMAGE_TYPES, which already says "the list unavoidably exists in two runtimes; if it changes here, change it there". Within the frontend, the two copies should become one import.

3. The `item_images` insert loop in `routes/admin.ts`

Lines around 274 and 330. Create and update both do:

await client.query(
  `INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, $2, $3)`,
  [/* item id */, `/uploads/${file.filename}`, /* sort */]
);

They differ only in where the id comes from and how the sort order starts — zero for a new item, MAX(sort_order) + 1 for an existing one. One helper taking the client, the item id, the files and a starting index covers both, and it also means the /uploads/ prefix is written once. That prefix matters more than it looks: #103 made it the value uploadUrl joins an origin onto, so it is now a contract rather than a string.

4. Duplicated guard in the same file

POST /items and PUT /items/:id open with the same two lines:

const categoryId = readCategoryId(req.body.category_id);
if (categoryId === undefined && req.body.category_id !== undefined) { ... }

Small, but it is the same decision expressed twice, and the second copy is where a future change gets forgotten.

Worth saying about the count

Four found by searching is not proof they are SonarQube's four — its detector works on token sequences above a length threshold, so it may be pointing at different blocks, and #2 spans workspaces which may or may not be within the analysis scope. The dashboard has the authoritative list. These four are worth removing on their own merits regardless of whether they are the ones being counted.

The first is the one to do first, since it is three days old and the copy has not yet had time to drift from the original.

SonarQube reports 4 duplicated blocks across 8,200 lines. Searching for them turns up four candidates, and the first is the clearest because it was created three days ago. ## 1. The category tree adapter, duplicated by #139 `frontend/src/admin/CategoryTreeSelect.tsx` and `frontend/src/components/FilterDrawer.tsx` now hold the same interface and the same function, verbatim: ```ts interface CategoryTreeOption { value: number; title: string; children?: CategoryTreeOption[]; } function toTreeData(nodes: CategoryNode[]): CategoryTreeOption[] { ... } ``` This is mine. #139 moved the storefront's category filter from an antd `Tree` to a `TreeSelect`, and rather than reusing the admin's adapter I copied it — the commit even says the shape "matches the admin's CategoryTreeSelect so the two stay comparable", which is an argument for sharing one and instead produced two. `frontend/src/admin/Categories.tsx` has a third `toTreeData` returning antd's `DataNode[]`. That one is genuinely a different shape for a different control, so it is not the same duplication, but it is worth deciding whether all three want one adapter with two output shapes or whether the third stays separate. `buildCategoryTree` in `filters.ts` is already shared by all of them, and is the right precedent: the nesting logic lives in one place because it has one meaning. The adapter has one meaning too. ## 2. `TAG_COLORS` Present in four files across both workspaces: ``` frontend/src/admin/Admin.tsx frontend/src/admin/Tags.tsx backend/src/routes/adminTags.ts backend/src/utils.ts ``` This was noted as worth filing earlier and never was. The cross-workspace half cannot be deduplicated by an import — there is no shared package, and creating one for a colour list would cost more than it saves — so the honest fix is one definition per workspace with a comment on each pointing at the other, in the same spirit as `ALLOWED_IMAGE_TYPES`, which already says "the list unavoidably exists in two runtimes; if it changes here, change it there". Within the frontend, the two copies should become one import. ## 3. The `item_images` insert loop in `routes/admin.ts` Lines around 274 and 330. Create and update both do: ```ts await client.query( `INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, $2, $3)`, [/* item id */, `/uploads/${file.filename}`, /* sort */] ); ``` They differ only in where the id comes from and how the sort order starts — zero for a new item, `MAX(sort_order) + 1` for an existing one. One helper taking the client, the item id, the files and a starting index covers both, and it also means the `/uploads/` prefix is written once. That prefix matters more than it looks: #103 made it the value `uploadUrl` joins an origin onto, so it is now a contract rather than a string. ## 4. Duplicated guard in the same file `POST /items` and `PUT /items/:id` open with the same two lines: ```ts const categoryId = readCategoryId(req.body.category_id); if (categoryId === undefined && req.body.category_id !== undefined) { ... } ``` Small, but it is the same decision expressed twice, and the second copy is where a future change gets forgotten. ## Worth saying about the count Four found by searching is not proof they are SonarQube's four — its detector works on token sequences above a length threshold, so it may be pointing at different blocks, and #2 spans workspaces which may or may not be within the analysis scope. The dashboard has the authoritative list. These four are worth removing on their own merits regardless of whether they are the ones being counted. The first is the one to do first, since it is three days old and the copy has not yet had time to drift from the original.

bermudalamb referenced this issue from a commit

2026-08-25 13:09:19 -05:00

refactor: remove the duplicated blocks SonarQube found (#182)

bermudalamb referenced a pull request that will close this issue

2026-08-25 13:12:20 -05:00

refactor: remove the duplicated blocks SonarQube found (#182) #184

bermudalamb added reference feature/182-remove-duplication

2026-08-25 13:12:38 -05:00

bermudalamb self-assigned this 2026-08-25 13:12:49 -05:00

bermudalamb added this to the Code Quality and Hardening 2 project 2026-08-25 13:13:03 -05:00

bermudalamb moved this to Review in Code Quality and Hardening 2 on 2026-08-25 13:13:08 -05:00

bermudalamb closed this issue