Repository navigation
feat(customers): customer CRUD with search and email uniqueness - #7
Merged
Merged
Conversation
Customers introduce a field this project has not validated before, a telephone number, and the README leaves the exact pattern to this validator's own judgment the same way auth_validator.ts already documents its own choice of password complexity rule. The chosen pattern accepts a permissive but still bounded shape (optional leading plus, digits, spaces, hyphens, dots or parentheses, 7 to 20 characters) rather than trying to validate a specific country's numbering plan, which is a much larger problem than this branch needs to solve. Field length bounds mirror the README's Column Details table for the customers table exactly (first_name/last_name/address VARCHAR(255), telephone VARCHAR(50), email VARCHAR(255)), and a small customerSearchSchema is added alongside the create/update schemas so the list endpoint's ?search= query param gets a compiled schema of its own instead of being read directly in the controller, per this project's hard rule against inline VineJS schemas.
…queness
Owns every read and write to the customers table. findAll builds its
case-insensitive search across first name, last name and email exactly per
the README's own "ILIKE search in the service" code sample, scoping the
three orWhereILike calls inside a single where((q) => ...) group so a
future filter added to this method never accidentally falls inside that
OR chain. create and update enforce email uniqueness at the service layer,
the same E_EMAIL_TAKEN 409 pattern UsersService.update already establishes
for the same problem, including its "only re-check when the email is both
present and actually different from the current value" idiom so updating
a customer's address alone never trips a false conflict against their own
row.
remove enforces this branch's own business rule that a customer with at
least one order still associated with it cannot be deleted, checked with
the same related('orders').query().select('id').first() idiom
ProductsService.remove already uses for its own delete guard against
orders, made possible by the orders hasMany already present on the
Customer model from an earlier branch. Like ProductsService, this service
has no constructor dependencies and no caching, since the README does not
ask for caching on the customers list.
Thin HTTP layer over CustomersService: every action validates its input with the compiled schemas from customer_validator.ts, delegates to the service, and shapes the response with respond(), never touching the Customer model directly. index validates pagination and search as two separate request.validateUsing() calls against the same query string, the same split ProductsController.index already uses for pagination and its own categoryId/sortBy/order filter, since both concerns are independently reusable and neither needs to know about the other. Swagger JSDoc annotations follow the same @index/@show/@store/@update/ @destroy tag style as CategoriesController and ProductsController, with 401 and 403 response bodies documented on every action since, unlike those two controllers, every route on this one requires authentication, not only the mutation routes. The actual auth and role middleware enforcement is wired up at route declaration time in start/routes.ts, not here.
Adds GET/GET-detail/POST/PUT/DELETE under /api/v1/customers, matching the
README's Endpoints table for this branch exactly. This is the first
resource in the project where the list and detail routes are not public:
both are declared with .use(middleware.auth()) alone rather than left
unprotected, since the README's Authorization Rules table places customers
and orders under "Authenticated" for GET, not "Public" the way categories
and products are. The three mutation routes keep the same
.use([middleware.auth(), middleware.role({ roles: ['ADMIN'] })]) pattern
the categories and products mutation routes already use.
The :id param is already covered by the router.where('id',
router.matchers.number()) matcher declared once at the top of this file,
so a non-numeric customer id 404s at the routing layer without any extra
registration here.
Picks up CustomersController 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 plus the two business rules the README calls out for this branch: the ILIKE search filters correctly by first name, last name and email individually; create throws a 409 E_EMAIL_TAKEN for a duplicate email; update allows keeping the same email on the same customer while still throwing 409 when that email belongs to a different customer; and remove throws a 409 E_CUSTOMER_HAS_ORDERS once an order references the customer, then leaves the row in place. These tests hit the real test database inside a rolled-back global transaction, the same pattern products_service.spec.ts already uses, since both the search filters and the order-guard rule only mean anything once real customer and order rows exist to search or reference.
Exercises the full CRUD lifecycle over real HTTP requests, email uniqueness enforced on both create and update, and search by first name/last name/email, all through the ApiResponse envelope. This file also proves the auth matrix the README specifies for this branch: unlike categories.spec.ts and products.spec.ts, GET /customers itself requires a token here (401 with none, 200 with a USER token), since this is the first resource where the list and detail routes are not public, while the three mutation routes still follow the familiar 401/403/2xx progression across no token, a USER token and an ADMIN token. The unit spec for CustomersService already proves the email-uniqueness and order-guard checks throw the right errors at the service layer; this file proves the HTTP layer surfaces all of that correctly and that AuthMiddleware and RoleMiddleware are wired up exactly as the README's Endpoints table describes.
Neither create() nor update() lowercased the email before comparing or storing it, unlike every other email uniqueness check in this codebase (AuthService's register, login and forgotPassword, and UsersService.update all lowercase first). The customers.email column is a plain case-sensitive unique index, so Jane@Example.com and jane@example.com would have passed as distinct values and registered as two customers, defeating the "email uniqueness enforcement" this branch is specifically named for in the README. Both methods now lowercase the email before the uniqueness check and before the write, matching the pattern already established elsewhere, caught during code review before this shipped rather than after.
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
Customer CRUD with case insensitive ILIKE search across first name, last name and email, and email uniqueness enforced at the service layer with the same update-time exception for a record's own current value already established by UsersService. Built by a single agent in one pass, no separate QA hand-off, since this branch closely mirrors the shape already set by categories and products. Unlike those two branches, every route here requires authentication, not just the mutation routes, since customer records are not public data the way a product catalog is.
A customer with at least one existing order cannot be deleted, returning 409 rather than letting the migration's cascading foreign key silently remove order history, the same guard pattern already used for categories and products.
Code review caught one real bug before merge: neither create() nor update() lowercased the email before checking uniqueness or writing it, unlike every other email uniqueness check already in this codebase. Jane@Example.com and jane@example.com would have passed as distinct values and registered as two customers, directly contradicting this branch's own "email uniqueness enforcement" requirement, since the database's own unique index is exact-string and equally case-sensitive. Fixed to lowercase before both the check and the write, matching AuthService and UsersService's established pattern. Two secondary findings, a check-then-write race on the uniqueness check and light duplication between create() and update()'s conflict lookup, are the same class of finding already surfaced and consciously left unfixed on earlier branches, not worth the effort against this tutorial project's actual risk profile.
Task checklist
Every task in the README's feature/customers section is complete: createCustomerSchema and updateCustomerSchema, CustomersService with ILIKE search and the email uniqueness guards, CustomersController with Swagger annotations, routes with auth required everywhere and ADMIN required additionally on mutations, and the full unit and functional test suite including search, email uniqueness over HTTP, and the auth matrix.
Test plan
npm run typecheck, npm run lint and npm test all pass locally against a live PostgreSQL instance, 97 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.