Repository navigation
feat(categories): category CRUD with pagination, caching and Swagger docs - #5
Merged
Merged
Conversation
Adds the VineJS validators for the categories endpoints, compiled with vine.compile() per the project's validation rules. categoryName is trimmed and bounded at maxLength(255) to match the categories.category_name VARCHAR(255) column from the core schema migration, with minLength(1) added on top since the column's own NOT NULL constraint does not by itself rule out a blank string, the same reasoning role_validator.ts already applies to roleName. updateCategorySchema reuses the same bound but marks the field optional so PUT requests can send a partial payload, following the createCategorySchema/updateCategorySchema pair the README's own code sample for this branch shows.
Adds findAll, findOne, create, update and remove for the categories table, following the same shape RolesService and PermissionsService already established: no constructor dependencies so no @Inject(), findOrFail for detail lookups so a missing id surfaces as a clean 404, and toPageResponse() wrapping Lucid's paginate() output rather than a hand rolled envelope. findAll is cached through @adonisjs/cache's getOrSet(), scoped to a dedicated 'categories' namespace and keyed by page and limit so different pages never collide or serve each other's results. create, update and remove all clear that namespace before returning rather than trying to reason about which cached page a given write affects, since any insert or delete shifts the contents of every later page anyway; a short TTL is kept as a backstop. remove also enforces the README's business rule that a category still referenced by at least one product cannot be deleted, throwing a 409 via createError the same way RolesService's E_ROLE_IN_USE does, which doubles as the guard that keeps the products table's own ON DELETE CASCADE foreign key from ever actually firing through this service.
Adds the thin HTTP layer over CategoriesService: index and show are public, store, update and destroy validate the request with request.validateUsing() against the category_validator schemas and delegate everything else to the service, matching the project's rule that controllers never touch a model or contain business logic directly. paginationSchema handles page and limit on the list endpoint the same way it already does for roles and permissions. Each action carries adonis-autoswagger JSDoc annotations (@summary, @description, @paramQuery/@parampath, @RequestBody, @responsebody) per this branch's own task checklist item. @RequestBody references the exported validator names directly (createCategorySchema, updateCategorySchema) since adonis-autoswagger resolves schemas from app/validators/* by their exported identifier, and @responsebody uses the Category model schema autoswagger derives from app/models automatically. The paginated list response is documented with the package's own <Model[]>.paginated() helper as the closest built in approximation of this project's PageResponse envelope, since autoswagger has no first class notion of a custom pagination shape.
Registers GET /api/v1/categories, GET /api/v1/categories/:id, POST /api/v1/categories, PUT /api/v1/categories/:id and DELETE /api/v1/categories/:id, matching the README's Endpoints table for this branch exactly on method, path and access. The two GET routes stay public; the three mutation routes are each chained individually with .use([middleware.auth(), middleware.role({ roles: ['ADMIN'] })]), the exact per-route pattern the README's own routes.ts code sample for this branch shows, rather than a grouped prefix the way the user/role/permission administration routes use, since categories mixes public and ADMIN-only routes under the same resource.
CategoriesController is imported lazily, consistent with every other controller in this file and required for adonis-autoswagger to pick up its JSDoc annotations.
… rule The full CRUD surface plus the rule that a category with at least one product still associated with it cannot be deleted, mirroring the pattern already established for RolesService and PermissionsService. findAll's caching is exercised implicitly rather than through the cache service directly, every test reads the list immediately after a write that clears the categories cache namespace, proving the invalidation actually happens rather than only trusting the TTL.
Full CRUD lifecycle over real requests, the 409 business rule surfaced through the HTTP layer rather than only the service, and the auth matrix for the three ADMIN-only mutation routes. Mirrors roles.spec.ts and permissions.spec.ts's own reasoning: the unit spec already proves correctness at the service layer, this proves the controller and route wiring actually expose it the same way.
…-test stubs feature/categories just added the first real ADMIN-protected route, categories, so the two /api/v1/_stub/* routes that existed solely to give feature/auth's middleware smoke test something to hit before any real protected resource existed are no longer needed, exactly as their own comment said they would be. The unused respond import that only those inline route handlers needed is removed too.
The stub routes this test hit are removed now that a real
ADMIN-protected route exists. GET /api/v1/auth/me replaces the
auth-only stub, since it requires middleware.auth() with no role
check, the same shape the stub had. POST /api/v1/categories replaces
the admin-protected stub, since it requires middleware.auth() plus
middleware.role({ roles: ['ADMIN'] }), the same shape that stub had.
The test still proves exactly what it proved before, 401 without a
token, 200/403/201 with the right or wrong role, just against routes
with real business meaning instead of scaffolding.
This fake HttpContext's request.url() never issues a real HTTP request, it only feeds the fail() envelope's path field, but it referenced a stub route that no longer exists in start/routes.ts. Pointed at a real route for accuracy, no behavior change.
Both auto-generated, refreshed to include CategoriesController and the categories table's current shape. Neither is hand edited.
GET/PUT/DELETE /api/v1/categories/:id, and the equivalent :roleId/ :permissionId params on the admin routes, coerced params.id with a bare Number(...) in their controllers. A non-numeric value produced NaN, which reached Postgres unguarded as "WHERE id = NaN" and raised a raw DatabaseError with no .status property, falling through the exception handler's generic branch to an unhandled 500 instead of a clean 404. Verified this exact failure against a running dev server before fixing it. router.matchers.number() restricts these params to digit strings before a route ever matches, and casts the matched value to a real number, so a non-numeric id 404s at the routing layer instead of reaching the service at all. Applied once as a global matcher via router.where() rather than per-route, so it fixes the same latent issue in RolesController and PermissionsController too, not just the categories controller this branch adds.
… check
cache.namespace('categories') is a pure lookup with no per-call state,
so it was re-invoked identically at all four call sites in this
service for no behavioral gain. Hoisted once as a module constant
instead.
The still-in-use check in remove() also now selects only the id
column instead of every column on the matching product row, since the
query only ever needs to know whether a match exists, not what it
contains.
CategorySeeder writes directly through the Category model rather than CategoriesService, so it never triggered the cache invalidation CategoriesService.create/update/remove all call after a write. Reseeding an environment where the categories list endpoint had already been hit once would serve a stale cached page for up to the service's five minute TTL, even though the database itself was already correct. The seeder now clears the same cache namespace directly after seeding.
The middleware smoke test migrated off the removed /api/v1/_stub/* routes onto POST /api/v1/categories, which means its ADMIN-success case now implicitly depends on createCategorySchema and CategoriesService.create working correctly, not just the role check itself. A future change to that validator or service could fail this test for the wrong reason, misdiagnosed as an auth regression, since nothing pinned RoleMiddleware's own branching logic down in isolation the way auth_middleware.spec.ts already does for AuthMiddleware. This exercises RoleMiddleware.handle() directly against a fake HttpContext, no HTTP round trip and no dependency on any particular business route, so a regression in the role check itself fails here specifically regardless of which route happens to be ADMIN protected by the time this runs.
body() was independently copy-pasted into roles.spec.ts, permissions.spec.ts and categories.spec.ts, and uniqueName() into categories_service.spec.ts and categories.spec.ts, both the exact same implementation each time. Centralized here so a future spec file imports one implementation instead of writing a fourth or fifth copy, and so a change to either (for example the ApiResponse shape body() works around) only needs to happen once.
Replaces this file's own local copies with imports from tests/helpers/test_utils.ts, no behavior change.
Replaces this file's own local copy with an import from tests/helpers/test_utils.ts, no behavior change.
Replaces this file's own local copy with an import from tests/helpers/test_utils.ts, no behavior change.
Replaces this file's own local copy with an import from tests/helpers/test_utils.ts, no behavior change.
Auto-generated, refreshed to reflect the new global route param matchers. Neither is hand edited.
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
This is the first complete vertical slice: Category CRUD with pagination, ADMIN protected mutations, cached list reads, and Swagger documentation, all built by a single agent since the scope is a straightforward CRUD branch with no new architectural decisions to make.
The paginated categories list is cached through @adonisjs/cache (installed and configured on feature/core-architecture specifically for this), an in-memory layer over a database backed layer, keyed by page and limit inside a dedicated namespace that every create, update and delete clears. A category with at least one associated product cannot be deleted, returning 409 rather than letting the migration's cascading foreign key silently remove products out from under an admin who never asked for that.
A multi-angle code review after the initial implementation surfaced four things worth fixing. A non-numeric route id (GET /categories/abc, and the same shape on the roles/permissions admin routes from the previous branch) fell through to Postgres unguarded and produced a raw 500 instead of a clean 404; fixed once as a global route param matcher rather than per controller, so it also closes the same latent gap in RolesController and PermissionsController. CategorySeeder wrote directly through the Category model, bypassing the service's cache invalidation, so reseeding an already-warm environment would have served a stale list for up to five minutes; the seeder now clears the same cache namespace. The middleware smoke test that used to hit temporary stub routes was migrated onto this branch's own real ADMIN protected route once the stubs were removed, which two independent review angles flagged as coupling RoleMiddleware's regression coverage to this branch's own validator and service correctness; a dedicated RoleMiddleware unit test now pins that logic down in isolation, the same way AuthMiddleware already has one. A handful of test helper functions that had been copy-pasted two or three times across the auth and categories test files were consolidated into one shared file.
Task checklist
Every task in the README's feature/categories section is complete: createCategorySchema and updateCategorySchema, CategoriesService with cached findAll, findOne, create, update and remove, the 409 business rule for categories with products, CategoriesController with Swagger annotations, routes with access rules applied at declaration time, and the full unit and functional test suite including the auth matrix (401 without a token, 403 for USER, 201 for ADMIN).
Test plan
npm run typecheck, npm run lint and npm test all pass locally against a live PostgreSQL instance, 56 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. The route param matcher fix was also verified manually against a running dev server, confirming GET /api/v1/categories/abc now returns a clean 404 instead of the raw 500 it produced before the fix.