Repository navigation
feat(products): product CRUD with category filtering, sorting and Swagger docs - #6
Merged
Merged
Conversation
ProductsService.remove needs to check whether at least one order still references a product before allowing the delete, the same guard CategoriesService.remove already applies against products through Category's own hasMany relation. Product previously declared only the belongsTo side toward Category with no way to reach its orders through the ORM, so the delete guard had no relation to query. The Customer model already added the equivalent inverse hasMany toward Order for the same reason, with its own NOTE explaining why a relation not explicitly listed in a Tasks checklist was still worth adding, so this follows that precedent rather than querying Order directly with a hand-written where clause.
…uctFilterSchema
createProductSchema and updateProductSchema mirror the products table
from the README's Data Model section, with unitPrice.positive() enforcing
the same "> 0" rule the migration already enforces at the database level
via its check constraint, so a bad value is rejected with a clean 422
before it ever reaches Postgres. categoryId is validated for shape only
here; whether it refers to an existing category is a FK existence check
left to ProductsService.
productFilterSchema backs the list endpoint's query params. sortBy is
restricted with vine.enum() to the two documented sortable fields instead
of a bare string, closing off the possibility of an arbitrary column name
reaching the service's dynamic orderBy call. The allowlist deliberately
uses the camelCase model attribute names (productName, unitPrice) rather
than their snake_case column names, since Lucid's ModelQueryBuilder
already resolves a known attribute name to its column automatically, the
same way RolesService relies on for its own where('roleName', ...) call.
… guard Follows CategoriesService's own findAll/findOne/create/update/remove shape, minus the caching layer, since the README only asks for caching on the categories list and does not ask for it here. findAll builds the query the README's own "Dynamic QueryBuilder with optional filters" code sample shows: category is always preloaded, and the where(category_id) and orderBy(sortBy) calls are only chained on when the corresponding filter field is present, so an unfiltered, unsorted request still returns every product. create and update both check the referenced category exists via Category.findOrFail before writing the product row. Without that check a bad categoryId would only surface as a raw Postgres foreign key violation with no clean status code, the same class of bug a previous branch's code review caught for a different case, instead of the clean 404 a client can act on directly. remove enforces the README's business rule that a product still referenced by at least one order cannot be deleted, throwing a createError-based E_PRODUCT_IN_USE 409, the same pattern E_CATEGORY_IN_USE already establishes. It checks the orders relation added to the Product model in the previous commit rather than querying Order directly, keeping the same related(...).query().first() idiom CategoriesService.remove already uses for its own guard.
Thin HTTP layer over ProductsService, matching CategoriesController's own shape and Swagger JSDoc style: every action does validateUsing -> service -> respond(), with no business logic and no direct model access, per the project's hard rules. GET routes are declared public in start/routes.ts and the three mutation routes are declared ADMIN only there, mirroring exactly how the categories routes are gated. index validates pagination and product filtering as two separate request.validateUsing() calls against paginationSchema and productFilterSchema rather than merging them into one combined schema. Both run against the same query string and VineJS does not mind the two calls overlapping in what they read, and keeping them separate lets paginationSchema stay the same shared schema every other list endpoint already uses instead of forking a products-specific variant of it.
Adds the five routes from the README's feature/products Endpoints table:
list and detail are public, and the three mutation routes are each
individually declared behind .use([middleware.auth(),
middleware.role({ roles: ['ADMIN'] })]), the exact pattern the categories
routes already establish just above them in this file. The :id param
needs no new registration since router.where('id',
router.matchers.number()) at the top of this file already covers every
route using that param name, products included.
Picks up ProductsController in the generated controllers map after the routes that reference it were added, following the same regeneration pattern the previous branches' codegen commits already establish.
Covers the full CRUD surface the same way categories_service.spec.ts does, plus the two behaviors specific to products: findAll filtering by categoryId returns only matching rows, and sorting by unitPrice actually orders the results. create is proven to throw a 404 when categoryId does not reference an existing category, update is proven to throw the same 404 when a categoryId in the payload does not exist, and remove is proven to throw the 409 E_PRODUCT_IN_USE error once an order references the product and to succeed once none do. Runs against the real test database inside a rolled-back global transaction, the same pattern categories_service.spec.ts uses, since the categoryId filter and the order-guard rule only mean anything with real category, product and order rows to check against.
Follows categories.spec.ts's pattern closely: a full create/list/read/ update/delete lifecycle over real HTTP requests asserting every response matches the ApiResponse envelope, plus products-specific coverage the categories spec has no equivalent of. That includes the nested category object appearing in both the list and detail responses, categoryId filtering and unitPrice sorting together on the list endpoint, a 404 when POSTing with a categoryId that does not exist, and the 409 order-guard on delete succeeding once the blocking order is removed. A separate mutation-route-authorization group proves the same auth matrix categories.spec.ts proves for its own mutation routes: POST without a token returns 401, with a USER token returns 403, and with an ADMIN token returns 201, confirming the routes are actually gated the way start/routes.ts declares them rather than just trusting the declaration.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Product CRUD linked to categories, filterable by categoryId, sortable by productName or unitPrice, with the category always preloaded on both list and detail responses. Built by a single agent in one pass, no separate QA hand-off, since this is a well-scoped CRUD branch that closely mirrors the categories branch's own shape.
Creating or updating a product checks that the referenced category actually exists first, returning a clean 404 instead of letting a bad categoryId fall through to a raw Postgres foreign key violation. A product linked to at least one order cannot be deleted, returning 409 rather than letting the migration's cascading foreign key silently remove order history. The inverse orders relation this delete guard needs was added to the Product model, the same way the Customer model already added its own orders relation for the same reason.
A code review after the initial implementation found two check-then-act race conditions, a concurrent category deletion between the existence check and the product write, and a concurrent order insert between the in-use check and the product delete. Both are the same class of finding already surfaced and consciously left unfixed on the categories and roles/permissions branches: real in theory, but disproportionate effort to close properly (transaction-wrapped row locking) against this tutorial project's actual concurrency profile. No other findings.
Task checklist
Every task in the README's feature/products section is complete: createProductSchema, updateProductSchema and productFilterSchema, ProductsService with dynamic QueryBuilder filtering and sorting plus the category-existence and order-in-use guards, ProductsController with Swagger annotations, routes with ADMIN protection on mutations, and the full unit and functional test suite including filtering, sorting, the nested category in responses, and the auth matrix.
Test plan
npm run typecheck, npm run lint and npm test all pass locally against a live PostgreSQL instance, 74 tests passing with zero failures, verified against a database reset to a single fresh migration batch to match what a clean CI run will see.