refactor(api): split catalog module into product, category, brand
Rearranges the Rust backend by domain: `modules/catalog` bundled four distinct concepts (Product, SKU, Category, Brand) behind one router and one 392-line service file, unlike every other module in the codebase which owns exactly one bounded aggregate. Splits into `modules/product` (Product/SKU, publish lifecycle, shop-scoped CRUD), `modules/category` (category tree, subtree query), and `modules/brand` (brand list, admin replace-all). `Category`/`Brand` move out of the shared `models.rs` into their owning modules; `Product`/`Sku` stay since `favorite`/`flash_sale`/`group_buying` reference them across modules. Purely internal restructuring — no route, schema, or behavior changes. Implements openspec change split-catalog-into-product-category-brand. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Chengdong Zhang
co-authored by
Claude Sonnet 5
Claude Sonnet 5
parent
0b594c0147
commit
8446650baf
@@ -0,0 +1,2 @@
|
||||
schema: spec-driven
|
||||
created: 2026-09-22
|
||||
@@ -0,0 +1,40 @@
|
||||
## Context
|
||||
|
||||
`apps/api/src/modules/catalog` (dto/handlers/repo/service, 693 lines) currently owns four concepts: `Product`, `Sku`, `Category`, `Brand`. Routes are already flat resource paths (`/api/products`, `/api/categories`, `/api/brands`, `/api/admin/brands`, `/api/shop/products*`) rather than nested under `/api/catalog/*`, so the module boundary is purely internal — no client or `packages/shared` type depends on the Rust module name. `Category` has no other module dependents (checked: `content`, `points`, `group_buying`, `flash_sale` don't reference it). `Brand` is referenced only from `catalog::handlers`/`service`. `Product`/`Sku` are referenced by `favorite`, `flash_sale`, `group_buying` via `crate::models`, so those two stay in the shared `models.rs`.
|
||||
|
||||
## Goals / Non-Goals
|
||||
|
||||
**Goals:**
|
||||
- Give Product/SKU, Category, and Brand each their own top-level module under `apps/api/src/modules/`, following the existing `dto.rs`/`handlers.rs`/`repo.rs`/`service.rs` shape used by every other module.
|
||||
- Zero behavior change: identical routes, request/response JSON, SQL, and error codes.
|
||||
- Keep `Product`/`Sku` in `crate::models` (cross-module dependents); move `Category`/`Brand` out of `models.rs` into their new modules since nothing outside references them.
|
||||
|
||||
**Non-Goals:**
|
||||
- No API versioning, route renaming, or DB schema/migration changes.
|
||||
- No behavior changes to sorting, filtering, or the subtree category-matching CTE.
|
||||
- Not folding Brand into Category — they're kept as separate modules per the chosen split (Option B from exploration), since they have independent admin-replace vs. tree-query shapes.
|
||||
|
||||
## Decisions
|
||||
|
||||
- **Three modules, not two.** Category and Brand are both small, platform-owned lookup tables with no lifecycle, but they don't share logic (recursive CTE vs. flat replace-all transaction) — merging them into one `taxonomy` module would just recreate a smaller version of the same problem. Alternative considered: `product` + `taxonomy` (category+brand together) — rejected per explicit user choice.
|
||||
- **`Product`/`Sku` stay in `models.rs`; `Category`/`Brand` move out.** Determined by actual cross-module usage (grepped `crate::models::{...}` across all modules): `Sku`/`Product` are imported by `flash_sale`, `group_buying`, `favorite`; `Category`/`Brand` are imported nowhere outside the old `catalog` module. This matches the existing pattern where module-local types (e.g. `Shop`, `Coupon`, `AddressBookEntry`) already live in `models.rs` alongside genuinely shared ones — moving `Category`/`Brand` doesn't fully break that existing pattern, but reduces unnecessary sharing surface for types nothing else touches.
|
||||
- **`repo.rs`'s `attach_skus` moves to `product/repo.rs` unchanged.** It only touches `Product`/`Sku`/order tables — no category/brand coupling.
|
||||
- **No spec deltas.** Per `openspec/specs/catalog/spec.md` and `openspec/specs/brand/spec.md`, this change alters no requirement text — those specs describe HTTP-level behavior which is untouched. `docs/tech-specs/rust-api.md` (if it names `modules/catalog`) gets a doc update as part of tasks, not a spec change.
|
||||
|
||||
## Risks / Trade-offs
|
||||
|
||||
- **Import churn** → every file that did `use crate::models::{Brand, Category, Product, ...}` needs updating; mitigated by compiler errors making every missed reference a hard build failure, not a silent bug.
|
||||
- **Merge/rebase conflicts** for anyone with in-flight branches touching `modules/catalog` → mitigated by doing the move as one atomic commit and merging promptly.
|
||||
- **sqlx query cache / compile-time verification** (if `sqlx::query!` macros or `.sqlx/` cache are used) → verified `catalog/service.rs` uses `sqlx::query_as::<_, T>(&format!(...))` (runtime-checked, not compile-time macros), so no `cargo sqlx prepare` step is needed.
|
||||
|
||||
## Migration Plan
|
||||
|
||||
1. Create `modules/product`, `modules/category`, `modules/brand` with the moved code (see tasks.md for the file-by-file split).
|
||||
2. Update `modules/mod.rs` router merge list and `models.rs`.
|
||||
3. `cargo build` and `cargo test` (integration tests hit HTTP routes, unaffected by internal module names) to confirm zero behavior drift.
|
||||
4. Delete `modules/catalog/`.
|
||||
5. Single commit/PR — no incremental rollout needed since this is a same-process, same-deploy internal refactor. Rollback is a plain `git revert`.
|
||||
|
||||
## Open Questions
|
||||
|
||||
- Should `apps/api/tests/catalog.rs` be renamed/split into `product.rs`/`category.rs`/`brand.rs` to mirror the new module boundaries, or left as-is since it already tests via HTTP and doesn't reference Rust module paths? (Default: leave as-is unless the user wants test-file parity too — captured as an optional task.)
|
||||
@@ -0,0 +1,29 @@
|
||||
## Why
|
||||
|
||||
`apps/api/src/modules/catalog` currently bundles four distinct concepts — Product, SKU, Category, and Brand — behind one module, one router, and one 392-line service file. Every other module in the codebase (`favorite`, `coupon`, `flash_sale`, `address`, ...) owns exactly one bounded aggregate. `catalog` is the outlier, and it mixes a shop-owned, stateful aggregate (Product/SKU with a publish lifecycle) with platform-owned, admin-curated lookup data (Category tree, Brand list) that has no lifecycle of its own. Splitting now, while the module is still small, avoids compounding the mismatch as more product-side logic (variants, attributes) gets added.
|
||||
|
||||
## What Changes
|
||||
|
||||
- Split `modules/catalog` into three top-level modules: `modules/product` (Product + Sku, publish lifecycle, shop-scoped CRUD), `modules/category` (Category tree, subtree query), and `modules/brand` (Brand list, admin replace-all).
|
||||
- Move `Category` and `Brand` structs out of the shared `models.rs` into their owning modules' `dto.rs`/models, following the pattern already used by `Shop`, `Coupon`, etc. staying in `models.rs` only where genuinely cross-module (Product/Sku stay in `models.rs` since `flash_sale`/`group_buying`/`favorite` reference them).
|
||||
- Register the three new modules in `modules/mod.rs`, removing `catalog` from the router merge list.
|
||||
- No route paths, request/response shapes, or database schema change — `/api/products`, `/api/categories`, `/api/brands`, `/api/admin/brands`, and `/api/shop/products*` are unaffected.
|
||||
- No SQL query text changes beyond moving them to new files.
|
||||
|
||||
## Capabilities
|
||||
|
||||
No system behavior changes. To keep spec capability boundaries aligned with the new Rust module boundaries, the existing `catalog` capability is re-filed (not behaviorally changed) into two capabilities along the same line the code splits on:
|
||||
|
||||
### New Capabilities
|
||||
- `product`: product/SKU content, publish lifecycle, shop isolation, SKU pricing, and public product browse/filter/sort — carried over verbatim from `catalog`.
|
||||
- `category`: the category tree and its use as a browse filter (including subtree matching) — carried over verbatim from `catalog`.
|
||||
|
||||
### Modified Capabilities
|
||||
- `catalog`: requirements removed (re-filed into `product` and `category` above with identical text/scenarios; no behavior change). `brand` is unaffected and stays a separate capability as it already was.
|
||||
|
||||
## Impact
|
||||
|
||||
- **Code**: `apps/api/src/modules/catalog/*` deleted; replaced by `apps/api/src/modules/product/*`, `apps/api/src/modules/category/*`, `apps/api/src/modules/brand/*`. `apps/api/src/modules/mod.rs` and `apps/api/src/models.rs` updated.
|
||||
- **Tests**: `apps/api/tests/catalog.rs` stays as-is (integration tests hit HTTP routes, which are unchanged) or is optionally renamed/split to mirror the new module boundaries.
|
||||
- **API/DB**: none — purely internal Rust restructuring.
|
||||
- **Frontends / `packages/shared`**: none — no contract change.
|
||||
@@ -0,0 +1,77 @@
|
||||
## REMOVED Requirements
|
||||
|
||||
### Requirement: Localized product content
|
||||
Product and category names/descriptions SHALL be stored as JSONB maps keyed by locale (`en`, `zh`). The API MUST return the full map; clients pick the display locale.
|
||||
|
||||
#### Scenario: bilingual round-trip
|
||||
- **WHEN** a shop owner creates a product with name `{"en": "Mug", "zh": "马克杯"}`
|
||||
- **THEN** both public detail and shop-admin GET return the identical map
|
||||
|
||||
**Reason**: The `catalog` capability is split so its requirements are filed under the new `product` and `category` capabilities, mirroring the Rust module split (`modules/product`, `modules/category`). No behavior changes.
|
||||
**Migration**: See `specs/product/spec.md` ("Localized product content") for the identical requirement and scenario.
|
||||
|
||||
### Requirement: Publish lifecycle
|
||||
Products SHALL have status `draft | published | unpublished`. Only `published` products appear in public list/detail.
|
||||
|
||||
#### Scenario: publish then unpublish
|
||||
- **WHEN** a product is published
|
||||
- **THEN** it appears in `GET /api/products`
|
||||
- **WHEN** it is unpublished
|
||||
- **THEN** public detail returns 404 and it disappears from listings
|
||||
|
||||
#### Scenario: publish requires sellable SKU
|
||||
- **WHEN** publishing a product with no active SKU having price > 0
|
||||
- **THEN** the API returns 400
|
||||
|
||||
**Reason**: Re-filed under `product` capability; no behavior change.
|
||||
**Migration**: See `specs/product/spec.md` ("Publish lifecycle").
|
||||
|
||||
### Requirement: Shop isolation
|
||||
Shop-role users SHALL only see and mutate their own shop's products under `/api/shop/products`.
|
||||
|
||||
#### Scenario: cross-shop access denied
|
||||
- **WHEN** shop owner A requests `/api/shop/products/{id}` of shop B
|
||||
- **THEN** the API returns 404
|
||||
|
||||
**Reason**: Re-filed under `product` capability; no behavior change.
|
||||
**Migration**: See `specs/product/spec.md` ("Shop isolation").
|
||||
|
||||
### Requirement: SKU pricing
|
||||
Each SKU SHALL carry `price_minor` (integer minor units) and an ISO `currency` code; stock is a non-negative integer.
|
||||
|
||||
#### Scenario: negative stock rejected
|
||||
- **WHEN** upserting a SKU with stock < 0
|
||||
- **THEN** the API returns 400
|
||||
|
||||
**Reason**: Re-filed under `product` capability; no behavior change.
|
||||
**Migration**: See `specs/product/spec.md` ("SKU pricing").
|
||||
|
||||
### Requirement: Public product browse
|
||||
Public `GET /api/products` SHALL return only `published` products whose shop is active, and SHALL remain readable without authentication. When `category_id` is supplied, the filter SHALL match that category **and every category beneath it**, so requesting a parent category returns products assigned to its child and grandchild categories. When `brand_id` is supplied the filter SHALL match that brand and compose with the other filters. The listing SHALL accept an optional `sort` of `price` or `sales`: `price` orders by each product's lowest active SKU price, and `sales` orders by units sold across orders that reached payment, which SHALL also be reported per product as `sold_count`. Any other `sort` value SHALL be rejected with a 400 `ApiError` rather than silently ignored. An unsorted listing SHALL order newest first. Paging SHALL keep returning `page` and `per_page` alongside the filtered `total`.
|
||||
|
||||
#### Scenario: parent category includes descendant products
|
||||
- **WHEN** a shopper requests products for a category that has child categories holding published products
|
||||
- **THEN** the response contains the products assigned to those descendant categories, not only those assigned directly to the requested category
|
||||
|
||||
#### Scenario: sort by lowest active SKU price
|
||||
- **WHEN** a shopper requests the product list with `sort=price` and `order=asc`
|
||||
- **THEN** products come back ordered by their lowest active SKU price ascending
|
||||
|
||||
#### Scenario: sort by units sold
|
||||
- **WHEN** a shopper requests the product list with `sort=sales` and `order=desc`
|
||||
- **THEN** products come back ordered by their `sold_count` descending, and a product with no paid orders reports zero rather than being omitted
|
||||
|
||||
#### Scenario: filter by brand
|
||||
- **WHEN** a shopper requests products with a `brand_id` alongside a `category_id`
|
||||
- **THEN** only products matching both filters are returned, and `total` reflects the combined filter
|
||||
|
||||
#### Scenario: unsupported sort is rejected
|
||||
- **WHEN** a client requests a `sort` value that is neither `price` nor `sales`
|
||||
- **THEN** the API responds 400 with an `ApiError` body instead of ignoring the parameter
|
||||
|
||||
#### Scenario: unpublished products never appear
|
||||
- **WHEN** any public listing or filter is applied
|
||||
- **THEN** products that are not `published`, or whose shop is not active, are absent from both `items` and `total`
|
||||
|
||||
**Reason**: Re-filed under `product` capability (the listing endpoint itself, its lifecycle/shop-isolation/sort/paging contract); the category-subtree matching behavior it depends on is separately documented under the new `category` capability. No behavior change.
|
||||
**Migration**: See `specs/product/spec.md` ("Public product browse") and `specs/category/spec.md` ("Category subtree browse filtering").
|
||||
@@ -0,0 +1,15 @@
|
||||
## ADDED Requirements
|
||||
|
||||
### Requirement: Localized category content
|
||||
Category names SHALL be stored as JSONB maps keyed by locale (`en`, `zh`). The API MUST return the full map; clients pick the display locale.
|
||||
|
||||
#### Scenario: bilingual round-trip
|
||||
- **WHEN** an admin creates a category with name `{"en": "Mugs", "zh": "马克杯"}`
|
||||
- **THEN** `GET /api/categories` returns the identical map for that category
|
||||
|
||||
### Requirement: Category subtree browse filtering
|
||||
`GET /api/categories` SHALL return the full category tree as a flat list, each with `id`, `parent_id`, `name`, `slug`, and `position`, ordered by `position` then `slug`. When a product listing is filtered by `category_id`, the filter SHALL match that category **and every category beneath it** in the tree, so requesting a parent category returns products assigned to its child and grandchild categories.
|
||||
|
||||
#### Scenario: parent category includes descendant products
|
||||
- **WHEN** a shopper requests products for a category that has child categories holding published products
|
||||
- **THEN** the response contains the products assigned to those descendant categories, not only those assigned directly to the requested category
|
||||
@@ -0,0 +1,62 @@
|
||||
## ADDED Requirements
|
||||
|
||||
### Requirement: Localized product content
|
||||
Product names/descriptions SHALL be stored as JSONB maps keyed by locale (`en`, `zh`). The API MUST return the full map; clients pick the display locale.
|
||||
|
||||
#### Scenario: bilingual round-trip
|
||||
- **WHEN** a shop owner creates a product with name `{"en": "Mug", "zh": "马克杯"}`
|
||||
- **THEN** both public detail and shop-admin GET return the identical map
|
||||
|
||||
### Requirement: Publish lifecycle
|
||||
Products SHALL have status `draft | published | unpublished`. Only `published` products appear in public list/detail.
|
||||
|
||||
#### Scenario: publish then unpublish
|
||||
- **WHEN** a product is published
|
||||
- **THEN** it appears in `GET /api/products`
|
||||
- **WHEN** it is unpublished
|
||||
- **THEN** public detail returns 404 and it disappears from listings
|
||||
|
||||
#### Scenario: publish requires sellable SKU
|
||||
- **WHEN** publishing a product with no active SKU having price > 0
|
||||
- **THEN** the API returns 400
|
||||
|
||||
### Requirement: Shop isolation
|
||||
Shop-role users SHALL only see and mutate their own shop's products under `/api/shop/products`.
|
||||
|
||||
#### Scenario: cross-shop access denied
|
||||
- **WHEN** shop owner A requests `/api/shop/products/{id}` of shop B
|
||||
- **THEN** the API returns 404
|
||||
|
||||
### Requirement: SKU pricing
|
||||
Each SKU SHALL carry `price_minor` (integer minor units) and an ISO `currency` code; stock is a non-negative integer.
|
||||
|
||||
#### Scenario: negative stock rejected
|
||||
- **WHEN** upserting a SKU with stock < 0
|
||||
- **THEN** the API returns 400
|
||||
|
||||
### Requirement: Public product browse
|
||||
Public `GET /api/products` SHALL return only `published` products whose shop is active, and SHALL remain readable without authentication. When `category_id` is supplied, the filter SHALL match that category and every category beneath it (per the `category` capability's subtree matching). When `brand_id` is supplied the filter SHALL match that brand and compose with the other filters. The listing SHALL accept an optional `sort` of `price` or `sales`: `price` orders by each product's lowest active SKU price, and `sales` orders by units sold across orders that reached payment, which SHALL also be reported per product as `sold_count`. Any other `sort` value SHALL be rejected with a 400 `ApiError` rather than silently ignored. An unsorted listing SHALL order newest first. Paging SHALL keep returning `page` and `per_page` alongside the filtered `total`.
|
||||
|
||||
#### Scenario: parent category includes descendant products
|
||||
- **WHEN** a shopper requests products for a category that has child categories holding published products
|
||||
- **THEN** the response contains the products assigned to those descendant categories, not only those assigned directly to the requested category
|
||||
|
||||
#### Scenario: sort by lowest active SKU price
|
||||
- **WHEN** a shopper requests the product list with `sort=price` and `order=asc`
|
||||
- **THEN** products come back ordered by their lowest active SKU price ascending
|
||||
|
||||
#### Scenario: sort by units sold
|
||||
- **WHEN** a shopper requests the product list with `sort=sales` and `order=desc`
|
||||
- **THEN** products come back ordered by their `sold_count` descending, and a product with no paid orders reports zero rather than being omitted
|
||||
|
||||
#### Scenario: filter by brand
|
||||
- **WHEN** a shopper requests products with a `brand_id` alongside a `category_id`
|
||||
- **THEN** only products matching both filters are returned, and `total` reflects the combined filter
|
||||
|
||||
#### Scenario: unsupported sort is rejected
|
||||
- **WHEN** a client requests a `sort` value that is neither `price` nor `sales`
|
||||
- **THEN** the API responds 400 with an `ApiError` body instead of ignoring the parameter
|
||||
|
||||
#### Scenario: unpublished products never appear
|
||||
- **WHEN** any public listing or filter is applied
|
||||
- **THEN** products that are not `published`, or whose shop is not active, are absent from both `items` and `total`
|
||||
@@ -0,0 +1,42 @@
|
||||
## 1. Create `modules/category`
|
||||
|
||||
- [x] 1.1 Create `apps/api/src/modules/category/mod.rs` (mirror `catalog/mod.rs` shape: `mod dto; mod handlers; pub mod service;` + `router()`).
|
||||
- [x] 1.2 Move `Category` struct from `models.rs` into `category/dto.rs` (or a `category/models.rs` if the module needs more than one type later).
|
||||
- [x] 1.3 Move `list_categories` (the `SELECT id, parent_id, name, slug, position FROM categories ORDER BY position, slug` query) from `catalog/service.rs` into `category/service.rs`.
|
||||
- [x] 1.4 Move the `SUBTREE_CTE` constant (`WITH RECURSIVE subtree AS (...)`) into `category` — expose it as `pub(crate) const SUBTREE_CTE` (or a small helper fn) so `product/service.rs` can still build the category-subtree-filtered product query.
|
||||
- [x] 1.5 Move the `GET /categories` route + handler from `catalog/handlers.rs` into `category/handlers.rs`.
|
||||
|
||||
## 2. Create `modules/brand`
|
||||
|
||||
- [x] 2.1 Create `apps/api/src/modules/brand/mod.rs` (same shape as above).
|
||||
- [x] 2.2 Move `Brand` struct from `models.rs` into `brand/dto.rs`.
|
||||
- [x] 2.3 Move `list_brands`, `BrandInput`, `default_active`, and `replace_brands` (including its slug/bilingual-name validation and the transactional delete+reinsert) from `catalog/service.rs` into `brand/service.rs`.
|
||||
- [x] 2.4 Move the `GET /brands` and `PUT /admin/brands` routes + handlers (`list_brands`, `replace_brands`) from `catalog/handlers.rs` into `brand/handlers.rs`.
|
||||
|
||||
## 3. Create `modules/product`
|
||||
|
||||
- [x] 3.1 Create `apps/api/src/modules/product/mod.rs` (same shape as above).
|
||||
- [x] 3.2 Move `product/dto.rs`: `ProductWithSkus`, `PublicListQuery`, `SortBy` (+ `sort_by()`/`ascending()`), `ProductBody`, `SkuBody` from `catalog/dto.rs` unchanged.
|
||||
- [x] 3.3 Move `product/repo.rs`: `attach_skus` and `SoldRow` from `catalog/repo.rs` unchanged (only touches `Product`/`Sku`/order tables, no category/brand coupling).
|
||||
- [x] 3.4 Move `product/service.rs`: `list_public`, `get_public`, `load_own_product`, `list_shop_products`, `get_shop_product`, `validate_product_body`, `create_product`, `update_product`, `transition`, `upsert_sku`, plus the `SOLD_UNITS`, `MIN_PRICE`, `PRODUCT_COLS` constants — import `category`'s `SUBTREE_CTE` instead of redefining it locally.
|
||||
- [x] 3.5 Move `product/handlers.rs`: `/products`, `/products/{id_or_slug}`, `/shop/products`, `/shop/products/{id}`, `/shop/products/{id}/publish`, `/shop/products/{id}/unpublish`, `/shop/products/{id}/skus` routes + handlers, and the `ShopListQuery` struct.
|
||||
- [x] 3.6 Keep `Product` and `Sku` structs in `crate::models` (do not move) — confirmed cross-module dependents in `favorite`, `flash_sale`, `group_buying`.
|
||||
|
||||
## 4. Wire up and remove the old module
|
||||
|
||||
- [x] 4.1 In `apps/api/src/modules/mod.rs`: replace `pub mod catalog;` with `pub mod product; pub mod category; pub mod brand;`, and replace `.merge(catalog::router())` with `.merge(product::router()).merge(category::router()).merge(brand::router())`.
|
||||
- [x] 4.2 Update `apps/api/src/models.rs` to remove the (now-moved) `Category` and `Brand` structs.
|
||||
- [x] 4.3 `rg -n "modules::catalog|catalog::"` across `apps/api/src` and fix any remaining references.
|
||||
- [x] 4.4 Delete `apps/api/src/modules/catalog/`.
|
||||
|
||||
## 5. Verify
|
||||
|
||||
- [x] 5.1 `cargo build` (or `cargo check`) in `apps/api` — zero errors.
|
||||
- [x] 5.2 `cargo clippy` — zero new warnings.
|
||||
- [x] 5.3 `cargo test` — `apps/api/tests/catalog.rs` and the full suite pass unchanged (routes/behavior are identical).
|
||||
- [x] 5.4 Manually diff route list (`rg "\.route\(" apps/api/src/modules/{product,category,brand}`) against the original `catalog/handlers.rs` router to confirm no route was dropped or duplicated.
|
||||
- [x] 5.5 `rg -n "modules/catalog|modules::catalog"` across `docs/` (e.g. `docs/tech-specs/rust-api.md`) and update any references to the old module name.
|
||||
|
||||
## 6. Optional test-file parity
|
||||
|
||||
- [ ] 6.1 (Optional, per design.md open question) Rename/split `apps/api/tests/catalog.rs` into `tests/product.rs`, `tests/category.rs`, `tests/brand.rs` to mirror the new module boundaries — only if desired; not required since these are HTTP-level integration tests unaffected by internal module names.
|
||||
Reference in New Issue
Block a user