This repository was archived by the owner on May 12, 2026. It is now read-only.
feat: set up database schema from ERD - #4
Merged
Homoakin619 merged 2 commits intoMay 8, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAuth, user, and notifications simplified; OTP/Profile removed; comprehensive initial migration added; new entities/modules introduced (funnels, strategies, weekly logs, subscriptions, uploads, waitlist); AppModule and TypeORM config updated; tests streamlined; new system message constants added. ChangesCore refactor and new modules
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as Auth Controller
participant SVC as Auth Service
participant Repo as User Repository
participant JWT as JwtService
Client->>API: POST /auth/login
API->>SVC: login(dto)
SVC->>Repo: findOne(email)
SVC->>JWT: sign({id, sub, email})
SVC-->>API: {status_code, access_token, data}
API-->>Client: 200 OK
sequenceDiagram
participant Client
participant NCtrl as Notifications Controller
participant NSvc as Notifications Service
participant NRepo as Notification Repo
Client->>NCtrl: GET /notifications?page&limit
NCtrl->>NSvc: listForUser(userId, p, l)
NSvc->>NRepo: findAndCount + count(unread)
NSvc-->>NCtrl: {total, unread, notifications}
NCtrl-->>Client: 200 OK
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the authentication and user modules, updates the database schema with new entities (Waitlist, Strategies, Funnels, WeeklyLogs, Subscriptions, UploadedDocuments), and simplifies the user entity and notification system. The review identified several issues: the otp_code and expires_at fields in the User entity should be nullable to support non-email authentication methods, the database migration contains camelCase column names that violate the snake_case convention, and the NotificationPreference entity has a redundant unique constraint on user_id.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 30
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/modules/auth/dto/create-user.dto.ts (1)
36-45:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
@MaxLength(64)to the password field to prevent bcrypt truncation collisions.
bcrypt.hashsilently truncates input at 72 bytes, so any two passwords sharing the first 72 bytes will produce identical hashes. Without a maximum length constraint, this creates a collision vulnerability where distinct passwords can be used to authenticate as the same user. Additionally, allowing unbounded password length enables attackers to submit extremely long inputs that increase hashing overhead. Apply aMaxLengthconstraint within bcrypt's 72-byte limit—64 bytes provides a safe margin and aligns with OWASP recommendations for bcrypt-hashed passwords.Proposed fix
`@MinLength`(8) + `@MaxLength`(64) `@IsNotEmpty`() `@IsStrongPassword`( {}, { message: 'Password must contain at least one uppercase letter, one lowercase letter, one number, and one special character.', } ) password: string;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/modules/auth/dto/create-user.dto.ts` around lines 36 - 45, Add a MaxLength(64) constraint to the password property in the CreateUser DTO: import MaxLength from class-validator and annotate the password field (the property named "password" in the DTO with existing `@MinLength/`@IsStrongPassword decorators) with `@MaxLength`(64) so bcrypt truncation collisions and excessive-length inputs are prevented; ensure the import is added and any validation tests or error messages updated accordingly.src/modules/email/email.module.ts (1)
21-44:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
MailerModule.forRootAsyncis registered in bothEmailModuleandAppModulewith different template directories — one will silently win.
EmailModuleconfigures the template dir as…/hng-templates(Line 36) whileAppModuleuses…/templates(Line 89 ofapp.module.ts). When NestJS resolves the DI graph, one registration overwrites the other, so any template in the "losing" directory will fail to render at runtime with no compile-time warning.Remove
MailerModule.forRootAsyncfrom one of the two locations and reconcile the template directory to a single authoritative path. The canonical place for global infrastructure likeMailerModuleisAppModule.🛠️ Proposed fix (remove from EmailModule)
`@Module`({ providers: [EmailService, QueueService, EmailQueueConsumer], exports: [EmailService, QueueService], imports: [ TypeOrmModule.forFeature([User]), BullModule.registerQueueAsync({ name: 'emailSending', }), - MailerModule.forRootAsync({ - imports: [ConfigModule], - useFactory: async (configService: ConfigService) => ({ - transport: { ... }, - defaults: { ... }, - template: { - dir: process.cwd() + '/src/modules/email/hng-templates', - adapter: new HandlebarsAdapter(), - options: { strict: true }, - }, - }), - inject: [ConfigService], - }), ConfigModule, ], controllers: [EmailController], })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/modules/email/email.module.ts` around lines 21 - 44, The MailerModule.forRootAsync registration is duplicated between EmailModule and AppModule causing one to overwrite the other; remove the MailerModule.forRootAsync call from EmailModule and rely on the single global registration in AppModule, then reconcile the template directory there (choose either '/src/modules/email/hng-templates' or '/src/modules/email/templates') and update the MailerModule configuration in AppModule (transport, defaults, template.dir, HandlebarsAdapter usage) so all code using EmailModule relies on the single global MailerModule; also remove any MailerModule imports or forRoot/forRootAsync usage from EmailModule and ensure EmailModule injects MailerService (not re-register) if it needs to send mail.src/app.module.ts (1)
116-122:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winServeStaticModule root path mismatch — will not serve files from project-root
uploads/directory.At runtime,
join(__dirname, 'uploads')resolves todist/uploads/, but theuploads/directory exists at the project root. If files are written to the project-rootuploads/directory, they won't be served. Note: S3 is currently the primary upload mechanism; local file storage via the configured path isn't actively used, but this creates a latent bug if local storage is later implemented.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app.module.ts` around lines 116 - 122, The ServeStaticModule is using join(__dirname, 'uploads') which resolves to dist/uploads at runtime and won't serve files written to the project-root uploads directory; update the rootPath in ServeStaticModule.forRoot (the call shown using rootPath: join(__dirname, 'uploads')) to point to the project root, e.g. replace with join(process.cwd(), 'uploads') or path.resolve(process.cwd(), 'uploads') so the module serves files from the actual project-root uploads directory while preserving serveRoot: '/uploads' and serveStaticOptions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 29-31: The package.json lacks a prebuild hook for the
migration:revert script; add a premigration:revert script that runs the build
step (e.g., "premigration:revert": "npm run build") so migration:revert ("npx
typeorm migration:revert -d dist/src/database/data-source") executes against
fresh compiled output; update the scripts block to include this new
premigration:revert entry alongside premigration:run and migration:revert.
In `@src/database/migrations/1778257249178-InitSchemaFromErd.ts`:
- Around line 32-50: Add B-tree indexes on the foreign-key columns before adding
the FK constraints to avoid full-table scans on cascade/delete and lookups:
create indexes for strategies.user_id, weekly_logs.user_id,
weekly_logs.strategy_id, weekly_logs.funnel_stage_id, subscriptions.user_id,
uploaded_documents.user_id, token_usage.user_id, token_usage.strategy_id,
strategy_documents.strategy_id, strategy_documents.document_id,
notifications.user_id, admin_notifications.admin_id,
admin_notifications.related_user_id, notification_preferences.user_id,
funnel_tasks.funnel_stage_id, user_sessions.user_id and auth_metadata.user_id
(run CREATE INDEX ... for each) and then run the existing ALTER TABLE ... ADD
CONSTRAINT statements (e.g. those adding FK_c3e9760692a90d4f2d482ce60f8,
FK_b4dc06eede0bc611d500343b8ac, FK_4ebf38df71bf0c00d02251f888f,
FK_c48998b4b2015a8f58d145603de, FK_d0a95ef8a28188364c546eb65c1,
FK_b5423d1e7ccd9ff75ff3be1cc78, FK_f3e27270e575ec838d6039f6c5a,
FK_b2722408c71db7b997ecb5d26bc, FK_e38aab0d20f1a323911b7379ca4,
FK_82ffc490ced56c8a3ba94a7d50b, FK_9a8a82462cab47c73d25f49261f,
FK_c07cffb34f4e05a78f7663bc611, FK_47a62b5af42133558852323f1a2,
FK_64c90edc7310c6be7c10c96f675, FK_c23717cb7c7982dc0928d23d2a7,
FK_e9658e959c490b0a634dfc54783, FK_ddd81470f2c5703341629008c83) so indexes exist
prior to constraint creation.
In `@src/modules/auth/auth.service.ts`:
- Around line 86-103: changePassword currently updates the password but does not
invalidate prior JWTs, and createNewUser persists otp_code in plaintext; to fix,
add a per-user token invalidation field (e.g., tokens_invalidated_at or
token_version) on the User entity and increment or set it to now inside
changePassword (after hashing new password and before saving) so the JWT guard
(which must be updated to check token_version/tokens_invalidated_at in the token
payload against the DB in your auth guard) will reject old tokens, and change
createNewUser to hash otp_code before saving (use bcrypt or a secure HMAC/SHA256
with a server-side pepper) and update the OTP verification logic to compare
against the hashed value instead of plaintext; ensure you reference
userRepository and the User entity methods/fields (changePassword,
createNewUser, otp_code, tokens_invalidated_at/token_version) when making these
changes.
- Around line 23-56: createNewUser currently issues a JWT immediately even
though the new user is unverified; remove token issuance in createNewUser
(delete the jwtService.sign call and the access_token field in the returned
object) so registration responds only with CREATED, message and user data
(including otp_code/expires_at persisted) and require a separate verify/login
flow to obtain tokens; modify the createNewUser method in auth.service.ts
(references: createNewUser, this.jwtService.sign, and the returned access_token)
to return no token and adjust callers/tests that expect access_token
accordingly.
- Around line 105-109: The generateOtp function uses Math.random(), which is not
cryptographically secure; replace its logic to use crypto.randomInt from
node:crypto to produce a secure integer in the range [0, 10 ** OTP_LENGTH) and
then convert to string and padStart(OTP_LENGTH, '0'); ensure you import or
require randomInt from 'node:crypto' and keep the OTP_LENGTH constant and pad
behavior the same.
In `@src/modules/auth/entities/user-session.entity.ts`:
- Around line 22-23: The is_revoked column on the UserSession entity should not
be nullable and must have a DB-level default of false; update the Column
decorator for is_revoked in user-session.entity.ts to remove nullable: true and
add default: false (set the TypeScript property to boolean instead of boolean |
null), and then create/apply a migration to alter the existing DB column to set
NOT NULL with default false so existing rows get a concrete value.
In `@src/modules/auth/tests/auth.service.spec.ts`:
- Around line 79-82: The test currently only checks that
service.createNewUser(dto) rejects with CustomHttpException, which is too broad;
modify the assertion to verify the exception is the specific conflict error by
checking its HTTP status and message (e.g., status === HttpStatus.CONFLICT or
matching the conflict status code and the duplicate-email message). Locate the
test using userRepositoryMock.findOne.mockResolvedValueOnce({ id: 'existing' }),
service.createNewUser, and CustomHttpException, then change the
expect(...).rejects to inspect the thrown error object (status and message)
rather than only its class so the test fails for unrelated exceptions.
- Around line 73-76: The test currently asserts persistence of OTP fields on the
User (checking created.otp_code and created.expires_at) which couples creation
to verification state; instead update the spec for createNewUser: stop asserting
OTP fields on the persisted user or, better, add an explicit leak-prevention
assertion that the API response (result.data.user) does not include otp_code or
expires_at while still verifying auth_provider, and keep the existing inspection
of userRepositoryMock.create.mock.calls[0][0] only for fields that must be
stored; locate references to createNewUser, userRepositoryMock.create, and
result.data.user to make the change.
In `@src/modules/funnels/entities/funnel-stage.entity.ts`:
- Around line 18-22: The entity properties title and description are declared as
non-nullable strings but their columns allow NULL; update the FunnelStage entity
by changing the types of the title and description properties to accept null
(e.g., string | null) so TypeORM hydration of NULL values is type-safe, and
adjust any usages that assume non-null if necessary (look for references to
title and description in methods or consumers).
In `@src/modules/funnels/entities/funnel-task.entity.ts`:
- Around line 14-21: The entity declares nullable DB columns but uses
non-nullable TS types; update the FunnelTask entity fields title, description,
and task_order to include null in their types (e.g., title: string | null,
description: string | null, task_order: number | null) to match Column({
nullable: true }), following the same pattern used by checked_at (Date | null)
and other entities like User.
In `@src/modules/notifications/entities/admin-notification.entity.ts`:
- Around line 16-17: The AdminNotification entity is unused and its type column
lacks validation; either remove the AdminNotification entity and its migration
if you don't plan to use it, or enforce valid values by changing the type
property to use the existing NotificationType enum (or create a new
AdminNotificationType enum) and update the `@Column` to restrict values
accordingly (ensure the entity class name AdminNotification and property name
type are updated to use the enum and add any necessary validation decorators
before saving).
In `@src/modules/notifications/entities/notifications.entity.ts`:
- Around line 17-38: The entity declares DB columns as nullable but the
TypeScript fields are non-nullable; update the Notifications entity so the
fields whose Column decorators set nullable: true are typed to allow null (e.g.,
change the type field to NotificationType | null and title and body to string |
null) so TS strict-null checks align with the DB; keep existing decorators and
defaults unchanged and only adjust the field types for type, title, and body.
- Around line 5-42: Add database indexes to the Notification entity to match
query patterns used by NotificationsService: add an index on user_id plus
created_at (for listForUser ordering) and an index on user_id plus is_read (for
unread count). Update the Notification class to include `@Index` decorators
referencing the fields (user_id, created_at) and (user_id, is_read), and add
matching CREATE INDEX statements into the migration
1778257249178-InitSchemaFromErd.ts so the schema and migration stay in sync with
synchronize: false.
In `@src/modules/notifications/notifications.controller.ts`:
- Around line 20-24: Validate the incoming page and limit query params in the
controller before calling notificationsService.listForUser: parse them as
integers (e.g., parseInt), ensure they are whole numbers, enforce sensible
bounds (page >= 1, limit between 1 and a max like 100), and throw a
BadRequestException when validation fails; then call
this.notificationsService.listForUser(user.id, pageInt, limitInt) with the
validated integers. Reference: the handler using request['user'] and the
notificationsService.listForUser method.
In `@src/modules/notifications/notifications.module.ts`:
- Around line 9-14: The module currently exports TypeOrmModule which exposes all
repositories; change the export to expose the NotificationsService instead:
update the `@Module` exports array to export NotificationsService (ensure
NotificationsService is listed in providers) so downstream modules import
NotificationsModule to use NotificationsService without leaking
Notification/AdminNotification/NotificationPreference repositories.
In `@src/modules/notifications/notifications.service.ts`:
- Around line 15-22: The listForUser method allows page/limit that can make skip
negative or take zero; normalize and validate inputs at the top of listForUser:
coerce page and limit to integers (e.g., parseInt), default page to 1 and limit
to a sensible min (>=1) using Math.max, then compute skip = (page - 1) * limit
and take = limit so notificationRepository.findAndCount and
notificationRepository.count always receive non-negative skip and positive take;
also use the normalized limit when computing total_pages or any division to
avoid divide-by-zero or incorrect paging.
In `@src/modules/strategies/entities/strategy-document.entity.ts`:
- Around line 13-19: The `@ManyToOne` relations for Strategy and UploadedDocument
are missing nullable: false in ORM metadata; update the decorators on the
strategy and document relations (the `@ManyToOne`(() => Strategy, ...) for
property strategy and `@ManyToOne`(() => UploadedDocument, ...) for property
document in StrategyDocument entity) to include { nullable: false, onDelete:
'CASCADE' } so the TypeORM relation metadata matches the NOT NULL DB constraint
and prevents accidental null relation inserts.
In `@src/modules/strategies/entities/token-usage.entity.ts`:
- Around line 22-23: The token_used property on the TokenUsage entity is
declared nullable in the `@Column` decorator but typed as a non-null number;
change the token_used declaration to allow null (e.g., token_used: number |
null) in src/modules/strategies/entities/token-usage.entity.ts and adjust any
code that reads/writes token_used (constructors, DTO mappings, consumers) to
handle null safely (use null-coalescing or checks) so runtime null dereferences
are avoided.
In `@src/modules/strategies/strategies.module.ts`:
- Around line 1-11: The StrategiesModule currently registers TokenUsage
alongside Strategy and StrategyDocument; move TokenUsage into its own bounded
context by creating a TokenUsageModule that imports and exports
TypeOrmModule.forFeature([TokenUsage]) and any TokenUsage services/entities,
then update StrategiesModule to only register Strategy and StrategyDocument
(remove TokenUsage from TypeOrmModule.forFeature in StrategiesModule) and have
modules that need token usage import TokenUsageModule instead.
In `@src/modules/subscriptions/entities/subscription.entity.ts`:
- Around line 20-21: The started_at column is declared nullable but typed as
Date, which can hide null returns from TypeORM; update the Subscription entity's
started_at property to reflect nullability (e.g., change its type to Date |
null) so callers must handle the null case and TypeScript accurately represents
the DB contract; keep the `@Column`({ type: 'timestamp', nullable: true })
decorator as-is and only adjust the property type for the started_at field.
In `@src/modules/uploaded-documents/entities/uploaded-document.entity.ts`:
- Around line 16-26: Update the TypeScript property types on the
UploadedDocument entity so nullable DB columns are typed as nullable in TS:
change file_name to string | null, file_size_kb to number | null, file_type to
string | null, and storage_path to string | null on the UploadedDocument entity
(the class with properties file_name, file_size_kb, file_type, storage_path) so
TypeORM-hydrated null values are correctly represented.
In `@src/modules/user/entities/user.entity.ts`:
- Around line 34-38: Make both OTP fields nullable and namespace the expiry
field: change the entity properties so otp_code is nullable (nullable: true) and
rename expires_at to otp_expires_at (also nullable: true) on the User entity
(symbols: otp_code, expires_at -> otp_expires_at). Update all references/usages
of expires_at to use otp_expires_at (DTOs, services, tests) and ensure the
generated migration runs in lockstep to (a) rename the DB column expires_at ->
otp_expires_at and (b) alter both OTP columns to allow NULL so OAuth users can
have empty OTPs and OTPs can be cleared after use.
In `@src/modules/user/tests/user.service.spec.ts`:
- Around line 60-71: Add a test for the "not found" branch of softDeleteUser:
mock repositoryMock.findOne (used by getUserById) to resolve to null, call
service.softDeleteUser with any caller/id mismatch irrelevant, and assert it
rejects with a CustomHttpException and that the thrown error has status 404;
place this new it block alongside the existing tests in the
describe('softDeleteUser') group so softDeleteUser, repositoryMock.findOne, and
CustomHttpException are exercised for the not-found path.
- Around line 49-51: Replace the loose assertion on the deactivateUser response
by asserting the exact success message and status code: in the test calling
service.deactivateUser('user-1'), assert result.message ===
SYS_MSG.ACCOUNT_DEACTIVATED_SUCCESSFULLY and assert result.status_code equals
the expected success code (e.g., 200 or the constant used in your app), while
keeping the existing check that repositoryMock.save was called with an
objectContaining { is_active: false } to ensure state change.
In `@src/modules/user/user.controller.ts`:
- Around line 17-19: Replace the ad-hoc user extraction in the deactivate route
with a typed parameter decorator: implement a CurrentUser param decorator using
createParamDecorator that returns ctx.switchToHttp().getRequest().user as { id:
string }, then update the controller method signature from async
deactivate(`@Req`() request: Request) to async deactivate(`@CurrentUser`() user: {
id: string }) and call this.userService.deactivateUser(user.id); remove the
string-key access and manual cast in the method.
- Around line 44-51: The route is self-only but currently requires an :id URL
param; change the controller to a self-route by replacing `@Delete`(':id') with
`@Delete`('me'), remove the `@Param`('id', ParseUUIDPipe) id parameter and instead
obtain the target id from the authenticated requester (request['user'].id), then
call this.userService.softDeleteUser(requester.id, requester.id); also update
the `@ApiOperation/`@ApiResponse summary to reflect "Soft delete the authenticated
user account (me)". This keeps the existing service call (softDeleteUser) but
uses requester.id for the target and requester arguments.
- Around line 22-42: The getById handler currently returns sensitive flags
(is_active, is_verified, created_at) to any authenticated caller; update the
getById method to return a minimal public projection (id, full_name, avatar_url)
for non-owners and non-admins and only return the full projection when the
requester is the same user or has admin privileges. Implement this by inspecting
the current principal (e.g., request.user or a decorator like `@Req`() /
`@CurrentUser`()) inside getById, comparing request.user.id to the id param and/or
checking an isAdmin role flag (or RolesGuard), then branch the response: call
userService.getUserById as now but redact sensitive fields for public callers
(or call a new userService.getPublicById helper) and keep the existing full data
for owner/admin; ensure any auth/role checks use existing guards/decorators
rather than exposing full state to every authenticated request.
In `@src/modules/user/user.service.ts`:
- Around line 23-25: The getUserByEmail method currently calls
this.userRepository.findOne which silently excludes soft-deleted rows; update
getUserByEmail to accept an optional flag (e.g., includeSoftDeleted?: boolean or
options object) and pass that through to the repository call (use the
withDeleted/withDeleted option or query builder .withDeleted() when
includeSoftDeleted is true) so callers can opt in to seeing soft-deleted users;
keep the default behavior unchanged for login flows.
- Around line 29-31: Replace the generic error constant used when a user is
already deactivated: instead of throwing new
CustomHttpException(SYS_MSG.BAD_REQUEST, HttpStatus.BAD_REQUEST) in the
is_active check (where user.is_active is false), throw a specific message
constant such as SYS_MSG.ACCOUNT_ALREADY_DEACTIVATED with the same
HttpStatus.BAD_REQUEST; add the new ACCOUNT_ALREADY_DEACTIVATED entry to the
SYS_MSG collection if missing (mirroring the style of
ACCOUNT_DEACTIVATED_SUCCESSFULLY) so callers can distinguish “already
deactivated” from other bad requests.
- Around line 40-43: In softDeleteUser, replace the 401 Unauthorized response
with a 403 Forbidden: when userId !== requesterId throw the CustomHttpException
using HttpStatus.FORBIDDEN (and, if available, swap SYS_MSG.UNAUTHORISED_TOKEN
for an appropriate forbidden message constant such as SYS_MSG.FORBIDDEN or
SYS_MSG.FORBIDDEN_ACTION); update the throw site to use HttpStatus.FORBIDDEN
while preserving the existing exception type CustomHttpException.
---
Outside diff comments:
In `@src/app.module.ts`:
- Around line 116-122: The ServeStaticModule is using join(__dirname, 'uploads')
which resolves to dist/uploads at runtime and won't serve files written to the
project-root uploads directory; update the rootPath in ServeStaticModule.forRoot
(the call shown using rootPath: join(__dirname, 'uploads')) to point to the
project root, e.g. replace with join(process.cwd(), 'uploads') or
path.resolve(process.cwd(), 'uploads') so the module serves files from the
actual project-root uploads directory while preserving serveRoot: '/uploads' and
serveStaticOptions.
In `@src/modules/auth/dto/create-user.dto.ts`:
- Around line 36-45: Add a MaxLength(64) constraint to the password property in
the CreateUser DTO: import MaxLength from class-validator and annotate the
password field (the property named "password" in the DTO with existing
`@MinLength/`@IsStrongPassword decorators) with `@MaxLength`(64) so bcrypt
truncation collisions and excessive-length inputs are prevented; ensure the
import is added and any validation tests or error messages updated accordingly.
In `@src/modules/email/email.module.ts`:
- Around line 21-44: The MailerModule.forRootAsync registration is duplicated
between EmailModule and AppModule causing one to overwrite the other; remove the
MailerModule.forRootAsync call from EmailModule and rely on the single global
registration in AppModule, then reconcile the template directory there (choose
either '/src/modules/email/hng-templates' or '/src/modules/email/templates') and
update the MailerModule configuration in AppModule (transport, defaults,
template.dir, HandlebarsAdapter usage) so all code using EmailModule relies on
the single global MailerModule; also remove any MailerModule imports or
forRoot/forRootAsync usage from EmailModule and ensure EmailModule injects
MailerService (not re-register) if it needs to send mail.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c59125c9-f59e-4bcd-baee-2c2d7a35d5c7
📒 Files selected for processing (108)
package.jsonsrc/app.module.tssrc/database/data-source.tssrc/database/migrations/1778257249178-InitSchemaFromErd.tssrc/modules/auth/auth.controller.tssrc/modules/auth/auth.module.tssrc/modules/auth/auth.service.tssrc/modules/auth/dto/auth-response.dto.tssrc/modules/auth/dto/create-user.dto.tssrc/modules/auth/dto/enable-2fa.dto.tssrc/modules/auth/dto/forgot-password.dto.tssrc/modules/auth/dto/generic-reponse.dto.tssrc/modules/auth/dto/google-auth.dto.tssrc/modules/auth/dto/login-error-dto.tssrc/modules/auth/dto/login-response.dto.tssrc/modules/auth/dto/request-signin-token.dto.tssrc/modules/auth/dto/update-user-password.dto.tssrc/modules/auth/dto/updatePasswordDto.tssrc/modules/auth/dto/verify-2fa.dto.tssrc/modules/auth/entities/auth-metadata.entity.tssrc/modules/auth/entities/user-session.entity.tssrc/modules/auth/interfaces/GoogleAuthPayloadInterface.tssrc/modules/auth/tests/auth.service.spec.tssrc/modules/auth/tests/login.e2e-spec.tssrc/modules/email/email.module.tssrc/modules/funnels/entities/funnel-stage.entity.tssrc/modules/funnels/entities/funnel-task.entity.tssrc/modules/funnels/funnels.module.tssrc/modules/notifications/dtos/create-notification-all-users-res.dto.tssrc/modules/notifications/dtos/create-notification-error.dto.tssrc/modules/notifications/dtos/create-notification-props.dto.tssrc/modules/notifications/dtos/create-notification-response.dto.tssrc/modules/notifications/dtos/create-notification.dto.tssrc/modules/notifications/dtos/create-notifiction-all-users.dto.tssrc/modules/notifications/dtos/get-notification-response-dto.tssrc/modules/notifications/dtos/mark-all-notifications-as-read-error.dto.tssrc/modules/notifications/dtos/mark-all-notifications-as-read.dto.tssrc/modules/notifications/dtos/mark-notification-as-read-error.dto.tssrc/modules/notifications/dtos/mark-notification-as-read.dto.tssrc/modules/notifications/dtos/notification-data.dto.tssrc/modules/notifications/dtos/notification-prop.dto.tssrc/modules/notifications/dtos/notification-response.dto.tssrc/modules/notifications/dtos/notification.dto.tssrc/modules/notifications/dtos/unread-notifications-response.dto.tssrc/modules/notifications/dtos/unread-response-error.dto.tssrc/modules/notifications/entities/admin-notification.entity.tssrc/modules/notifications/entities/notification-preference.entity.tssrc/modules/notifications/entities/notifications.entity.tssrc/modules/notifications/enums/notification-type.enum.tssrc/modules/notifications/notifications.controller.tssrc/modules/notifications/notifications.module.tssrc/modules/notifications/notifications.service.tssrc/modules/notifications/tests/mocks/notification-repo.mock.tssrc/modules/notifications/tests/mocks/user-repo.mock.tssrc/modules/notifications/tests/notifications.service.spec.tssrc/modules/notifications/tests/user-service.mock.tssrc/modules/otp/dto/otp.dto.tssrc/modules/otp/entities/otp.entity.tssrc/modules/otp/otp.module.tssrc/modules/otp/otp.service.spec.tssrc/modules/otp/otp.service.tssrc/modules/profile/dto/file.validator.tssrc/modules/profile/dto/update-profile.dto.tssrc/modules/profile/dto/upload-profile-pic.dto.tssrc/modules/profile/entities/profile.entity.tssrc/modules/profile/mocks/mockUser.tssrc/modules/profile/mocks/profileMock.tssrc/modules/profile/profile.controller.tssrc/modules/profile/profile.module.tssrc/modules/profile/profile.service.tssrc/modules/profile/tests/profile.service.spec.tssrc/modules/strategies/entities/strategy-document.entity.tssrc/modules/strategies/entities/strategy.entity.tssrc/modules/strategies/entities/token-usage.entity.tssrc/modules/strategies/strategies.module.tssrc/modules/subscriptions/entities/subscription.entity.tssrc/modules/subscriptions/subscriptions.module.tssrc/modules/uploaded-documents/entities/uploaded-document.entity.tssrc/modules/uploaded-documents/uploaded-documents.module.tssrc/modules/user/dto/deactivate-account.dto.tssrc/modules/user/dto/get-user-data-by-id-response.dto.tssrc/modules/user/dto/get-user-stats-response.dto.tssrc/modules/user/dto/reactivate-account.dto.tssrc/modules/user/dto/update-user-dto.tssrc/modules/user/dto/update-user-response.dto.tssrc/modules/user/dto/update-user-status-response.dto.tssrc/modules/user/dto/update-user-status.dto.tssrc/modules/user/dto/user-data-export.dto.tssrc/modules/user/dto/user-response.dto.tssrc/modules/user/entities/user-role.entity.tssrc/modules/user/entities/user.entity.tssrc/modules/user/enums/user-role.enum.tssrc/modules/user/interfaces/UserInterface.tssrc/modules/user/interfaces/user-payload.interface.tssrc/modules/user/options/CreateNewUserOptions.tssrc/modules/user/options/UpdateUserRecordOption.tssrc/modules/user/options/UserIdentifierOptions.tssrc/modules/user/tests/mocks/user.mock.tssrc/modules/user/tests/reactivate-user.service.spec.tssrc/modules/user/tests/user.service.spec.tssrc/modules/user/user.controller.tssrc/modules/user/user.module.tssrc/modules/user/user.service.tssrc/modules/waitlist/entities/waitlist.entity.tssrc/modules/waitlist/waitlist.module.tssrc/modules/weekly-logs/entities/weekly-log.entity.tssrc/modules/weekly-logs/weekly-logs.module.tssrc/shared/constants/SystemMessages.ts
💤 Files with no reviewable changes (65)
- src/modules/user/interfaces/user-payload.interface.ts
- src/modules/notifications/dtos/mark-notification-as-read.dto.ts
- src/modules/notifications/dtos/notification-response.dto.ts
- src/modules/notifications/dtos/create-notification-all-users-res.dto.ts
- src/modules/notifications/dtos/mark-notification-as-read-error.dto.ts
- src/modules/user/dto/get-user-stats-response.dto.ts
- src/modules/notifications/dtos/create-notification-error.dto.ts
- src/modules/auth/dto/enable-2fa.dto.ts
- src/modules/notifications/dtos/notification-prop.dto.ts
- src/modules/auth/interfaces/GoogleAuthPayloadInterface.ts
- src/modules/user/dto/update-user-dto.ts
- src/modules/auth/dto/verify-2fa.dto.ts
- src/modules/user/tests/mocks/user.mock.ts
- src/modules/user/dto/update-user-status.dto.ts
- src/modules/notifications/dtos/mark-all-notifications-as-read-error.dto.ts
- src/modules/user/dto/update-user-response.dto.ts
- src/modules/notifications/dtos/notification-data.dto.ts
- src/modules/user/options/UserIdentifierOptions.ts
- src/modules/user/options/CreateNewUserOptions.ts
- src/modules/notifications/tests/user-service.mock.ts
- src/modules/notifications/dtos/create-notification-response.dto.ts
- src/modules/notifications/dtos/create-notifiction-all-users.dto.ts
- src/modules/notifications/dtos/create-notification-props.dto.ts
- src/modules/otp/entities/otp.entity.ts
- src/modules/auth/dto/request-signin-token.dto.ts
- src/modules/user/options/UpdateUserRecordOption.ts
- src/modules/user/dto/update-user-status-response.dto.ts
- src/modules/user/dto/deactivate-account.dto.ts
- src/modules/auth/dto/updatePasswordDto.ts
- src/modules/auth/dto/google-auth.dto.ts
- src/modules/profile/dto/update-profile.dto.ts
- src/modules/notifications/dtos/unread-notifications-response.dto.ts
- src/modules/profile/profile.module.ts
- src/modules/auth/dto/forgot-password.dto.ts
- src/modules/auth/dto/login-error-dto.ts
- src/modules/notifications/dtos/create-notification.dto.ts
- src/modules/user/dto/get-user-data-by-id-response.dto.ts
- src/modules/user/dto/user-data-export.dto.ts
- src/modules/notifications/dtos/get-notification-response-dto.ts
- src/modules/notifications/tests/mocks/user-repo.mock.ts
- src/modules/notifications/dtos/mark-all-notifications-as-read.dto.ts
- src/modules/auth/dto/update-user-password.dto.ts
- src/modules/otp/otp.service.spec.ts
- src/modules/user/tests/reactivate-user.service.spec.ts
- src/modules/profile/dto/file.validator.ts
- src/modules/auth/tests/login.e2e-spec.ts
- src/modules/notifications/dtos/unread-response-error.dto.ts
- src/modules/profile/dto/upload-profile-pic.dto.ts
- src/modules/user/dto/user-response.dto.ts
- src/modules/notifications/tests/mocks/notification-repo.mock.ts
- src/modules/profile/profile.controller.ts
- src/modules/auth/dto/generic-reponse.dto.ts
- src/modules/auth/dto/login-response.dto.ts
- src/modules/profile/entities/profile.entity.ts
- src/modules/user/interfaces/UserInterface.ts
- src/modules/profile/profile.service.ts
- src/modules/otp/dto/otp.dto.ts
- src/modules/profile/mocks/mockUser.ts
- src/modules/user/dto/reactivate-account.dto.ts
- src/modules/profile/mocks/profileMock.ts
- src/modules/notifications/dtos/notification.dto.ts
- src/modules/otp/otp.module.ts
- src/modules/otp/otp.service.ts
- src/modules/profile/tests/profile.service.spec.ts
- src/modules/auth/dto/auth-response.dto.ts
ibraheembello
requested review from
BigLens,
Homoakin619,
JohnUghiovhe,
LazyCoder4542,
ObiFaith,
TheCodeGhinux and
elijaharhinful
May 8, 2026 17:57
Homoakin619
reviewed
May 8, 2026
Homoakin619
reviewed
May 8, 2026
- user_sessions.is_revoked: boolean NOT NULL DEFAULT false (was nullable) - funnel_stages.description: typed as string | null to match nullable column Per review feedback on PR hngprojects#4.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/modules/funnels/entities/funnel-stage.entity.ts (1)
18-19:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake
titlenullable in the TS model too.Line 19 is still typed as
stringeven though the column allowsNULL, so TypeORM can hydrate an entity that violates its own type contract.descriptionwas fixed already;titleneeds the same treatment.Suggested fix
`@Column`({ type: 'varchar', length: 100, nullable: true }) - title: string; + title: string | null;Please also re-scan
FunnelStageconsumers for any non-null assumptions aroundtitle.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/modules/funnels/entities/funnel-stage.entity.ts` around lines 18 - 19, Update the FunnelStage entity so the title property matches the DB nullability: change the type on the title field in class FunnelStage (the property annotated with `@Column`({ type: 'varchar', length: 100, nullable: true })) from string to string | null; then search for and update any consumers of FunnelStage.title (constructors, DTO mappers, templates, or business logic) that assume a non-null string to handle nulls safely.src/database/migrations/1778257249178-InitSchemaFromErd.ts (1)
32-51:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd indexes for the remaining foreign keys before shipping the initial schema.
PostgreSQL will not create them for you, so parent deletes and common joins will fall back to table scans as these tables grow. A few FKs are already covered by existing composite/unique indexes, but the remaining ones here are not — for example
strategies.user_id,weekly_logs.strategy_id,weekly_logs.funnel_stage_id,user_roles.user_id,subscriptions.user_id,uploaded_documents.user_id,token_usage.user_id,token_usage.strategy_id,strategy_documents.document_id,notifications.user_id,admin_notifications.admin_id,admin_notifications.related_user_id,funnel_tasks.funnel_stage_id,user_sessions.user_id,auth_metadata.user_id, andrequest.apiHealthId.Example of the missing index additions
+ await queryRunner.query(`CREATE INDEX "IDX_strategies_user_id" ON "strategies" ("user_id")`); + await queryRunner.query(`CREATE INDEX "IDX_weekly_logs_strategy_id" ON "weekly_logs" ("strategy_id")`); + await queryRunner.query(`CREATE INDEX "IDX_weekly_logs_funnel_stage_id" ON "weekly_logs" ("funnel_stage_id")`); + await queryRunner.query(`CREATE INDEX "IDX_user_roles_user_id" ON "user_roles" ("user_id")`); + await queryRunner.query(`CREATE INDEX "IDX_subscriptions_user_id" ON "subscriptions" ("user_id")`); + await queryRunner.query(`CREATE INDEX "IDX_uploaded_documents_user_id" ON "uploaded_documents" ("user_id")`); + await queryRunner.query(`CREATE INDEX "IDX_token_usage_user_id" ON "token_usage" ("user_id")`); + await queryRunner.query(`CREATE INDEX "IDX_token_usage_strategy_id" ON "token_usage" ("strategy_id")`); + await queryRunner.query(`CREATE INDEX "IDX_strategy_documents_document_id" ON "strategy_documents" ("document_id")`); + await queryRunner.query(`CREATE INDEX "IDX_notifications_user_id" ON "notifications" ("user_id")`); + await queryRunner.query(`CREATE INDEX "IDX_admin_notifications_admin_id" ON "admin_notifications" ("admin_id")`); + await queryRunner.query(`CREATE INDEX "IDX_admin_notifications_related_user_id" ON "admin_notifications" ("related_user_id")`); + await queryRunner.query(`CREATE INDEX "IDX_funnel_tasks_funnel_stage_id" ON "funnel_tasks" ("funnel_stage_id")`); + await queryRunner.query(`CREATE INDEX "IDX_user_sessions_user_id" ON "user_sessions" ("user_id")`); + await queryRunner.query(`CREATE INDEX "IDX_auth_metadata_user_id" ON "auth_metadata" ("user_id")`); + await queryRunner.query(`CREATE INDEX "IDX_request_api_health_id" ON "request" ("apiHealthId")`);Please mirror the corresponding
DROP INDEXstatements indown().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/database/migrations/1778257249178-InitSchemaFromErd.ts` around lines 32 - 51, The migration's up() adds many foreign keys but doesn't create supporting indexes; update the migration (methods up() and down()) to CREATE INDEX for each FK column listed (e.g., strategies.user_id, weekly_logs.strategy_id, weekly_logs.funnel_stage_id, user_roles.user_id, subscriptions.user_id, uploaded_documents.user_id, token_usage.user_id, token_usage.strategy_id, strategy_documents.document_id, notifications.user_id, admin_notifications.admin_id, admin_notifications.related_user_id, funnel_tasks.funnel_stage_id, user_sessions.user_id, auth_metadata.user_id, request.apiHealthId) using explicit index names (e.g., IDX_<table>_<column>) and add corresponding DROP INDEX statements in down() to mirror them; ensure index creation uses the same column names and index identifiers as referenced by the ALTER TABLE FK statements so rollbacks remove the correct indexes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/database/migrations/1778257249178-InitSchemaFromErd.ts`:
- Around line 32-51: The migration's up() adds many foreign keys but doesn't
create supporting indexes; update the migration (methods up() and down()) to
CREATE INDEX for each FK column listed (e.g., strategies.user_id,
weekly_logs.strategy_id, weekly_logs.funnel_stage_id, user_roles.user_id,
subscriptions.user_id, uploaded_documents.user_id, token_usage.user_id,
token_usage.strategy_id, strategy_documents.document_id, notifications.user_id,
admin_notifications.admin_id, admin_notifications.related_user_id,
funnel_tasks.funnel_stage_id, user_sessions.user_id, auth_metadata.user_id,
request.apiHealthId) using explicit index names (e.g., IDX_<table>_<column>) and
add corresponding DROP INDEX statements in down() to mirror them; ensure index
creation uses the same column names and index identifiers as referenced by the
ALTER TABLE FK statements so rollbacks remove the correct indexes.
In `@src/modules/funnels/entities/funnel-stage.entity.ts`:
- Around line 18-19: Update the FunnelStage entity so the title property matches
the DB nullability: change the type on the title field in class FunnelStage (the
property annotated with `@Column`({ type: 'varchar', length: 100, nullable: true
})) from string to string | null; then search for and update any consumers of
FunnelStage.title (constructors, DTO mappers, templates, or business logic) that
assume a non-null string to handle nulls safely.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f8065746-a521-4caa-9f99-36fd33c9aa15
📒 Files selected for processing (3)
src/database/migrations/1778257249178-InitSchemaFromErd.tssrc/modules/auth/entities/user-session.entity.tssrc/modules/funnels/entities/funnel-stage.entity.ts
Homoakin619
approved these changes
May 8, 2026
BigLens
approved these changes
May 8, 2026
This was referenced May 9, 2026
Merged
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
Pull Request
Description
Sets up the canonical database schema for FlowBrand exactly as defined by the ERD shared by the team (https://dbdiagram.io/d/FlowBrand-ERD-69fb4c9b7a923b947233c29d). Adds 15 ERD tables, restructures `users` + `notifications` to match, and removes legacy modules whose tables are not on the ERD. Project compiles, lints, all tests pass, and the server boots cleanly on the new schema.
Related Issue
No ticket — initial schema scaffold so feature tickets can land against a stable DB.
Type of Change
What's added (15 ERD tables)
What's removed (and why — rationale for the team)
The ERD does not include `profile` or `otp` tables. The ERD merges OTP fields (`otp_code`, `expires_at`) directly into `users`, and has no `profile` concept. To keep the schema strictly matching the team's ERD and to avoid leaving dead code that references removed columns:
All of this is preserved in git history (commit `c77fb01`'s parent is `b38f989`) — easily restorable if the team decides any of it should come back.
The remaining auth/user surface is intentionally minimal: `POST /auth/register`, `POST /auth/login`, `POST /auth/change-password`, `GET /users/:id`, `PATCH /users/deactivate`, `DELETE /users/:id`, plus a basic notifications list/markRead. Future tickets re-introduce richer flows against the new schema.
Migration
`src/database/migrations/1778257249178-InitSchemaFromErd.ts` — creates all 15 ERD tables + the existing `api_health` / `request` operational tables (kept; not on ERD but unrelated to product domain). Generated via `npm run migration:generate`, then verified by:
```
npm run migration:run # ✅ 19 tables created
npm run migration:revert # ✅ all dropped, only `migrations` table left
npm run migration:run # ✅ re-applied cleanly
```
The migration also enables the `uuid-ossp` extension on first run.
Other notes
How Has This Been Tested?
`npm run lint && npm run build && npm test` — clean 3 consecutive runs:
Server starts cleanly on the new schema (`npm run start` → `Nest application successfully started` on `http://localhost:3000/api/v1\`). All ERD modules' `TypeOrmModule` dependencies resolve, all routes mapped.
Test Evidence
Screenshots (if applicable)
N/A — no UI changes.
Documentation Screenshots (if applicable)
N/A — no public-facing docs in this PR. New entities will be documented per-ticket as services/controllers ship.
Checklist
Additional Notes
This is a schema-only PR by design — feature tickets layer on top. Aggressively trims dependent code to keep build/lint/tests green per the request. Everything removed lives in git history.