Conversation
…ository functions
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
Clean implementation of CRUD repository functions following the established credentialsRepository pattern. All CI checks pass. The code is correct, consistent with existing patterns, and ready to merge.
Should Fix (non-blocking)
OrgUser.role typed as string instead of the domain union
src/db/repositories/usersRepository.ts:19 — OrgUser.role is typed as string, while the existing DashboardUser.role uses the union type 'member' | 'admin' | 'superadmin'. The same loose typing applies to createUser (line 118) and updateUser (line 138) params.
getUserById already validates roles at runtime with VALID_ROLES. The new functions skip that validation, which means callers can pass (or receive) arbitrary role strings without any type-level or runtime guard.
This isn't blocking — the schema column is text with no DB constraint, and the callers don't exist yet. But when the service/API layer is built on top, consider:
- Extracting a shared
UserRoletype (e.g.,type UserRole = 'member' | 'admin' | 'superadmin') - Using it for
OrgUser.role,createUser.role, andupdateUser.role - This gives compile-time safety and makes the valid values discoverable
Everything else looks good — sparse update pattern matches credentialsRepository.updateCredential, passwordHash is correctly excluded from listOrgUsers select, createUser properly delegates hashing to the caller, and delete/update-by-id without org-scoping is consistent with the existing credential CRUD pattern. Tests are well-structured using the shared mockDb helper.
Summary
src/db/repositories/usersRepository.tswith four new CRUD functions:listOrgUsers,createUser,updateUser,deleteUsercredentialsRepository.tspattern — standalone async functions usinggetDb(), org-scoped querieslistOrgUsersselectsid, orgId, email, name, role, createdAt, updatedAtand intentionally omitspasswordHashcreateUseraccepts a pre-hashed password (bcrypt hashing is the caller's responsibility) and returns{ id }updateUserperforms a sparse update (only provided fields), always setsupdatedAtdeleteUserdeletes by id; sessions auto-cascade via existingON DELETE CASCADEFK constraintgetUserByEmail,getUserById,createSession, etc.) remain unchangedTest plan
listOrgUsersdoes not includepasswordHashin resultsupdateUsersetsupdatedAton every callcreateUserstores pre-hashed password as-isTrello card: https://trello.com/c/42kHRzay/320-as-an-admin-i-want-users-repository-crud-functions-so-that-the-data-layer-supports-user-management
🤖 Generated with Claude Code