Task#00000 Referral Tracking implementation#741
Open
Tusharmahajan12 wants to merge 4 commits into
Open
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughAdds a ReferralsModule: DTOs, enums, TypeORM entities, slug utilities, ReferralsService (CRUD, import, bulk, reporting), controller endpoints (including report), Elasticsearch referLink support, and wiring into app, forms, and Postgres user creation to record user attribution. ChangesReferral Management System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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 introduces a comprehensive referral management system, including modules for tracking referral entities, slug history, and user attribution. Key features include automated slug generation, CSV import capabilities, and integration with Elasticsearch for filtering users by referral links. Feedback focused on improving data consistency through transactions during slug updates, adopting a more robust CSV parsing strategy, and refining code quality by removing unnecessary type casts and correcting misleading error messages.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (9)
src/forms/services/form-submission.service.ts (1)
99-99: 💤 Low valueConsider adding a trailing comma for consistency.
Adding a trailing comma after the last parameter improves diff cleanliness when parameters are added later and maintains consistency with common TypeScript/JavaScript style guides.
✨ Suggested change
- private readonly referralsService: ReferralsService + private readonly referralsService: ReferralsService,🤖 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/forms/services/form-submission.service.ts` at line 99, The constructor parameter list ending with "private readonly referralsService: ReferralsService" should include a trailing comma for style consistency; update the constructor (the parameter declaration containing referralsService) to add a comma after that parameter so future diffs remain cleaner and align with TypeScript/JS style guides.src/referrals/entities/referral-slug-history.entity.ts (3)
17-19: ⚡ Quick winRemove redundant unique constraint.
The
@Index({ unique: true })decorator already creates a unique constraint, so specifyingunique: truein the@Columndecorator is redundant.♻️ Proposed fix
`@Index`({ unique: true }) - `@Column`({ type: 'varchar', length: 100, unique: true }) + `@Column`({ type: 'varchar', length: 100 }) oldSlug: string;As per coding guidelines, the code should adhere to best practices for TypeORM.
🤖 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/referrals/entities/referral-slug-history.entity.ts` around lines 17 - 19, Remove the redundant unique constraint on the oldSlug column: keep the `@Index`({ unique: true }) decorator and remove the unique: true option from the `@Column` decorator on the oldSlug property in the ReferralSlugHistory entity (look for oldSlug, `@Index` and `@Column` in the file) so the unique constraint is defined only once via the index.
27-28: ⚡ Quick winRemove redundant default function from timestamp column.
The
default: () => 'now()'is redundant with@CreateDateColumn, which automatically handles the timestamp default.♻️ Proposed fix
- `@CreateDateColumn`({ type: 'timestamp', default: () => 'now()' }) + `@CreateDateColumn`({ type: 'timestamp' }) changedAt: Date;As per coding guidelines, the code should adhere to best practices for TypeORM.
🤖 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/referrals/entities/referral-slug-history.entity.ts` around lines 27 - 28, The `@CreateDateColumn` decorator on the changedAt property currently includes a redundant default: () => 'now()'; remove that default option so the decorator is simply `@CreateDateColumn`({ type: 'timestamp' }) (or just `@CreateDateColumn`()) and leave the changedAt: Date; property intact to let TypeORM manage the timestamp automatically.
14-15: ⚡ Quick winConsider adding foreign key relationship for referralEntityId.
The
referralEntityIdcolumn lacks a@ManyToOnerelationship toReferralEntity, which would enable TypeORM to manage referential integrity and allow easier querying with joins.🔗 Proposed enhancement
+import { ManyToOne, JoinColumn } from 'typeorm'; +import { ReferralEntity } from './referral-entity.entity'; + `@ManyToOne`(() => ReferralEntity, { onDelete: 'CASCADE' }) + `@JoinColumn`({ name: 'referralEntityId' }) + referralEntity: ReferralEntity; + `@Column`({ type: 'uuid' }) referralEntityId: string;As per coding guidelines, the code should adhere to best practices for TypeORM and database integrity.
🤖 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/referrals/entities/referral-slug-history.entity.ts` around lines 14 - 15, The referralEntityId column in ReferralSlugHistory should be modeled as a foreign key relation to ReferralEntity: add a `@ManyToOne`(() => ReferralEntity, { nullable: false }) relation property (e.g., referralEntity) on the ReferralSlugHistory class and annotate it with `@JoinColumn`({ name: 'referralEntityId' }), keeping or adding the referralEntityId `@Column`({ type: 'uuid' }) if you want the explicit FK field; this lets TypeORM enforce referential integrity and enables join queries using the ReferralSlugHistory.referralEntity and the ReferralEntity entity.src/referrals/entities/referral-entity.entity.ts (3)
59-63: ⚡ Quick winRemove redundant default functions from timestamp columns.
The
default: () => 'now()'is redundant with@CreateDateColumnand@UpdateDateColumn, which automatically handle timestamps.♻️ Proposed fix
- `@CreateDateColumn`({ type: 'timestamp', default: () => 'now()' }) + `@CreateDateColumn`({ type: 'timestamp' }) createdAt: Date; - `@UpdateDateColumn`({ type: 'timestamp', default: () => 'now()' }) + `@UpdateDateColumn`({ type: 'timestamp' }) updatedAt: Date;As per coding guidelines, the code should adhere to best practices for TypeORM.
🤖 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/referrals/entities/referral-entity.entity.ts` around lines 59 - 63, Remove the redundant default functions on the timestamp columns: the `@CreateDateColumn` and `@UpdateDateColumn` decorators already manage timestamps, so update the referral entity by removing the "default: () => 'now()'" option from the `@CreateDateColumn` and `@UpdateDateColumn` decorators for createdAt and updatedAt (keep the decorators and the createdAt/updatedAt fields intact).
28-30: ⚡ Quick winRemove redundant unique constraint.
The
@Index({ unique: true })decorator already creates a unique constraint, so specifyingunique: truein the@Columndecorator is redundant.♻️ Proposed fix
`@Index`({ unique: true }) - `@Column`({ type: 'varchar', length: 100, unique: true }) + `@Column`({ type: 'varchar', length: 100 }) slug: string;As per coding guidelines, the code should adhere to best practices for TypeORM.
🤖 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/referrals/entities/referral-entity.entity.ts` around lines 28 - 30, The Column decorator on the slug field redundantly declares unique: true while there is already an `@Index`({ unique: true }) on the same field; remove the unique: true option from the Column decorator so the unique constraint is defined only by `@Index`, leaving slug: string and `@Index`({ unique: true }) intact (update any related tests/migrations if they rely on the duplicate option).
47-48: ⚡ Quick winConsider using JSON or array column type for additionalEmails.
Storing an email array as a varchar(500) string requires manual serialization/deserialization. TypeORM supports JSON columns or Postgres array types (text[]) which handle this automatically and provide better type safety.
💡 Alternative approaches
Option 1: JSON column (database-agnostic)
- `@Column`({ type: 'varchar', length: 500, nullable: true }) - additionalEmails: string | null; + `@Column`({ type: 'json', nullable: true }) + additionalEmails: string[] | null;Option 2: Postgres array (if using PostgreSQL)
- `@Column`({ type: 'varchar', length: 500, nullable: true }) - additionalEmails: string | null; + `@Column`({ type: 'text', array: true, nullable: true }) + additionalEmails: string[] | null;As per coding guidelines, the code should adhere to best practices for performance and type safety.
🤖 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/referrals/entities/referral-entity.entity.ts` around lines 47 - 48, The additionalEmails column currently uses a varchar string which requires manual serialization; update the Referral entity's additionalEmails property to use a typed array/JSON column instead: change the Column decorator on additionalEmails to either a JSON column (e.g., type: 'json', property type string[] | null) for a DB-agnostic solution, or to a Postgres array (e.g., type: 'text', array: true, property type string[] | null) if you are on PostgreSQL; ensure any repository code that reads/writes additionalEmails treats it as an array and remove manual split/join serialization.src/referrals/entities/user-attribution.entity.ts (2)
22-23: ⚡ Quick winRemove redundant default function from timestamp column.
The
default: () => 'now()'is redundant with@CreateDateColumn, which automatically handles the timestamp default.♻️ Proposed fix
- `@CreateDateColumn`({ type: 'timestamp', default: () => 'now()' }) + `@CreateDateColumn`({ type: 'timestamp' }) createdAt: Date;As per coding guidelines, the code should adhere to best practices for TypeORM.
🤖 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/referrals/entities/user-attribution.entity.ts` around lines 22 - 23, The CreateDateColumn on the createdAt property in the UserAttribution entity redundantly specifies default: () => 'now()'; remove the default option so the decorator is used as intended—locate the createdAt field annotated with `@CreateDateColumn` in user-attribution.entity (the createdAt property) and delete the default: () => 'now()' configuration, leaving simply `@CreateDateColumn`({ type: 'timestamp' }) createdAt: Date; to rely on TypeORM's built-in behavior.
8-14: ⚡ Quick winConsider adding foreign key relationships.
Both
userIdandreferralEntityIdlack@ManyToOnerelationships to their respective entities. Adding these would enable TypeORM to manage referential integrity and support cascading operations.🔗 Proposed enhancement
+import { ManyToOne, JoinColumn } from 'typeorm'; +import { User } from '../../user/user.entity'; // Adjust path as needed +import { ReferralEntity } from './referral-entity.entity'; + `@ManyToOne`(() => User, { onDelete: 'CASCADE' }) + `@JoinColumn`({ name: 'userId' }) + user: User; + `@Index`() `@Column`({ type: 'uuid' }) userId: string; + `@ManyToOne`(() => ReferralEntity, { onDelete: 'SET NULL', nullable: true }) + `@JoinColumn`({ name: 'referralEntityId' }) + referralEntity: ReferralEntity; + `@Index`() `@Column`({ type: 'uuid', nullable: true }) referralEntityId: string | null;As per coding guidelines, the code should adhere to best practices for TypeORM and database integrity.
🤖 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/referrals/entities/user-attribution.entity.ts` around lines 8 - 14, Add proper TypeORM relations for the userId and referralEntityId columns: replace or augment the raw columns with `@ManyToOne` relations (e.g., add properties user: User and referralEntity: ReferralEntity) decorated with `@ManyToOne`(() => User, u => u.<backRef> , { onDelete: 'CASCADE' }) and `@JoinColumn`({ name: 'userId' }) for user, and similarly `@ManyToOne`(() => ReferralEntity, r => r.<backRef> , { onDelete: 'SET NULL' }) with `@JoinColumn`({ name: 'referralEntityId' }) for referralEntity; also import User and ReferralEntity and remove duplicate/conflicting column metadata or ensure the numeric/uuid `@Column` definitions remain in sync with the relation metadata (use nullable on referralEntityId).
🤖 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 `@src/adapters/postgres/user-adapter.ts`:
- Around line 1997-2010: The code only computes referLinkForElastic during
initial creation (referLinkForElastic, referralsService.createUserAttribution,
buildReferLink, result.userId), so ES upsert/rebuild paths miss it; extract this
logic into a reusable helper (e.g., getReferLinkForElastic(userId,
incomingSlug?)) that tries referralsService.createUserAttribution (or an
equivalent service call to fetch existing attribution) and falls back to
buildReferLink when resolvedSlug exists, and call that helper from both the
initial create flow and any profile upsert/rebuild code paths so referLink is
always populated for Elasticsearch documents.
In `@src/forms/services/form-submission.service.ts`:
- Line 3163: Wrap the call to
this.referralsService.getUserReferLink(user.userId) in a try-catch to prevent
exceptions from bubbling up and breaking the Elasticsearch user document build;
if the call fails assign a safe fallback (e.g., null or empty string) to
referLink and log the error (use the existing logger instance, e.g.,
this.logger.error) with contextual info including user.userId and the caught
error. Ensure you only change the area around the existing const referLink =
await this.referralsService.getUserReferLink(user.userId); statement so other
logic that consumes referLink continues to work with the fallback.
In `@src/referrals/dto/create-referral-entity.dto.ts`:
- Around line 1-68: The file CreateReferralEntityDto uses single quotes across
imports, decorators and string literals which violates the project's
Prettier/formatting rules; run the project's Prettier formatter (or your
configured formatter) on src/referrals/dto/create-referral-entity.dto.ts to
convert all single quotes to double quotes and normalize spacing for imports,
decorator metadata (ApiProperty/ApiPropertyOptional), and string literals (e.g.,
descriptions and type arrays) so formatting matches the repository standard and
linter/prettier checks pass.
In `@src/referrals/dto/import-referrals.dto.ts`:
- Around line 1-13: The file ImportReferralsDto uses single quotes for string
literals and should be formatted with double quotes per project Prettier
settings; update all string literals (e.g., the ApiProperty description, import
specifiers if necessary) to use double quotes and then run Prettier (or your
editor formatter) to apply the project’s formatting rules so ImportReferralsDto,
its csv property, and decorators (`@ApiProperty`, `@IsString`, `@IsNotEmpty`) follow
the configured style.
In `@src/referrals/dto/list-referrals.dto.ts`:
- Around line 1-55: The file uses single quotes for imports and decorators;
update all string literals to use double quotes to match project Prettier
settings (e.g., in ReferralFiltersDto and ListReferralsDto usage:
ApiPropertyOptional type strings, enum references, and import statements). Run
the project's Prettier formatter (or replace single quotes with double quotes
across the file) and ensure linting passes for symbols ReferralFiltersDto,
ListReferralsDto, ReferralEntityType, and ReferralEntitySubType.
- Around line 50-54: The filters property in ListReferralsDto currently uses
`@Type`(() => ReferralFiltersDto) but lacks nested validation; add
`@ValidateNested`() (from class-validator) to the filters property so the
transformed object is validated against ReferralFiltersDto constraints, ensuring
the decorator is applied alongside `@IsOptional`(), `@IsObject`(), `@Type`(() =>
ReferralFiltersDto) on the filters?: ReferralFiltersDto field.
In `@src/referrals/dto/update-referral-slug.dto.ts`:
- Around line 1-62: The file UpdateReferralSlugDto uses single quotes for
imports and strings; update all string literals and import quote marks to double
quotes and run the project's Prettier/formatting script (or your editor's
formatter) to apply project-wide rules so the file conforms to the Google style
and Prettier config; ensure you re-save UpdateReferralSlugDto and commit only
the formatting changes (no logic changes).
In `@src/referrals/entities/referral-entity.entity.ts`:
- Around line 1-64: The file uses single quotes instead of the project's
double-quote style; update all string literals and import/ decorator metadata to
use double quotes (e.g., the import lines, Entity name string, Column type
values, default values, and any other quoted strings found in the ReferralEntity
class and its decorators) and then run Prettier to reformat — target symbols:
the top-level imports, the `@Entity` decorator, all
`@Column/`@CreateDateColumn/@UpdateDateColumn options, and the ReferralEntity
class declaration to ensure consistent double-quote formatting throughout.
In `@src/referrals/entities/referral-slug-history.entity.ts`:
- Around line 1-30: Replace all single-quoted string literals with double quotes
in this entity (e.g., decorator arguments like Entity("ReferralSlugHistory"),
PrimaryGeneratedColumn("uuid"), Column({ type: "uuid" }), Index({ unique: true
}) and Column length/type values) and then run Prettier to apply project
formatting; ensure exported class ReferralSlugHistory and its properties (id,
referralEntityId, oldSlug, newSlug, changedBy, changedAt) retain their types and
decorators exactly as before while only changing quote style and running the
formatter.
In `@src/referrals/entities/user-attribution.entity.ts`:
- Around line 1-24: The file uses single quotes for string literals which
violates Prettier; update all string literals to use double quotes (including
import module specifiers, Entity name, Column type strings,
PrimaryGeneratedColumn argument, and CreateDateColumn default SQL string) in the
UserAttribution class and its decorators (see imports and decorators like
Entity, PrimaryGeneratedColumn, Column, CreateDateColumn, Index) and then run
Prettier to auto-format the file to ensure it matches the project's formatting
rules.
In `@src/referrals/referrals.controller.ts`:
- Around line 109-111: The bulkInsert handler currently uses `@Body`() with
ValidationPipe which won't validate each array element; update the `@Body`
decorator to use ParseArrayPipe so each item is validated: replace `@Body`() dtos:
CreateReferralEntityDto[] with `@Body`(new ParseArrayPipe({ items:
CreateReferralEntityDto })) dtos: CreateReferralEntityDto[] (keep the existing
ValidationPipe on the method), referencing the bulkInsert method and
CreateReferralEntityDto to enforce per-item validation.
In `@src/referrals/referrals.service.ts`:
- Around line 277-297: The CSV parsing currently uses naive split(',') in the
header and row loops (header = lines[0].split(',') and cols =
lines[rowIdx].split(',')), which breaks when fields are quoted or contain
escaped commas; replace this with a proper CSV parse step (e.g., use a CSV
parser library or a quote-aware parser function) before computing header/indexes
(idx, iFirst, iLast, etc.) and before iterating rows so header and cols are
constructed from parsed records rather than string.split, preserving quoted
fields and escapes; ensure the rest of the logic (references to iFirst, iType,
iSubType, firstName, etc.) consumes the parsed record arrays/objects.
- Around line 36-37: Normalize dto.contactEmail (trim + toLowerCase) before any
uniqueness checks and before persisting: create a normalizedEmail =
dto.contactEmail?.trim().toLowerCase() and use that when calling
this.referralRepo.findOne and when assigning to the entity/DTO saved to the DB;
apply the same normalization in the update path (the code around the update
handler and the block that currently queries in lines handling updates) so both
the create and update flows use the normalized value and prevent case-variant
duplicates. Ensure you also update dto.contactEmail or the entity field to the
normalizedEmail prior to saving.
- Around line 182-185: When updating referral fields (the dto → entity block
handling dto.firstName/lastName/type/subType/contactEmail), ensure you compute
the effective values (let effectiveType = dto.type ?? entity.type and let
effectiveContactEmail = dto.contactEmail ?? entity.contactEmail) and, if
effectiveType === 'INTERNAL', run the existing "internal contact existence"
validation against effectiveContactEmail before persisting changes; apply the
same fix to the similar block around the 196-207 logic so that changing type to
INTERNAL without supplying contactEmail still validates the stored
entity.contactEmail.
- Around line 340-345: The parsed batchSize from
this.configService.get('REFERRAL_BULK_BATCH_SIZE') must be validated before the
for loop that uses i += batchSize; ensure the value assigned to batchSize is a
positive integer (e.g., parseInt(...,10), check for NaN, <= 0, or non-finite)
and if invalid replace it with a safe default (like 100) before iterating over
dtos; update the variable used in the loop (batchSize) so the loop cannot be
non-terminating and add a brief comment referencing batchSize and dtos to
clarify the guard.
In `@src/referrals/referrals.types.ts`:
- Around line 1-16: The enum string literals use single quotes and must be
formatted with double quotes to satisfy Prettier; update the string values in
ReferralEntityType, ReferralEntitySubType, and ReferralEntityStatus to use
double quotes (or run the project's Prettier/formatter) so entries like EXTERNAL
= "external", ORGANISATION = "organisation", and ACTIVE = "active" follow the
project's formatting rules.
In `@src/referrals/utils/referral-slug.util.ts`:
- Around line 4-10: DEFAULT_REFERRAL_BASE_URL currently returns a
possibly-undefined FRONTEND_URL string and buildReferLink concatenates it
directly causing malformed links; change DEFAULT_REFERRAL_BASE_URL to throw /
return error fast when process.env.FRONTEND_URL is not set, and rewrite
buildReferLink to construct the link using the standard URL and URLSearchParams
APIs (use new URL(base) to normalize slashes and preserve existing path/query,
then append or set the "refer" search param via url.searchParams.set('refer',
encodeURIComponent(slug.trim())), and return url.toString()); update references
to DEFAULT_REFERRAL_BASE_URL() and buildReferLink(slug, baseUrl?) accordingly so
missing base URL fails fast and existing query strings are preserved.
---
Nitpick comments:
In `@src/forms/services/form-submission.service.ts`:
- Line 99: The constructor parameter list ending with "private readonly
referralsService: ReferralsService" should include a trailing comma for style
consistency; update the constructor (the parameter declaration containing
referralsService) to add a comma after that parameter so future diffs remain
cleaner and align with TypeScript/JS style guides.
In `@src/referrals/entities/referral-entity.entity.ts`:
- Around line 59-63: Remove the redundant default functions on the timestamp
columns: the `@CreateDateColumn` and `@UpdateDateColumn` decorators already manage
timestamps, so update the referral entity by removing the "default: () =>
'now()'" option from the `@CreateDateColumn` and `@UpdateDateColumn` decorators for
createdAt and updatedAt (keep the decorators and the createdAt/updatedAt fields
intact).
- Around line 28-30: The Column decorator on the slug field redundantly declares
unique: true while there is already an `@Index`({ unique: true }) on the same
field; remove the unique: true option from the Column decorator so the unique
constraint is defined only by `@Index`, leaving slug: string and `@Index`({ unique:
true }) intact (update any related tests/migrations if they rely on the
duplicate option).
- Around line 47-48: The additionalEmails column currently uses a varchar string
which requires manual serialization; update the Referral entity's
additionalEmails property to use a typed array/JSON column instead: change the
Column decorator on additionalEmails to either a JSON column (e.g., type:
'json', property type string[] | null) for a DB-agnostic solution, or to a
Postgres array (e.g., type: 'text', array: true, property type string[] | null)
if you are on PostgreSQL; ensure any repository code that reads/writes
additionalEmails treats it as an array and remove manual split/join
serialization.
In `@src/referrals/entities/referral-slug-history.entity.ts`:
- Around line 17-19: Remove the redundant unique constraint on the oldSlug
column: keep the `@Index`({ unique: true }) decorator and remove the unique: true
option from the `@Column` decorator on the oldSlug property in the
ReferralSlugHistory entity (look for oldSlug, `@Index` and `@Column` in the file) so
the unique constraint is defined only once via the index.
- Around line 27-28: The `@CreateDateColumn` decorator on the changedAt property
currently includes a redundant default: () => 'now()'; remove that default
option so the decorator is simply `@CreateDateColumn`({ type: 'timestamp' }) (or
just `@CreateDateColumn`()) and leave the changedAt: Date; property intact to let
TypeORM manage the timestamp automatically.
- Around line 14-15: The referralEntityId column in ReferralSlugHistory should
be modeled as a foreign key relation to ReferralEntity: add a `@ManyToOne`(() =>
ReferralEntity, { nullable: false }) relation property (e.g., referralEntity) on
the ReferralSlugHistory class and annotate it with `@JoinColumn`({ name:
'referralEntityId' }), keeping or adding the referralEntityId `@Column`({ type:
'uuid' }) if you want the explicit FK field; this lets TypeORM enforce
referential integrity and enables join queries using the
ReferralSlugHistory.referralEntity and the ReferralEntity entity.
In `@src/referrals/entities/user-attribution.entity.ts`:
- Around line 22-23: The CreateDateColumn on the createdAt property in the
UserAttribution entity redundantly specifies default: () => 'now()'; remove the
default option so the decorator is used as intended—locate the createdAt field
annotated with `@CreateDateColumn` in user-attribution.entity (the createdAt
property) and delete the default: () => 'now()' configuration, leaving simply
`@CreateDateColumn`({ type: 'timestamp' }) createdAt: Date; to rely on TypeORM's
built-in behavior.
- Around line 8-14: Add proper TypeORM relations for the userId and
referralEntityId columns: replace or augment the raw columns with `@ManyToOne`
relations (e.g., add properties user: User and referralEntity: ReferralEntity)
decorated with `@ManyToOne`(() => User, u => u.<backRef> , { onDelete: 'CASCADE'
}) and `@JoinColumn`({ name: 'userId' }) for user, and similarly `@ManyToOne`(() =>
ReferralEntity, r => r.<backRef> , { onDelete: 'SET NULL' }) with `@JoinColumn`({
name: 'referralEntityId' }) for referralEntity; also import User and
ReferralEntity and remove duplicate/conflicting column metadata or ensure the
numeric/uuid `@Column` definitions remain in sync with the relation metadata (use
nullable on referralEntityId).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b8c6c8ce-08e1-4006-a964-06f66fec9a76
📒 Files selected for processing (23)
src/adapters/postgres/postgres-module.tssrc/adapters/postgres/user-adapter.tssrc/app.module.tssrc/common/utils/api-id.config.tssrc/common/utils/response.messages.tssrc/elasticsearch/elasticsearch.service.tssrc/elasticsearch/interfaces/user.interface.tssrc/elasticsearch/user-elasticsearch.service.tssrc/forms/forms.module.tssrc/forms/services/form-submission.service.tssrc/referrals/dto/create-referral-entity.dto.tssrc/referrals/dto/import-referrals.dto.tssrc/referrals/dto/list-referrals.dto.tssrc/referrals/dto/update-referral-slug.dto.tssrc/referrals/entities/referral-entity.entity.tssrc/referrals/entities/referral-slug-history.entity.tssrc/referrals/entities/user-attribution.entity.tssrc/referrals/referrals.controller.tssrc/referrals/referrals.module.tssrc/referrals/referrals.service.tssrc/referrals/referrals.types.tssrc/referrals/utils/referral-slug.util.tssrc/user/dto/user-create.dto.ts
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
src/referrals/referrals.service.ts (4)
342-347:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winGuard
batchSizebefore using it in loop increment.
0or negative values can make the bulk loop non-terminating.Suggested fix
- const batchSize = parseInt(this.configService.get('REFERRAL_BULK_BATCH_SIZE') || '100', 10); + const parsedBatchSize = Number.parseInt( + this.configService.get("REFERRAL_BULK_BATCH_SIZE") || "100", + 10, + ); + const batchSize = + Number.isInteger(parsedBatchSize) && parsedBatchSize > 0 + ? parsedBatchSize + : 100;🤖 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/referrals/referrals.service.ts` around lines 342 - 347, The loop uses batchSize parsed from this.configService.get('REFERRAL_BULK_BATCH_SIZE') and can hang if batchSize is 0 or negative; before the for loop that slices dtos, validate batchSize is a positive integer (e.g., if parsed value <= 0 or NaN, set it to a safe default like 100 or throw a clear error), then proceed to use batchSize in the for (let i = 0; i < dtos.length; i += batchSize) loop to ensure termination and predictable batching.
38-57:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize
contactEmailbefore create-time uniqueness check and persistence.Case/whitespace variants can bypass uniqueness checks and create duplicate logical identities.
Suggested fix
async createReferralEntity(dto: CreateReferralEntityDto, createdBy?: string) { - if (dto.contactEmail) { - const existingEmail = await this.referralRepo.findOne({ where: { contactEmail: dto.contactEmail } }); + const normalizedEmail = dto.contactEmail?.trim().toLowerCase(); + if (normalizedEmail) { + const existingEmail = await this.referralRepo.findOne({ where: { contactEmail: normalizedEmail } }); if (existingEmail) { - throw new ConflictException(`Referral slug already exists for email ${dto.contactEmail}`); + throw new ConflictException(`Referral slug already exists for email ${normalizedEmail}`); } if (dto.type === ReferralEntityType.INTERNAL) { - const existingUser = await this.userRepo.findOne({ where: { email: dto.contactEmail.toLowerCase() } }); + const existingUser = await this.userRepo.findOne({ where: { email: normalizedEmail } }); if (!existingUser) { - throw new BadRequestException(`Internal user email ${dto.contactEmail} does not exist in the system`); + throw new BadRequestException(`Internal user email ${normalizedEmail} does not exist in the system`); } } } @@ - contactEmail: dto.contactEmail ?? null, + contactEmail: normalizedEmail ?? null,🤖 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/referrals/referrals.service.ts` around lines 38 - 57, Normalize dto.contactEmail (trim and toLowerCase) before any uniqueness or existence checks and before persisting: compute a normalizedEmail from dto.contactEmail (e.g., dto.contactEmail?.trim().toLowerCase()), use normalizedEmail in the referralRepo.findOne uniqueness check and in the userRepo.findOne check for ReferralEntityType.INTERNAL, and ensure the object passed to referralRepo.create stores contactEmail: normalizedEmail (or null if absent). Update references to dto.contactEmail in the shown block to use this normalized value so case/whitespace variants cannot bypass checks.
197-210:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate path should validate effective INTERNAL email even when
contactEmailis omitted.Changing
typetoINTERNALwithout sendingcontactEmailskips existence validation for the stored email.Suggested fix
- if (dto.contactEmail !== undefined && dto.contactEmail !== entity.contactEmail) { - const existingEmail = await this.referralRepo.findOne({ where: { contactEmail: dto.contactEmail } }); + const effectiveType = dto.type ?? entity.type; + const effectiveContactEmail = (dto.contactEmail ?? entity.contactEmail)?.trim().toLowerCase(); + + if (dto.contactEmail !== undefined && effectiveContactEmail !== entity.contactEmail) { + const existingEmail = await this.referralRepo.findOne({ where: { contactEmail: effectiveContactEmail } }); if (existingEmail && existingEmail.id !== entity.id) { - throw new ConflictException(`Contact email '${dto.contactEmail}' is already used by another referral`); + throw new ConflictException(`Contact email '${effectiveContactEmail}' is already used by another referral`); } - const resolvedType = dto.type ?? entity.type; - if (resolvedType === ReferralEntityType.INTERNAL) { - const existingUser = await this.userRepo.findOne({ where: { email: dto.contactEmail.toLowerCase() } }); - if (!existingUser) { - throw new BadRequestException(`Internal user email ${dto.contactEmail} does not exist in the system`); - } - } - entity.contactEmail = dto.contactEmail; + entity.contactEmail = effectiveContactEmail; + } + + if (effectiveType === ReferralEntityType.INTERNAL) { + if (!effectiveContactEmail) { + throw new BadRequestException('Internal referral must have contactEmail'); + } + const existingUser = await this.userRepo.findOne({ where: { email: effectiveContactEmail } }); + if (!existingUser) { + throw new BadRequestException(`Internal user email ${effectiveContactEmail} does not exist in the system`); + } }🤖 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/referrals/referrals.service.ts` around lines 197 - 210, The update currently only validates internal-user existence when dto.contactEmail is provided; instead compute resolvedType = dto.type ?? entity.type first and if resolvedType === ReferralEntityType.INTERNAL ensure the effectiveEmail = (dto.contactEmail ?? entity.contactEmail) exists and passes lookup via this.userRepo.findOne({ where: { email: effectiveEmail.toLowerCase() } }) throwing BadRequestException if not found; keep the uniqueness check for dto.contactEmail when present (using this.referralRepo.findOne and id comparison) and only assign entity.contactEmail = dto.contactEmail after these validations.
279-299:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftCSV parsing is not quote-safe.
Using
split(",")breaks valid CSV rows with quoted commas, causing column shifts and bad imports.🤖 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/referrals/referrals.service.ts` around lines 279 - 299, The CSV parsing uses naive split(',') which breaks on quoted fields (see header, idx, and the loop that builds cols and firstName), so replace the ad-hoc parsing with a proper CSV parser (e.g., papaparse or csv-parse) to read lines into columns safely and preserve quoted commas; update the code that computes header, idx('firstName'...), and the rows loop to use the parser's output (parsed header array and parsed rows) and then use those indices (iFirst, iLast, iType, iSubType, iRegion, iEmail, iCountry) to access values instead of split(',') so quoted fields are handled correctly and column alignment is preserved.src/referrals/referrals.controller.ts (1)
110-112:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
ParseArrayPipefor bulk DTO item validation.
ValidationPipedoes not reliably validate each element for a plain top-levelCreateReferralEntityDto[]body.As per coding guidelines, "The code adheres to best practices associated with nestjs framework."Suggested fix
-import { Body, Controller, Get, Param, Patch, Post, Query, UsePipes, ValidationPipe, UseFilters, Res, HttpStatus, UseGuards, Req } from '`@nestjs/common`'; +import { Body, Controller, Get, Param, Patch, Post, Query, UsePipes, ValidationPipe, UseFilters, Res, HttpStatus, UseGuards, Req, ParseArrayPipe } from '`@nestjs/common`'; @@ - async bulkInsert(`@Body`() dtos: CreateReferralEntityDto[], `@Req`() request: RequestWithUser, `@Res`() response: Response) { + async bulkInsert( + `@Body`(new ParseArrayPipe({ items: CreateReferralEntityDto })) dtos: CreateReferralEntityDto[], + `@Req`() request: RequestWithUser, + `@Res`() response: Response + ) {🤖 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/referrals/referrals.controller.ts` around lines 110 - 112, The bulkInsert endpoint in referrals.controller.ts currently uses a plain ValidationPipe on a top-level CreateReferralEntityDto[] which won't validate each array element; update the bulkInsert method to use Nest's ParseArrayPipe configured with the item type CreateReferralEntityDto and include a ValidationPipe (e.g., ParseArrayPipe({ items: CreateReferralEntityDto, whitelist: true, transform: true }) or ParseArrayPipe({ items: CreateReferralEntityDto, options: { transform: true, whitelist: true } })) so each DTO in the array is individually validated before reaching the bulkInsert(`@Body`() dtos: CreateReferralEntityDto[], ...) handler.
🤖 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 `@src/referrals/dto/referral-report.dto.ts`:
- Around line 71-73: The filters property on ReferralReportReportDto isn't being
validated recursively; add class-validator/class-transformer decorators to
enforce nested DTO validation by applying `@ValidateNested`() and use `@Type`(() =>
ReferralReportFiltersDto) on the filters?: ReferralReportFiltersDto property so
the constraints inside ReferralReportFiltersDto (e.g., `@IsString`, `@IsArray`,
`@IsEnum`) are executed; keep the existing `@IsOptional`() and
`@ApiPropertyOptional`({ type: ReferralReportFiltersDto }) in place and adjust
imports as needed.
- Around line 22-25: The slug_id property is only validated as a string; add
`@IsUUID`() to slug_id (keep `@IsOptional`()) so invalid UUIDs return 400s; for the
filters property, add `@ValidateNested`() and `@Type`(() => <FiltersDtoClass>)
(replace <FiltersDtoClass> with the actual nested DTO class used for filters) to
ensure nested DTO properties are validated, and import Type from
class-transformer and ValidateNested/IsUUID from class-validator accordingly.
---
Duplicate comments:
In `@src/referrals/referrals.controller.ts`:
- Around line 110-112: The bulkInsert endpoint in referrals.controller.ts
currently uses a plain ValidationPipe on a top-level CreateReferralEntityDto[]
which won't validate each array element; update the bulkInsert method to use
Nest's ParseArrayPipe configured with the item type CreateReferralEntityDto and
include a ValidationPipe (e.g., ParseArrayPipe({ items: CreateReferralEntityDto,
whitelist: true, transform: true }) or ParseArrayPipe({ items:
CreateReferralEntityDto, options: { transform: true, whitelist: true } })) so
each DTO in the array is individually validated before reaching the
bulkInsert(`@Body`() dtos: CreateReferralEntityDto[], ...) handler.
In `@src/referrals/referrals.service.ts`:
- Around line 342-347: The loop uses batchSize parsed from
this.configService.get('REFERRAL_BULK_BATCH_SIZE') and can hang if batchSize is
0 or negative; before the for loop that slices dtos, validate batchSize is a
positive integer (e.g., if parsed value <= 0 or NaN, set it to a safe default
like 100 or throw a clear error), then proceed to use batchSize in the for (let
i = 0; i < dtos.length; i += batchSize) loop to ensure termination and
predictable batching.
- Around line 38-57: Normalize dto.contactEmail (trim and toLowerCase) before
any uniqueness or existence checks and before persisting: compute a
normalizedEmail from dto.contactEmail (e.g.,
dto.contactEmail?.trim().toLowerCase()), use normalizedEmail in the
referralRepo.findOne uniqueness check and in the userRepo.findOne check for
ReferralEntityType.INTERNAL, and ensure the object passed to referralRepo.create
stores contactEmail: normalizedEmail (or null if absent). Update references to
dto.contactEmail in the shown block to use this normalized value so
case/whitespace variants cannot bypass checks.
- Around line 197-210: The update currently only validates internal-user
existence when dto.contactEmail is provided; instead compute resolvedType =
dto.type ?? entity.type first and if resolvedType ===
ReferralEntityType.INTERNAL ensure the effectiveEmail = (dto.contactEmail ??
entity.contactEmail) exists and passes lookup via this.userRepo.findOne({ where:
{ email: effectiveEmail.toLowerCase() } }) throwing BadRequestException if not
found; keep the uniqueness check for dto.contactEmail when present (using
this.referralRepo.findOne and id comparison) and only assign entity.contactEmail
= dto.contactEmail after these validations.
- Around line 279-299: The CSV parsing uses naive split(',') which breaks on
quoted fields (see header, idx, and the loop that builds cols and firstName), so
replace the ad-hoc parsing with a proper CSV parser (e.g., papaparse or
csv-parse) to read lines into columns safely and preserve quoted commas; update
the code that computes header, idx('firstName'...), and the rows loop to use the
parser's output (parsed header array and parsed rows) and then use those indices
(iFirst, iLast, iType, iSubType, iRegion, iEmail, iCountry) to access values
instead of split(',') so quoted fields are handled correctly and column
alignment is preserved.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b1019cdd-6497-4e4c-84f3-c2320b826d96
📒 Files selected for processing (5)
src/common/utils/api-id.config.tssrc/common/utils/response.messages.tssrc/referrals/dto/referral-report.dto.tssrc/referrals/referrals.controller.tssrc/referrals/referrals.service.ts
✅ Files skipped from review due to trivial changes (1)
- src/common/utils/response.messages.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/elasticsearch/user-elasticsearch.service.ts (1)
777-785:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate or coerce referLink filter value appropriately.
If
valueis not a string,JSON.stringify(value)produces a JSON-formatted string (e.g.,'{"foo":"bar"}'or'["abc"]') that won't meaningfully match referLink values. This leads to confusing search behavior.Consider either:
- Validating that
valueis a string and throwing a descriptive error for other types- Using
.toString()for primitives and rejecting objects/arrays explicitly- Documenting that only string values are accepted
🔧 Proposed fix
if (field === 'referLink') { - const referLinkValue = typeof value === 'string' ? value : JSON.stringify(value); + if (typeof value !== 'string') { + throw new Error('referLink filter must be a string'); + } + const referLinkValue = value; searchQuery.bool.filter.push({ wildcard: { 'profile.referLink': `*${referLinkValue.toLowerCase()}*`, }, }); return; }As per coding guidelines, the code should adhere to NestJS best practices for input validation.
🤖 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/elasticsearch/user-elasticsearch.service.ts` around lines 777 - 785, The referLink branch in the search builder treats non-string values by JSON.stringify(value), which yields JSON that won't meaningfully match profile.referLink; update the logic in the method that builds searchQuery (the block checking if (field === 'referLink') and pushing to searchQuery.bool.filter) to first validate/coerce value: if value is a string use it as-is (lowercased), if it's a primitive (number/boolean) use value.toString(), and if it's an object/array throw or return a descriptive validation error (or reject the request) instead of JSON.stringify; ensure the error follows NestJS validation conventions so callers receive a clear message about accepted types.
♻️ Duplicate comments (1)
src/referrals/referrals.service.ts (1)
342-342:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winGuard
batchSizebefore using it as loop increment.Line 342 can yield
0, negative, orNaN; with Line 346 (i += batchSize) this can stall or break batching.Suggested fix
- const batchSize = Number.parseInt(this.configService.get('REFERRAL_BULK_BATCH_SIZE') || '100', 10); + const parsedBatchSize = Number.parseInt( + this.configService.get('REFERRAL_BULK_BATCH_SIZE') || '100', + 10, + ); + const batchSize = + Number.isInteger(parsedBatchSize) && parsedBatchSize > 0 + ? parsedBatchSize + : 100;🤖 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/referrals/referrals.service.ts` at line 342, The parsed batchSize from this.configService.get('REFERRAL_BULK_BATCH_SIZE') can be 0, negative, or NaN which will break the batching loop that uses i += batchSize; validate and normalize batchSize before the loop (where batchSize is declared) by coercing it to a positive integer and falling back to a safe default (e.g., 100) — e.g., compute a sanitizedBatchSize = Number.isFinite(batchSize) && batchSize > 0 ? Math.floor(batchSize) : 100 and then use sanitizedBatchSize for the loop increment (and any related logic) instead of batchSize.
🤖 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.
Outside diff comments:
In `@src/elasticsearch/user-elasticsearch.service.ts`:
- Around line 777-785: The referLink branch in the search builder treats
non-string values by JSON.stringify(value), which yields JSON that won't
meaningfully match profile.referLink; update the logic in the method that builds
searchQuery (the block checking if (field === 'referLink') and pushing to
searchQuery.bool.filter) to first validate/coerce value: if value is a string
use it as-is (lowercased), if it's a primitive (number/boolean) use
value.toString(), and if it's an object/array throw or return a descriptive
validation error (or reject the request) instead of JSON.stringify; ensure the
error follows NestJS validation conventions so callers receive a clear message
about accepted types.
---
Duplicate comments:
In `@src/referrals/referrals.service.ts`:
- Line 342: The parsed batchSize from
this.configService.get('REFERRAL_BULK_BATCH_SIZE') can be 0, negative, or NaN
which will break the batching loop that uses i += batchSize; validate and
normalize batchSize before the loop (where batchSize is declared) by coercing it
to a positive integer and falling back to a safe default (e.g., 100) — e.g.,
compute a sanitizedBatchSize = Number.isFinite(batchSize) && batchSize > 0 ?
Math.floor(batchSize) : 100 and then use sanitizedBatchSize for the loop
increment (and any related logic) instead of batchSize.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e88f6b3f-eb10-4151-9065-c58aeb7041ff
📒 Files selected for processing (4)
src/elasticsearch/user-elasticsearch.service.tssrc/referrals/dto/list-referrals.dto.tssrc/referrals/referrals.service.tssrc/referrals/utils/referral-slug.util.ts
|
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
1 participant
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 by CodeRabbit