Feature/offer notifications#145
Conversation
✅ Added offer_accepted and offer_rejected notification types
✅ Enhanced NotificationService with DB persistence + Pusher integration
✅ Added deduplication via entityId field
✅ Modified OffersService to trigger notifications on status changes
✅ Includes correct payload: { offerId, requestTitle }
✅ Tests included for notification flows
✅ Migration for database schema updates
Files modified:
- src/modules/notifications/* - New notification types and enhanced service
- src/modules/offers/* - Notification triggers and tests
- src/migrations/* - Database schema updates
- src/config/index.ts - Development configuration
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds multi-store feature with entities, DTOs, services, controllers, and module wiring. Migrates user IDs to UUID, shifts public APIs to walletAddress, and extends user profile fields. Introduces role-based DTO validation, enhances notifications and offers workflows, updates multiple modules to UUID types, and adds comprehensive documentation and migrations. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant AC as AuthController
participant AS as AuthService
participant US as UserService
participant RS as RoleService
participant SS as StoreService
participant DB as Database
Note over C,AC: Registration (POST /auth/register-with-wallet)
C->>AC: RegisterUserDto (walletAddress, role, ...buyer/sellerData)
AC->>AS: registerWithWallet(dto)
AS->>US: createUser(dto + profile fields)
US->>DB: Insert user, link role (UserRole)
DB-->>US: User
AS->>RS: assignRole(walletAddress, role)
RS->>DB: Insert UserRole
alt role == seller
AS->>SS: createDefaultStore(user.id, sellerData)
SS->>DB: Insert default Store (PENDING_APPROVAL)
DB-->>SS: Store
SS-->>AS: Store (ignored on failure)
end
AS-->>AC: { token, expiresIn, user (no id) }
AC-->>C: 201 Created
sequenceDiagram
autonumber
participant C as Client
participant API as OffersController
participant OS as OffersService
participant NS as NotificationService
participant DB as Database
Note over C,API: Accept Offer
C->>API: PUT /offers/:id/accept
API->>OS: accept(offerId, user)
OS->>DB: findOne(offer + seller, buyerRequest)
OS->>DB: save(status=ACCEPTED)
OS->>NS: createAndSendNotificationToUser({ type: offer_accepted, entityId: offerId, userId: sellerId, payload })
OS->>DB: find(pending offers for same request)
loop each pending offer
OS->>DB: save(status=REJECTED)
OS->>NS: createAndSendNotificationToUser({ type: offer_rejected, entityId, userId: sellerId, payload })
end
OS-->>API: Accepted offer
API-->>C: 200 OK
sequenceDiagram
autonumber
participant CL as Client
participant JS as JwtStrategy
participant AS as AuthService
participant DB as Database
Note over CL,JS: JWT validate
CL-->>JS: JWT payload { walletAddress?, id? }
alt walletAddress present
JS->>AS: getUserByWalletAddress(walletAddress)
else id present
JS->>AS: getUserById(id)
else
JS-->>CL: Unauthorized (Invalid token payload)
end
AS->>DB: Query user (+roles)
DB-->>AS: User | null
alt found
JS-->>CL: req.user = { id (uuid), walletAddress, role[] }
else
JS-->>CL: Unauthorized (User not found)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 43
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
src/modules/reviews/dto/review.dto.ts (2)
8-16: Type mismatch: productId is number here but string in CreateReviewDto. Align both sides.Decide on a single productId type (likely number if Product IDs are numeric). If productId should be numeric, update CreateReviewDto accordingly (see next comment).
Apply this diff if productId is numeric:
export class ReviewResponseDTO { id: string; userId: string; - productId: number; + productId: number; rating: number; comment?: string; createdAt: Date; userName?: string; // Optional user information to include with reviews }If productId should be uuid/string, then change this DTO field to
stringand propagate through services/controllers.
24-41: CreateReviewDto: enforce numeric types; remove userId from input to avoid impersonation.
- productId should be numeric (or uuid) and validated accordingly; current
IsStringdoesn’t match the response DTO.- rating should be an integer (1–5).
- Accepting
userIdfrom the client is a security risk (user spoofing). Usereq.user.idserver-side instead.Apply this diff if productId is numeric:
export class CreateReviewDto { - @IsString() - @IsNotEmpty() - productId: string; + @IsNumber() + @Min(1) + productId: number; - @IsNumber() + @IsNumber() @Min(1) @Max(5) rating: number; @IsString() @IsNotEmpty() comment: string; - @IsString() - @IsOptional() - userId?: string; + // userId is derived from req.user and must not be accepted from the client }Alternatively, if productId is uuid:
- @IsString() - @IsNotEmpty() - productId: string; + @IsString() + @IsNotEmpty() + @IsUUID() + productId: string;src/modules/auth/middleware/jwt-auth.middleware.ts (2)
35-39: UUID regression: parseInt on decoded.id breaks user lookupID is now a UUID string.
parseInt(decoded.id)will yieldNaNand break the query. Query by the string UUID directly.- const user = await userRepository.findOne({ - where: { id: parseInt(decoded.id) }, - relations: ['userRoles', 'userRoles.role'], - }); + const user = await userRepository.findOne({ + where: { id: decoded.id }, + relations: ['userRoles', 'userRoles.role'], + });
1-1: Drop the any-cast by typing req as AuthenticatedRequestType the handler to
AuthenticatedRequestand assign a properly-typed user to removeas any. This keeps static checks aligned with the updated user shape.import { Request, Response, NextFunction } from 'express'; +import { AuthenticatedRequest } from '../../../types/auth-request.type'; @@ -export const jwtAuthMiddleware = async (req: Request, res: Response, next: NextFunction) => { +export const jwtAuthMiddleware = async (req: AuthenticatedRequest, res: Response, next: NextFunction) => { @@ - req.user = { + req.user = { id: user.id, email: user.email, walletAddress: user.walletAddress, name: user.name, role: user.userRoles?.map((ur) => ur.role.name as Role) || [decoded.role as Role], - } as any; + };Also applies to: 9-9, 51-51
src/modules/auth/middleware/authorize-roles.middleware.ts (2)
12-17: Guard against non-array role payloads and handle empty allowedRolesBe resilient if
req.user.roleis not an array. Optionally, treatallowedRoles.length === 0as “no restriction”.- const userRoles = req.user.role as Role[]; - const hasAllowedRole = userRoles.some((role) => allowedRoles.includes(role)); + const rawRoles = req.user.role as Role[] | Role; + const userRoles = Array.isArray(rawRoles) ? (rawRoles as Role[]) : ([rawRoles as Role]); + const hasAllowedRole = + allowedRoles.length === 0 || userRoles.some((role) => allowedRoles.includes(role)); - if (!hasAllowedRole) { - throw new UnauthorizedException('Insufficient permissions'); - } + if (!hasAllowedRole) { + throw new ForbiddenException('Insufficient permissions'); + }Add this import outside the shown range:
import { UnauthorizedException, ForbiddenException } from '@nestjs/common';
21-32: Return 401 for unauthenticated, 403 for insufficient permissionsCurrently all
UnauthorizedExceptionmap to 403. Differentiate statuses.- if (error instanceof UnauthorizedException) { - res.status(403).json({ + if (error instanceof UnauthorizedException) { + res.status(401).json({ success: false, message: error.message, }); - } else { + } else if (error instanceof ForbiddenException) { + res.status(403).json({ + success: false, + message: error.message, + }); + } else { res.status(500).json({ success: false, message: 'Internal server error', }); }src/modules/reviews/services/review.service.ts (1)
41-47: Prevent race-condition duplicates with a DB unique constraintChecking for an existing review in code is not sufficient under concurrency. Enforce a DB-level unique constraint on (userId, productId) and handle unique violations.
Proposed migration (outside this diff):
await queryRunner.query(` ALTER TABLE "reviews" ADD CONSTRAINT "UQ_reviews_user_product" UNIQUE ("userId", "productId") `);Then, in createReview, keep the pre-check for UX but rely on DB to guarantee consistency.
src/types/express.d.ts (1)
20-21: Unify user.id typing to string to match UUID migration.Request.user.id allows string | number while Express.User.id is string. This inconsistency will leak friction into handlers and tests.
Apply:
- id: string | number; // Allow both string and number types + id: string;Also applies to: 40-41
src/modules/buyer-requests/services/buyer-requests.service.ts (1)
100-104: Dynamic orderBy is vulnerable to SQL injection; whitelist allowed columns.Interpolating sortBy directly into orderBy is unsafe. Gate the input against an allowlist and normalize sortOrder.
Apply this diff:
- // Apply sorting - queryBuilder.orderBy(`request.${sortBy}`, sortOrder); - if (sortBy !== 'createdAt') { - queryBuilder.addOrderBy('request.createdAt', 'DESC'); - } + // Apply sorting (whitelist to avoid SQL injection) + const allowedSortBy = new Set(['createdAt', 'budgetMin', 'budgetMax', 'expiresAt', 'status', 'categoryId']); + const safeSortBy = allowedSortBy.has(sortBy) ? sortBy : 'createdAt'; + const safeSortOrder: 'ASC' | 'DESC' = sortOrder === 'ASC' ? 'ASC' : 'DESC'; + queryBuilder.orderBy(`request.${safeSortBy}`, safeSortOrder); + if (safeSortBy !== 'createdAt') { + queryBuilder.addOrderBy('request.createdAt', 'DESC'); + }src/modules/offers/services/offers.service.ts (1)
78-92: Fix race condition: accept flow is not atomic and can lead to double-acceptances.Between the “alreadyAccepted” check and saving the current offer as ACCEPTED, another concurrent accept can pass the same check and also get ACCEPTED. Rejections of pending offers are also not atomic with the acceptance.
Make the whole accept flow transactional and add a DB-level uniqueness guarantee that only one ACCEPTED offer can exist per buyerRequest. Also, send notifications after the transaction commits to avoid notifying on rolled-back state.
Apply this diff to make the accept flow transactional:
- async accept(offerId: string, buyerId: string): Promise<Offer> { - const offer = await this.offerRepository.findOne({ - where: { id: offerId }, - relations: ['buyerRequest', 'seller'], // ✅ Incluir seller para notificación - }); - - if (!offer) { - throw new NotFoundException('Offer not found'); - } - - if (offer.buyerRequest.userId.toString() !== buyerId) { - throw new ForbiddenException('You are not authorized to accept this offer'); - } - - if (offer.status !== OfferStatus.PENDING) { - throw new BadRequestException(`Cannot accept an offer that is already ${offer.status}`); - } - - const alreadyAccepted = await this.offerRepository.findOne({ - where: { - buyerRequestId: offer.buyerRequestId, - status: OfferStatus.ACCEPTED, - }, - }); - - if (alreadyAccepted) { - throw new BadRequestException('Another offer has already been accepted for this request'); - } - - // ✅ 1. Aceptar la oferta actual - offer.status = OfferStatus.ACCEPTED; - const acceptedOffer = await this.offerRepository.save(offer); - - // ✅ 2. Notificar al seller que su oferta fue aceptada - await this.notifyOfferStatusChange(acceptedOffer, 'accepted'); - - // ✅ 3. Obtener todas las otras ofertas pendientes para notificarlas individualmente - const pendingOffers = await this.offerRepository.find({ - where: { - buyerRequestId: offer.buyerRequestId, - status: OfferStatus.PENDING, - }, - relations: ['buyerRequest', 'seller'], - }); - - // ✅ 4. Rechazar y notificar cada oferta pendiente individualmente - for (const pendingOffer of pendingOffers) { - pendingOffer.status = OfferStatus.REJECTED; - await this.offerRepository.save(pendingOffer); - await this.notifyOfferStatusChange(pendingOffer, 'rejected'); - } - - console.log(`Offer #${offerId} accepted by buyer #${buyerId}. ${pendingOffers.length} other pending offers rejected.`); - return acceptedOffer; - } + async accept(offerId: string, buyerId: string): Promise<Offer> { + // Perform state changes atomically + const { acceptedOffer, pendingOffers } = await this.dataSource.transaction(async (manager) => { + // Lock the target offer row to prevent concurrent updates + const offer = await manager + .getRepository(Offer) + .createQueryBuilder('offer') + .leftJoinAndSelect('offer.buyerRequest', 'buyerRequest') + .leftJoinAndSelect('offer.seller', 'seller') + .where('offer.id = :offerId', { offerId }) + .setLock('pessimistic_write') + .getOne(); + + if (!offer) { + throw new NotFoundException('Offer not found'); + } + if (offer.buyerRequest.userId.toString() !== buyerId) { + throw new ForbiddenException('You are not authorized to accept this offer'); + } + if (offer.status !== OfferStatus.PENDING) { + throw new BadRequestException(`Cannot accept an offer that is already ${offer.status}`); + } + + // Guard at DB-level as well (see migration suggestion below) + const acceptedOfferCount = await manager + .getRepository(Offer) + .count({ where: { buyerRequestId: offer.buyerRequestId, status: OfferStatus.ACCEPTED } }); + if (acceptedOfferCount > 0) { + throw new BadRequestException('Another offer has already been accepted for this request'); + } + + offer.status = OfferStatus.ACCEPTED; + const savedAccepted = await manager.save(Offer, offer); + + const pendingOffers = await manager.getRepository(Offer).find({ + where: { buyerRequestId: offer.buyerRequestId, status: OfferStatus.PENDING }, + relations: ['buyerRequest', 'seller'], + lock: { mode: 'pessimistic_write' } as any, + }); + + for (const p of pendingOffers) { + p.status = OfferStatus.REJECTED; + await manager.save(Offer, p); + } + + return { acceptedOffer: savedAccepted, pendingOffers }; + }); + + // Send notifications after transaction commits + await this.notifyOfferStatusChange(acceptedOffer, 'accepted'); + for (const pendingOffer of pendingOffers) { + await this.notifyOfferStatusChange(pendingOffer, 'rejected'); + } + + console.log( + `Offer #${offerId} accepted by buyer #${buyerId}. ${pendingOffers.length} other pending offers rejected.`, + ); + return acceptedOffer; + }Additionally, add a partial unique index to the offers table to hard-prevent multiple ACCEPTED per buyerRequest (PostgreSQL):
CREATE UNIQUE INDEX IF NOT EXISTS ux_offers_buyerRequest_accepted ON "offers" ("buyerRequestId") WHERE status = 'ACCEPTED';And handle unique-violation (23505) on save to surface a friendly error if a race still occurs.
Also consider an outbox or “notify-after-commit” mechanism to avoid sending notifications if the transaction rolls back. The above refactor sends them post-commit.
Also applies to: 96-111
| ```sql | ||
| CREATE TABLE users ( | ||
| id SERIAL PRIMARY KEY, | ||
| email VARCHAR UNIQUE, | ||
| name VARCHAR, | ||
| wallet_address VARCHAR UNIQUE NOT NULL, | ||
| location VARCHAR, -- NEW FIELD | ||
| country VARCHAR, -- NEW FIELD | ||
| buyer_data JSONB, -- NEW FIELD | ||
| seller_data JSONB, -- NEW FIELD | ||
| created_at TIMESTAMP DEFAULT NOW(), | ||
| updated_at TIMESTAMP DEFAULT NOW() | ||
| ); | ||
| ``` |
There was a problem hiding this comment.
Column naming inconsistency: buyer_data/seller_data vs buyerData/sellerData.
The schema block uses snake_case, but the migration later (and code) use camelCase. This will confuse implementers.
Apply this diff to align with camelCase (matching TypeORM columns mentioned later):
- buyer_data JSONB, -- NEW FIELD
- seller_data JSONB, -- NEW FIELD
+ "buyerData" JSONB, -- NEW FIELD
+ "sellerData" JSONB, -- NEW FIELDAlso wrap column names in quotes to reflect actual Postgres identifiers if using camelCase.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ```sql | |
| CREATE TABLE users ( | |
| id SERIAL PRIMARY KEY, | |
| email VARCHAR UNIQUE, | |
| name VARCHAR, | |
| wallet_address VARCHAR UNIQUE NOT NULL, | |
| location VARCHAR, -- NEW FIELD | |
| country VARCHAR, -- NEW FIELD | |
| buyer_data JSONB, -- NEW FIELD | |
| seller_data JSONB, -- NEW FIELD | |
| created_at TIMESTAMP DEFAULT NOW(), | |
| updated_at TIMESTAMP DEFAULT NOW() | |
| ); | |
| ``` | |
| CREATE TABLE users ( | |
| id SERIAL PRIMARY KEY, | |
| email VARCHAR UNIQUE, | |
| name VARCHAR, | |
| wallet_address VARCHAR UNIQUE NOT NULL, | |
| location VARCHAR, -- NEW FIELD | |
| country VARCHAR, -- NEW FIELD | |
| "buyerData" JSONB, -- NEW FIELD | |
| "sellerData" JSONB, -- NEW FIELD | |
| created_at TIMESTAMP DEFAULT NOW(), | |
| updated_at TIMESTAMP DEFAULT NOW() | |
| ); |
🤖 Prompt for AI Agents
In docs/IMPLEMENTATION_GUIDE.md around lines 49 to 62, the SQL schema uses
snake_case column names (buyer_data/seller_data) but the migrations and code use
camelCase; update the CREATE TABLE block to use camelCase identifiers (e.g.,
walletAddress, location, country, buyerData, sellerData, createdAt, updatedAt)
and wrap each camelCase column name in double quotes to reflect actual Postgres
identifiers; preserve column types, constraints (e.g., UNIQUE, NOT NULL) and
defaults exactly as before while only renaming and quoting the columns to match
TypeORM usage.
| **Endpoint:** `PUT /api/v1/users/update/:id` | ||
|
|
||
| **Request Body:** Same as registration but all fields optional | ||
|
|
||
| ### User Retrieval | ||
|
|
||
| **Endpoint:** `GET /api/v1/users/:id` | ||
|
|
||
| **Response:** Same structure as registration response | ||
|
|
There was a problem hiding this comment.
Endpoint examples conflict with the UUID/walletAddress migration.
This section still shows numeric id paths. Per UUID-MIGRATION.md, public endpoints should use walletAddress.
Apply this diff:
-**Endpoint:** `PUT /api/v1/users/update/:id`
+**Endpoint:** `PUT /api/v1/users/update/:walletAddress`
@@
-**Endpoint:** `GET /api/v1/users/:id`
+**Endpoint:** `GET /api/v1/users/:walletAddress`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **Endpoint:** `PUT /api/v1/users/update/:id` | |
| **Request Body:** Same as registration but all fields optional | |
| ### User Retrieval | |
| **Endpoint:** `GET /api/v1/users/:id` | |
| **Response:** Same structure as registration response | |
| **Endpoint:** `PUT /api/v1/users/update/:walletAddress` | |
| **Request Body:** Same as registration but all fields optional | |
| ### User Retrieval | |
| **Endpoint:** `GET /api/v1/users/:walletAddress` | |
| **Response:** Same structure as registration response |
🤖 Prompt for AI Agents
In docs/IMPLEMENTATION_GUIDE.md around lines 126 to 135, update the endpoint
path examples to use walletAddress (per UUID-MIGRATION.md) instead of numeric
id; replace `:id` with `:walletAddress` in the PUT and GET examples (and adjust
the PUT path to match the project's convention for updates if different, e.g.,
`PUT /api/v1/users/:walletAddress` or keep `PUT
/api/v1/users/update/:walletAddress`), and ensure the request/response
descriptions still apply to the walletAddress-based endpoints.
| ```typescript | ||
| @Entity('users') | ||
| export class User { | ||
| @PrimaryGeneratedColumn() | ||
| id: number; | ||
|
|
||
| @Column({ unique: true, nullable: true }) | ||
| email?: string; | ||
|
|
||
| @Column({ nullable: true }) | ||
| name?: string; | ||
|
|
||
| @Column({ unique: true }) | ||
| walletAddress: string; | ||
|
|
There was a problem hiding this comment.
Update the User entity snippet to reflect UUID id and jsonb fields.
The spec still shows a numeric id and json columns, which diverge from the code changes.
Apply:
-@Entity('users')
-export class User {
- @PrimaryGeneratedColumn()
- id: number;
+@Entity('users')
+export class User {
+ @PrimaryGeneratedColumn('uuid')
+ id: string;
@@
- @Column({ unique: true })
- walletAddress: string;
+ @Column({ unique: true })
+ walletAddress: string;
@@
- @Column({ type: 'json', nullable: true })
+ @Column({ type: 'jsonb', nullable: true })
buyerData?: any;
@@
- @Column({ type: 'json', nullable: true })
+ @Column({ type: 'jsonb', nullable: true })
sellerData?: any;(Optional: You can annotate walletAddress with @Index or rely solely on unique.)
Also applies to: 98-103
| ```typescript | ||
| interface UserResponse { | ||
| id: number; | ||
| walletAddress: string; | ||
| name: string; | ||
| email: string; | ||
| role: string; | ||
| location?: string; | ||
| country?: string; | ||
| buyerData?: any; | ||
| sellerData?: any; | ||
| createdAt?: Date; | ||
| updatedAt?: Date; | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Align UserResponse to public contract (omit internal id or make it a UUID string).
Auth controller changes mention omitting id from public responses. The spec’s UserResponse with id: number contradicts that and the UUID migration.
Apply one of:
Option A (omit id):
-interface UserResponse {
- id: number;
+interface UserResponse {
walletAddress: string;
name: string;
email: string;
- role: string;
+ role: string;
location?: string;
country?: string;
buyerData?: any;
sellerData?: any;
createdAt?: Date;
updatedAt?: Date;
}Option B (UUID string):
- id: number;
+ id: string;Also confirm whether role is singular or plural (array) in public responses.
Committable suggestion skipped: line range outside the PR's diff.
| ### Register as a Buyer | ||
| ```json | ||
| { | ||
| "walletAddress": "GABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890123456789012345678901234567890", |
There was a problem hiding this comment.
Fix invalid Stellar wallet examples (length must be exactly 56).
Both example walletAddress values exceed 56 chars. Replace with valid-length placeholders.
Apply this diff:
- "walletAddress": "GABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890123456789012345678901234567890",
+ "walletAddress": "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",- "walletAddress": "GXYZABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890123456789012345678901234567890",
+ "walletAddress": "GBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB",Also applies to: 48-48
🤖 Prompt for AI Agents
In docs/user-registration.md around lines 35 and 48, the example Stellar
walletAddress values are invalid because they exceed the required 56-character
length; replace each oversized placeholder with a valid 56-character Stellar
public key placeholder (e.g. begin with 'G' and total length of 56 characters)
so both examples conform to Stellar's exact-length requirement.
| @PrimaryGeneratedColumn('uuid') | ||
| id: string; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
UUID migration ripple: verify all FKs to users are updated (e.g., sellerId in offers/stores).
User.id is now a UUID string. Ensure all related entities (Store.sellerId, Offer.sellerId, Product.sellerId, etc.) use compatible UUID types and corresponding migrations update FK columns to uuid. Current snippets show number types, which would break relations.
Run this script to locate likely mismatches:
This will surface spots that need to be converted to uuid and corresponding migrations updated.
Also applies to: 55-56
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "FKs with seller_id expecting number:"
rg -nP --type=ts -C2 "@Column\([^)]*name:\s*'seller_id'[^)]*\)\s*\n\s*sellerId:\s*number;"
echo
echo "FKs with user_id expecting number:"
rg -nP --type=ts -C2 "@Column\([^)]*name:\s*'user_id'[^)]*\)\s*\n\s*user_id:\s*number;"
echo
echo "Generic userId:number fields:"
rg -nP --type=ts -C3 '\buserId:\s*number;'
echo
echo "Entities referencing User with potential seller_id join:"
rg -nP --type=ts -C3 '@ManyToOne\(\s*\(\)\s*=>\s*User\b.*?\)\s*[\s\S]*?@JoinColumn\(\s*{[^}]*name:\s*[\'"]seller_id[\'"]\s*}\s*\)'Length of output: 201
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Fields with *Id typed as number:"
rg -nP --type=ts '\b\w+Id\s*:\s*number;' -C2
echo
echo "Columns defined with type 'int' or 'integer':"
rg -nP --type=ts "@Column\(\s*\{[^}]*type:\s*['\"](?:int|integer)['\"][^}]*\}\)" -C2
echo
echo "ManyToOne relations pointing to User:"
rg -nP --type=ts '@ManyToOne\(\s*\(\)\s*=>\s*User\b' -C3
echo
echo "JoinColumn definitions using seller_id:"
rg -nP --type=ts "@JoinColumn\(\s*\{[^}]*name:\s*['\"]seller_id['\"]" -C3
echo
echo "JoinColumn definitions using user_id:"
rg -nP --type=ts "@JoinColumn\(\s*\{[^}]*name:\s*['\"]user_id['\"]" -C3Length of output: 16292
Ensure all User FK columns use UUID (string) types
The user’s primary key has switched to a UUID, but we still have several relations and DTOs typed as number. These must be updated in both the entity definitions and any related DTOs/services, plus corresponding migrations to alter the DB columns.
Files needing critical fixes:
-
src/modules/stores/entities/store.entity.ts
• ChangesellerId: number→sellerId: string
• Update@Column()to@Column({ name: 'seller_id', type: 'uuid' }) -
src/modules/products/entities/product.entity.ts
• ChangesellerId: number→sellerId: string
• Update@Column({ name: 'seller_id' })to includetype: 'uuid' -
src/modules/offers/entities/offer.entity.ts
• ChangesellerId: number→sellerId: string
• Update@Column({ name: 'seller_id' })to includetype: 'uuid' -
src/modules/stores/dto/store.dto.ts
• Change@ApiProperty() sellerId: number→string -
src/modules/products/services/product.service.ts
• Update any method signatures and usages taking/returningsellerId: numbertostring -
src/modules/cart/entities/cart.entity.ts
• Make userId column explicit UUID:
@Column({ name: 'user_id', type: 'uuid' }) userId: string; -
src/modules/auth/entities/user-role.entity.ts
• Ensure the join columnuserIdis created as UUID in migration (no explicit TS property to change) -
src/modules/buyer-requests/entities/buyer-request.entity.ts
• Ensure the join columnuserIdis created as UUID in migration (no explicit TS property)
Plus:
• Add/adjust TypeORM migrations to ALTER these FK columns from integer to UUID.
• Verify any remaining *Id: number patterns referencing users.
🤖 Prompt for AI Agents
In src/modules/users/entities/user.entity.ts around lines 18-20 the PK has been
changed to UUID but several related entities, DTOs, services and DB columns
still use number types; update the following: in
src/modules/stores/entities/store.entity.ts change sellerId: number → sellerId:
string and update @Column() to @Column({ name: 'seller_id', type: 'uuid' }); in
src/modules/products/entities/product.entity.ts change sellerId: number →
sellerId: string and add type: 'uuid' to the seller_id column decorator; in
src/modules/offers/entities/offer.entity.ts change sellerId: number → sellerId:
string and add type: 'uuid' to the seller_id column decorator; in
src/modules/stores/dto/store.dto.ts change ApiProperty() sellerId: number →
string; in src/modules/products/services/product.service.ts update any method
signatures and usages that accept/return sellerId:number to use string; in
src/modules/cart/entities/cart.entity.ts make userId explicit UUID via @Column({
name: 'user_id', type: 'uuid' }) userId: string; ensure join-columns for userId
in auth/user-role and buyer-requests are created as UUIDs in migrations;
add/adjust TypeORM migrations to ALTER existing FK columns from integer to uuid
(including updating constraints and foreign keys), run/find-and-replace for any
remaining "*Id: number" user-related patterns, and update related tests/types to
reflect string UUIDs.
| @Column({ unique: true }) | ||
| @Index() | ||
| walletAddress: string; | ||
|
|
||
| @Column({ nullable: true }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid redundant index on walletAddress (unique already creates an index).
@column({ unique: true }) adds a unique index. The additional @Index() creates a second index on the same column, wasting resources and potentially causing migration noise.
- @Column({ unique: true })
- @Index()
+ @Column({ unique: true })
walletAddress: string;If you need a non-unique index instead of unique, drop unique: true and keep @Index({ unique: false }). Otherwise, keep only unique.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Column({ unique: true }) | |
| @Index() | |
| walletAddress: string; | |
| @Column({ nullable: true }) | |
| @Column({ unique: true }) | |
| walletAddress: string; | |
| @Column({ nullable: true }) |
🤖 Prompt for AI Agents
In src/modules/users/entities/user.entity.ts around lines 27 to 31, there is a
redundant @Index() on walletAddress because @Column({ unique: true }) already
creates a unique index; remove the standalone @Index() decorator to avoid
duplicate indexes and migration noise, or if you intended a non-unique index
instead, change to @Index({ unique: false }) and remove unique: true from the
@Column — pick one approach and apply that single change so only one index is
created.
| @Column({ type: 'json', nullable: true }) | ||
| buyerData?: any; | ||
|
|
||
| @Column({ type: 'json', nullable: true }) | ||
| sellerData?: any; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use jsonb instead of json for Postgres (matches docs and improves performance).
The docs and migrations reference JSONB. Using jsonb enables indexing and better operations in Postgres.
- @Column({ type: 'json', nullable: true })
+ @Column({ type: 'jsonb', nullable: true })
buyerData?: any;
- @Column({ type: 'json', nullable: true })
+ @Column({ type: 'jsonb', nullable: true })
sellerData?: any;🤖 Prompt for AI Agents
In src/modules/users/entities/user.entity.ts around lines 37 to 42, the entity
columns are defined with type: 'json' but Postgres migrations/docs use JSONB;
update both @Column decorators to use type: 'jsonb' (e.g., @Column({ type:
'jsonb', nullable: true })) for buyerData and sellerData so the entity matches
migrations and enables Postgres JSONB features and indexing.
| location?: string; | ||
| country?: string; | ||
| buyerData?: any; | ||
| sellerData?: any; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid any for buyerData/sellerData; prefer structured, safer types
Use a more precise type to retain type-safety.
Apply:
- location?: string;
- country?: string;
- buyerData?: any;
- sellerData?: any;
+ location?: string;
+ country?: string;
+ buyerData?: Record<string, unknown>;
+ sellerData?: Record<string, unknown>;Additionally, the codebase is migrating to UUIDs. Consider narrowing id to string to prevent type drift.
Outside this hunk, a possible target shape:
export interface AuthRequest extends Request {
user: {
id: string; // UUID
walletAddress: string;
name?: string;
email?: string;
role: Role[];
location?: string;
country?: string;
buyerData?: Record<string, unknown>;
sellerData?: Record<string, unknown>;
createdAt?: Date;
updatedAt?: Date;
};
}Check for duplicate AuthRequest/AuthenticatedRequest definitions to avoid divergence and centralize the type:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all AuthRequest/AuthenticatedRequest interface definitions that extend Request
rg -nP -C2 --type=ts 'export\s+interface\s+(AuthRequest|AuthenticatedRequest)\s+extends\s+Request\b'
# Also inspect express ambient augmentations
rg -nP -C2 --type=ts 'declare\s+namespace\s+Express' Length of output: 1653
Consolidate AuthRequest definitions and tighten types for buyerData/sellerData
It looks like you’re defining AuthRequest/AuthenticatedRequest in multiple places with diverging shapes. Let’s centralize this and enforce safer typing:
• Replace the loose any on buyerData/sellerData with a structured type.
- buyerData?: any;
- sellerData?: any;
+ buyerData?: Record<string, unknown>;
+ sellerData?: Record<string, unknown>;• Narrow id to string (UUID) everywhere instead of string | number.
- id: string | number;
+ id: string; // UUID• Consolidate into one shared interface (e.g. src/types/auth-request.type.ts) and import it rather than redefining. You currently have duplicates here:
- src/types/auth-request.type.ts
- src/modules/shared/types/auth-request.type.ts
- src/modules/shared/middleware/auth.middleware.ts
- src/modules/wishlist/common/types/auth-request.type.ts
Please remove the module-local definitions and point all usages to the single, canonical type. This will keep your shape consistent and preserve type-safety across the codebase.
🤖 Prompt for AI Agents
In src/modules/wishlist/common/types/auth-request.type.ts around lines 11 to 14,
the file uses loose local definitions (buyerData/sellerData as any and id as
string|number) that diverge from other auth-request types; remove this
module-local type and import the single canonical
AuthRequest/AuthenticatedRequest from src/types/auth-request.type.ts, update
buyerData and sellerData to use the structured types defined in the canonical
file (not any), and ensure id is typed strictly as string (UUID) everywhere;
then update any local imports/usages to reference the shared type and delete
duplicate type files across the listed paths.
| id: string; // UUID | ||
| walletAddress: string; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify there are no remaining numeric ID assumptions
Ensure no code still treats user IDs as numbers (e.g., parseInt(id), numeric comparisons, or DTOs with number IDs).
Run this quick scan to surface common leftovers:
Also applies to: 16-16
🏁 Script executed:
#!/bin/bash
# Find parseInt applied to identifiers named id
rg -nP -C2 --type=ts '\bparseInt\s*\(\s*[^)]*\bid\b[^)]*\)'
# Find DTO/interface fields still typed as number for user IDs
rg -nP -C2 --type=ts '\b(userId|user\.id)\s*:\s*number\b'
# Find repository lookups that still coerce ID to number
rg -nP -C2 --type=ts 'where:\s*\{\s*id:\s*(Number|parseInt|Math\.floor)\s*\('Length of output: 7266
Remove remaining numeric user ID assumptions
Several places still treat user IDs as numbers, despite migrating to string (UUID) IDs. Please refactor the following to use string IDs throughout:
• src/modules/auth/middleware/jwt-auth.middleware.ts
– Replace
where: { id: parseInt(decoded.id) }
with
where: { id: decoded.id }
• src/modules/shared/services/role.service.ts
– Change method signatures from
removeRoleFromUser(userId: number, roleId: number)
hasRole(userId: number, …)
hasAnyRole(userId: number, …)
to
removeRoleFromUser(userId: string, roleId: number)
etc.
• src/modules/auth/services/auth.service.ts
– Update
async updateUserById(userId: number, …)
to
async updateUserById(userId: string, …)
• src/modules/offers/services/offers.service.ts
– Adjust
async update(id: string, …, userId: number)
async remove(id: string, userId: number)
to use userId: string
• src/modules/offers/services/offer-attachment.service.ts
– Convert all userId: number parameters to userId: string
Refactoring these will ensure consistency and avoid runtime errors when handling UUIDs.
🤖 Prompt for AI Agents
In src/types/auth-request.type.ts around lines 5-6, the user ID is treated as a
numeric value in several places despite being a UUID string; update the type and
all affected code to use string IDs: change the id field to a string (UUID),
remove any parseInt/number conversions (e.g., replace where: { id:
parseInt(decoded.id) } with where: { id: decoded.id }), and update method
signatures and parameter types in the listed files to accept userId: string
(role.service methods, auth.service.updateUserById, offers.service
update/remove, offer-attachment.service) so all comparisons, database queries,
and type annotations use string IDs consistently.
|
|
||
| ## Key Features | ||
|
|
||
| - **🔄 Automatic Store Creation**: Default store created automatically when seller registers |
There was a problem hiding this comment.
Grammar: add article
“when seller registers” → “when a seller registers”.
Apply this diff:
-- **🔄 Automatic Store Creation**: Default store created automatically when seller registers
+- **🔄 Automatic Store Creation**: Default store created automatically when a seller registers📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **🔄 Automatic Store Creation**: Default store created automatically when seller registers | |
| - **🔄 Automatic Store Creation**: Default store created automatically when a seller registers |
🤖 Prompt for AI Agents
In docs/store-system.md around line 9, the phrase "when seller registers" is
missing the indefinite article; update the sentence to read "when a seller
registers" so it becomes "**🔄 Automatic Store Creation**: Default store created
automatically when a seller registers". Ensure spacing and punctuation match the
surrounding style.
| ```typescript | ||
| @Entity('stores') | ||
| export class Store { | ||
| @PrimaryGeneratedColumn() | ||
| id: number; | ||
|
|
||
| @Column() | ||
| name: string; | ||
|
|
||
| @Column({ type: 'text', nullable: true }) | ||
| description?: string; | ||
|
|
||
| @Column({ nullable: true }) | ||
| logo?: string; | ||
|
|
||
| @Column({ nullable: true }) | ||
| banner?: string; | ||
|
|
||
| @Column({ type: 'jsonb', nullable: true }) | ||
| contactInfo?: ContactInfo; | ||
|
|
||
| @Column({ type: 'jsonb', nullable: true }) | ||
| address?: Address; | ||
|
|
||
| @Column({ type: 'jsonb', nullable: true }) | ||
| businessHours?: BusinessHours; | ||
|
|
||
| @Column({ type: 'jsonb', nullable: true }) | ||
| categories?: string[]; | ||
|
|
||
| @Column({ type: 'jsonb', nullable: true }) | ||
| tags?: string[]; | ||
|
|
||
| @Column({ type: 'decimal', precision: 3, scale: 2, nullable: true }) | ||
| rating?: number; | ||
|
|
||
| @Column({ type: 'int', default: 0 }) | ||
| reviewCount: number; | ||
|
|
||
| @Column({ type: 'jsonb', nullable: true }) | ||
| policies?: Policies; | ||
|
|
||
| @Column({ type: 'jsonb', nullable: true }) | ||
| settings?: StoreSettings; | ||
|
|
||
| @Column({ | ||
| type: 'enum', | ||
| enum: StoreStatus, | ||
| default: StoreStatus.PENDING_APPROVAL, | ||
| }) | ||
| status: StoreStatus; | ||
|
|
||
| @Column({ type: 'boolean', default: false }) | ||
| isVerified: boolean; | ||
|
|
||
| @Column({ type: 'boolean', default: false }) | ||
| isFeatured: boolean; | ||
|
|
||
| @ManyToOne(() => User, (user) => user.stores) | ||
| @JoinColumn({ name: 'seller_id' }) | ||
| seller: User; | ||
|
|
||
| @Column() | ||
| sellerId: number; | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align entity sample with fields used in service and SQL schema
The service uses verifiedAt, and the SQL section includes verifiedAt, featuredAt, createdAt, and updatedAt. Add them to the entity sample for consistency.
Apply this diff inside the entity example:
@Column({ type: 'boolean', default: false })
isFeatured: boolean;
+ @Column({ type: 'timestamp', nullable: true })
+ verifiedAt?: Date;
+
+ @Column({ type: 'timestamp', nullable: true })
+ featuredAt?: Date;
+
+ @CreateDateColumn()
+ createdAt: Date;
+
+ @UpdateDateColumn()
+ updatedAt: Date;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ```typescript | |
| @Entity('stores') | |
| export class Store { | |
| @PrimaryGeneratedColumn() | |
| id: number; | |
| @Column() | |
| name: string; | |
| @Column({ type: 'text', nullable: true }) | |
| description?: string; | |
| @Column({ nullable: true }) | |
| logo?: string; | |
| @Column({ nullable: true }) | |
| banner?: string; | |
| @Column({ type: 'jsonb', nullable: true }) | |
| contactInfo?: ContactInfo; | |
| @Column({ type: 'jsonb', nullable: true }) | |
| address?: Address; | |
| @Column({ type: 'jsonb', nullable: true }) | |
| businessHours?: BusinessHours; | |
| @Column({ type: 'jsonb', nullable: true }) | |
| categories?: string[]; | |
| @Column({ type: 'jsonb', nullable: true }) | |
| tags?: string[]; | |
| @Column({ type: 'decimal', precision: 3, scale: 2, nullable: true }) | |
| rating?: number; | |
| @Column({ type: 'int', default: 0 }) | |
| reviewCount: number; | |
| @Column({ type: 'jsonb', nullable: true }) | |
| policies?: Policies; | |
| @Column({ type: 'jsonb', nullable: true }) | |
| settings?: StoreSettings; | |
| @Column({ | |
| type: 'enum', | |
| enum: StoreStatus, | |
| default: StoreStatus.PENDING_APPROVAL, | |
| }) | |
| status: StoreStatus; | |
| @Column({ type: 'boolean', default: false }) | |
| isVerified: boolean; | |
| @Column({ type: 'boolean', default: false }) | |
| isFeatured: boolean; | |
| @ManyToOne(() => User, (user) => user.stores) | |
| @JoinColumn({ name: 'seller_id' }) | |
| seller: User; | |
| @Column() | |
| sellerId: number; | |
| } | |
| ``` |
🤖 Prompt for AI Agents
In docs/store-system.md around lines 21 to 86, the Store entity example is
missing timestamp and flag fields used elsewhere (verifiedAt, featuredAt,
createdAt, updatedAt); update the entity sample to include these fields with
appropriate Column decorators and types: add verifiedAt and featuredAt as
nullable timestamp/Date columns, and add createdAt and updatedAt as timestamp
columns (createdAt defaulting to current timestamp and updatedAt set to update
on change or nullable if you prefer DB-managed timestamps) so the example
matches the service and SQL schema.
| async updateStore(storeId: number, sellerId: number, updateStoreDto: UpdateStoreDto): Promise<Store> { | ||
| const store = await this.storeRepository.findOne({ | ||
| where: { id: storeId, sellerId }, | ||
| }); | ||
|
|
||
| if (!store) { | ||
| throw new NotFoundException('Store not found or access denied'); | ||
| } | ||
|
|
||
| Object.assign(store, updateStoreDto); | ||
| return await this.storeRepository.save(store); | ||
| } |
There was a problem hiding this comment.
Block privilege escalation in seller update path
UpdateStoreDto includes admin-only fields (status, isVerified, isFeatured). The current Object.assign allows sellers to modify them. This is a security hole.
Apply this diff to strip restricted fields from seller updates:
- Object.assign(store, updateStoreDto);
- return await this.storeRepository.save(store);
+ // Strip admin-only fields to prevent privilege escalation
+ const { status, isVerified, isFeatured, ...safeUpdates } = (updateStoreDto as any) || {};
+ Object.assign(store, safeUpdates);
+ return await this.storeRepository.save(store);Follow-up (recommended): introduce a SellerUpdateStoreDto that excludes admin-only fields, and reserve UpdateStoreDto for admin flows. I can draft that DTO if helpful.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async updateStore(storeId: number, sellerId: number, updateStoreDto: UpdateStoreDto): Promise<Store> { | |
| const store = await this.storeRepository.findOne({ | |
| where: { id: storeId, sellerId }, | |
| }); | |
| if (!store) { | |
| throw new NotFoundException('Store not found or access denied'); | |
| } | |
| Object.assign(store, updateStoreDto); | |
| return await this.storeRepository.save(store); | |
| } | |
| async updateStore(storeId: number, sellerId: number, updateStoreDto: UpdateStoreDto): Promise<Store> { | |
| const store = await this.storeRepository.findOne({ | |
| where: { id: storeId, sellerId }, | |
| }); | |
| if (!store) { | |
| throw new NotFoundException('Store not found or access denied'); | |
| } | |
| // Strip admin-only fields to prevent privilege escalation | |
| const { status, isVerified, isFeatured, ...safeUpdates } = (updateStoreDto as any) || {}; | |
| Object.assign(store, safeUpdates); | |
| return await this.storeRepository.save(store); | |
| } |
🤖 Prompt for AI Agents
In src/modules/stores/services/store.service.ts around lines 117 to 128, the
current Object.assign(store, updateStoreDto) allows sellers to change admin-only
fields (status, isVerified, isFeatured); to fix, before assigning/saving strip
those restricted keys from the incoming updateStoreDto when the update is
performed in the seller path (e.g., delete updateStoreDto.status,
updateStoreDto.isVerified, updateStoreDto.isFeatured or create a sanitized copy
without those fields) then assign/save the sanitized object; as a follow-up
replace seller-facing updates with a SellerUpdateStoreDto that omits admin
fields and reserve UpdateStoreDto for admin flows.
2 participants
🚀 StarShop Pull Request
Mark with an
xall the checkboxes that apply (like[x])📌 Type of Change
📝 Changes description
📸 Evidence (A photo is required as evidence)
⏰ Time spent breakdown
🌌 Comments
This implementation provides a solid foundation for real-time seller notifications. The system is designed to be scalable and includes proper error handling and deduplication mechanisms. All tests pass and the functionality has been verified with real database operations.
Ready for code review!
Thank you for contributing to StarShop, we are glad that you have chosen us as your project of choice and we hope that you continue to contribute to this great project, so that together we can make our mark at the top!
Summary by CodeRabbit
New Features
Documentation
Tests
Chores