Feat/auto expire offers#144
Conversation
|
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 store management module, offer auto-expiration lifecycle, and walletAddress/UUID migration across users and related entities. Extends user registration/profile with location/country and role-specific JSON data. Updates controllers, DTOs, services, and middleware to walletAddress-first flows. Introduces migrations for users, offers, stores, and foreign keys. Adds docs and tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as AuthController
participant Auth as AuthService
participant Users as UserRepository
participant Roles as RoleService
participant Stores as StoreService
Client->>API: POST /auth/register (walletAddress, role, ...+buyerData/sellerData)
API->>Auth: registerWithWallet(data)
Auth->>Users: find by walletAddress
alt user exists
Auth->>Users: update profile (location/country/+role-specific data)
else create new
Auth->>Users: create user (UUID id)
Auth->>Roles: assign role to user.id
opt role == seller
Auth->>Stores: createDefaultStore(user.id, sellerData)
Stores-->>Auth: store or noop
end
end
Auth-->>API: user (no id), token, expiresIn
API-->>Client: 201 Created (user fields incl. role-specific data)
sequenceDiagram
autonumber
participant Cron as OfferExpirationService (Cron)
participant OfferSvc as OfferService
participant DB as OfferRepository
participant Notify as NotificationService
Cron->>OfferSvc: handleOfferExpiration()
OfferSvc->>DB: find PENDING where expiresAt < now
alt any expired
OfferSvc->>DB: update status = REJECTED
loop each expired
OfferSvc->>Notify: emit seller notification
end
else none
OfferSvc-->>Cron: 0
end
Note over Cron,OfferSvc: Also exposed via POST /offers/expire-manual and<br/>GET /offers/expiring-soon, GET /offers/:id/expired
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
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: 53
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (13)
src/modules/auth/middleware/authorize-roles.middleware.ts (1)
2-2: Use ForbiddenException for RBAC denials; keep Unauthorized for missing authThrowing UnauthorizedException for insufficient permissions is semantically off (should be 403). Use ForbiddenException.
Apply this diff:
-import { UnauthorizedException } from '@nestjs/common'; +import { UnauthorizedException, ForbiddenException } from '@nestjs/common'; @@ - if (!hasAllowedRole) { - throw new UnauthorizedException('Insufficient permissions'); - } + if (!hasAllowedRole) { + throw new ForbiddenException('Insufficient permissions'); + }Also applies to: 15-17
src/types/express.d.ts (2)
18-35: Unify Request.user shape with Express.User to avoid drift and type mismatch (id should be string).Currently Request.user.id is typed as string | number (Line 20) while Express.User.id is string (Line 40). This inconsistency causes unnecessary type-widening and invites bugs, especially with UUID migration. Recommend making Request.user use Express.User directly and keep id as string everywhere.
Apply this diff to align Request.user with Express.User and remove the duplicated inline shape:
declare module 'express-serve-static-core' { interface Request { - user?: { - id: string | number; // Allow both string and number types - walletAddress: string; - name?: string; - email?: string; - role: Role[]; - location?: string; - country?: string; - buyerData?: any; - sellerData?: any; - createdAt?: Date; - updatedAt?: Date; - }; + user?: Express.User; fileProvider?: 'cloudinary' | 's3'; fileType?: string; } }
55-57: Consolidate duplicate AuthenticatedRequest declarationsWe found redundant interfaces in:
- src/modules/shared/types/auth-request.type.ts (lines 4–6)
- src/modules/shared/middleware/auth.middleware.ts (lines 11–13)
- src/types/express.d.ts (lines 55–57)
- src/types/auth-request.type.ts (lines 14–16)
Having multiple copies of the same interface can lead to subtle type mismatches and maintenance overhead. Please pick a single canonical definition (for example, in src/types/auth-request.type.ts), remove the others, update your Express augmentation to reference that central type, and have all modules import or re-export it as needed.
src/modules/shared/middleware/session.middleware.ts (1)
37-43: Stop usingas any; assign a fully-typed Express.User and include newly added fields.You can keep type safety and populate the extended shape. Also coerce id to string to align with UUID migration.
- // Attach user to request - req.user = { - id: user.id, - walletAddress: user.walletAddress, - role: user.userRoles.map((ur) => ur.role.name as Role), - } as any; + // Attach user to request (typed) + const reqUser: Express.User = { + id: String(user.id), + walletAddress: user.walletAddress, + name: user.name ?? undefined, + email: user.email ?? undefined, + role: user.userRoles.map((ur) => ur.role.name as Role), + location: user.location ?? undefined, + country: user.country ?? undefined, + buyerData: user.buyerData ?? undefined, + sellerData: user.sellerData ?? undefined, + createdAt: user.createdAt, + updatedAt: user.updatedAt, + }; + req.user = reqUser;src/modules/shared/services/role.service.ts (1)
56-62: UnifyuserIdtype in RoleService; remove allparseIntcallsThe
UserRole.userIdcolumn is defined as a UUID (string), but insrc/modules/shared/services/role.service.tswe’re still treating it as a number. To avoid brittle conversions:• File: src/modules/shared/services/role.service.ts
– Line 49: in your “add” method you’re calling
save({ userId: parseInt(userId), roleId: role.id })
→ change signature to(userId: string, roleId: number)and passuserIddirectly.
– Lines 56–59 (getUserRoles): removeparseInt(userId); use
{ where: { userId }, relations: ['role'] }.
– Lines 52–54 (removeRoleFromUser): change signature fromuserId: numbertouserId: stringand passuserIddirectly to.delete().• (Optionally) Add helper methods for consistency:
async hasRole(userId: string, roleName: RoleName): Promise<boolean> { return (await this.getUserRoles(userId)) .some(role => role.name === roleName); } async hasAnyRole(userId: string, roleNames: RoleName[]): Promise<boolean> { return (await this.getUserRoles(userId)) .some(role => roleNames.includes(role.name)); }• Verify that
src/modules/auth/entities/user-role.entity.tsdefines
@Column({ type: 'uuid' }) userId: string;(it does) and ensure your DB migrations align.This refactor unifies UUIDs end-to-end and removes risky integer parsing.
src/modules/shared/middleware/auth.middleware.ts (2)
32-44: Fix invalid instantiation of AuthMiddleware in requireRole (make mapper static or standalone).new AuthMiddleware(null, null) will not compile with strict types and leaks a construction dependency into a pure mapping call.
Make mapRoleToEnum static and call it without instantiation:
- public mapRoleToEnum(roleName: string): Role { + public static mapRoleToEnum(roleName: string): Role { switch (roleName.toLowerCase()) { case 'admin': return Role.ADMIN; case 'seller': return Role.SELLER; case 'buyer': case 'user': return Role.BUYER; default: return Role.BUYER; } }- return (req: AuthenticatedRequest, res: Response, next: NextFunction) => { - const requiredRole = new AuthMiddleware(null, null).mapRoleToEnum(roleName); + return (req: AuthenticatedRequest, res: Response, next: NextFunction) => { + const requiredRole = AuthMiddleware.mapRoleToEnum(roleName); if (!req.user || !req.user.role.some(role => role === requiredRole)) { throw new ReferenceError('Insufficient permissions'); } next(); };Also applies to: 80-86
84-89: Return 403 instead of throwing ReferenceError in middleware.Throwing ReferenceError can bubble as 500 and bypass Nest’s exception handling in middleware. Respond with 403 or next(new ForbiddenException()).
Option A (pure Express style):
return (req: AuthenticatedRequest, res: Response, next: NextFunction) => { const requiredRole = AuthMiddleware.mapRoleToEnum(roleName); - if (!req.user || !req.user.role.some(role => role === requiredRole)) { - throw new ReferenceError('Insufficient permissions'); - } + if (!req.user || !req.user.role.some(role => role === requiredRole)) { + return res.status(403).json({ message: 'Insufficient permissions' }); + } next(); };Option B (if you’re certain Nest will handle HttpExceptions here):
+import { ForbiddenException } from '@nestjs/common'; ... - if (!req.user || !req.user.role.some(role => role === requiredRole)) { - throw new ReferenceError('Insufficient permissions'); - } + if (!req.user || !req.user.role.some(role => role === requiredRole)) { + throw new ForbiddenException('Insufficient permissions'); + }src/modules/auth/strategies/jwt.strategy.ts (1)
47-49: Bug: Catch block masks specific UnauthorizedException messages.Any UnauthorizedException thrown above (e.g., "User not found", "Invalid token payload") gets replaced by "Invalid token", making debugging and DX worse.
Apply this diff to preserve specific UnauthorizedException while still handling unexpected errors:
- } catch (error) { - throw new UnauthorizedException('Invalid token'); - } + } catch (error) { + if (error instanceof UnauthorizedException) { + // Preserve original reason (user not found, invalid payload, etc.) + throw error; + } + throw new UnauthorizedException('Invalid token'); + }src/modules/reviews/services/review.service.ts (1)
41-47: Handle unique‐constraint violations in review creationThe unique index on (userId, productId) is already enforced at the DB level; to ensure a consistent BadRequestError under concurrent requests, catch the unique‐violation error when saving.
• Unique constraint already defined in
– src/modules/reviews/entities/review.entity.ts:
@Unique(['userId', 'productId'])
– src/migrations/1751199236860-CreateOfferTable.ts:
CONSTRAINT "UQ_9007ffba411fd471dfe233dabfb" UNIQUE ("userId", "productId")
• In src/modules/reviews/services/review.service.ts (e.g., in createReview), update the save logic:// after checking for existingReview... - await this.repository.save(review); + try { + await this.repository.save(review); + } catch (error: any) { + // Postgres unique‐violation code + if (error.code === '23505') { + throw new BadRequestError('You have already reviewed this product'); + } + throw error; + }This ensures that even if two requests race past the initial findOne check, the second will be translated into the same BadRequestError rather than a generic database exception.
src/modules/offers/services/offer.service.ts (1)
24-25: NotificationService DI will fail unless NotificationsModule is imported in OffersModule.OfferService injects NotificationService, but OffersModule doesn’t import NotificationsModule. Add NotificationsModule to OffersModule imports (unless NotificationsModule is @global and exports NotificationService).
See suggested fix in OffersModule review.
src/modules/auth/services/role.service.ts (1)
44-52: Ensure typedroleName, consistent numericuserId, and prevent duplicates inassignRoleToUser
- Change the method signature to use
RoleNameinstead ofstringforroleName.- Parse
userIdto a number (parseInt(userId, 10)) before any DB calls to match patterns inshared/services/role.service.ts.- Add a duplicate‐check with
findOneand return early if the user already has that role.--- a/src/modules/auth/services/role.service.ts @@ -44,7 +44,13 @@ - async assignRoleToUser(userId: string, roleName: string): Promise<void> { + async assignRoleToUser(userId: string, roleName: RoleName): Promise<void> { const role = await this.findByName(roleName); if (!role) { throw new Error(`Role ${roleName} not found`); } + const numericUserId = parseInt(userId, 10); + const existing = await this.userRoleRepository.findOne({ + where: { userId: numericUserId, roleId: role.id }, + }); + if (existing) return; await this.userRoleRepository.save({ - userId, + userId: numericUserId, roleId: role.id, });src/modules/buyer-requests/services/buyer-requests.service.ts (1)
100-105: Sanitize sortBy/sortOrder to avoid SQL injection in dynamic ORDER BYUsing an unvalidated sortBy in a template string for orderBy enables injection via crafted input (e.g., sortBy='createdAt DESC NULLS LAST, 1=1 --'). Whitelist fields and normalize order.
Apply this diff:
- // Apply sorting - queryBuilder.orderBy(`request.${sortBy}`, sortOrder); - if (sortBy !== 'createdAt') { + // Apply sorting (sanitize inputs) + const allowedSortFields = new Set(['createdAt', 'budgetMin', 'budgetMax', 'expiresAt', 'categoryId', 'title']); + const safeSortBy = allowedSortFields.has(sortBy) ? sortBy : 'createdAt'; + const safeSortOrder = sortOrder === 'ASC' ? 'ASC' : 'DESC'; + queryBuilder.orderBy(`request.${safeSortBy}`, safeSortOrder as 'ASC' | 'DESC'); + if (safeSortBy !== 'createdAt') { queryBuilder.addOrderBy('request.createdAt', 'DESC'); }src/modules/users/services/user.service.ts (1)
146-188: Apply the same invariants to updateUserById.Keep compatibility path consistent with walletAddress-based updates.
Apply this diff:
async updateUserById( id: string, data: { name?: string; email?: string; location?: string; country?: string; buyerData?: any; sellerData?: any; }, ): Promise<User> { const user = await this.getUserById(id); + const roleNames = (user.userRoles || []) + .map((ur) => ur.role?.name) + .filter(Boolean) as string[]; if (data.email) { const existingUser = await this.userRepository.findOne({ where: { email: data.email } }); if (existingUser && existingUser.id !== user.id) { throw new BadRequestError('Email already in use'); } user.email = data.email; } if (data.name) { user.name = data.name; } if (data.location !== undefined) { user.location = data.location; } if (data.country !== undefined) { user.country = data.country; } + if ( + data.sellerData !== undefined && + roleNames.includes('buyer') && + !roleNames.includes('seller') + ) { + throw new BadRequestError('Buyers cannot have seller data'); + } if (data.buyerData !== undefined) { user.buyerData = data.buyerData; } if (data.sellerData !== undefined) { user.sellerData = data.sellerData; } return this.userRepository.save(user); }
docs/IMPLEMENTATION_GUIDE.md
Outdated
| ```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.
Update the database schema to reflect UUID primary keys.
The SQL schema shows id SERIAL PRIMARY KEY which generates integers, but based on the entity changes, user IDs are now UUIDs.
CREATE TABLE users (
- id SERIAL PRIMARY KEY,
+ id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
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()
);Also update the user_roles table:
CREATE TABLE user_roles (
id SERIAL PRIMARY KEY,
- user_id INTEGER REFERENCES users(id),
+ user_id UUID REFERENCES users(id),
role_id INTEGER REFERENCES roles(id)
);🤖 Prompt for AI Agents
In docs/IMPLEMENTATION_GUIDE.md around lines 49 to 62, the users table uses an
integer SERIAL primary key but IDs are now UUIDs; change the schema to use a
UUID primary key with a default UUID generator (e.g., gen_random_uuid() or
uuid_generate_v4()) and note enabling the necessary extension (pgcrypto or
uuid-ossp) if not already enabled; also update the user_roles table schema so
its user_id column is UUID (not integer) and its foreign key constraint
references users(id) with matching type; keep comments for new fields and ensure
created_at/updated_at remain unchanged.
docs/IMPLEMENTATION_GUIDE.md
Outdated
| // Validate role-specific data | ||
| if (data.role === 'buyer' && data.buyerData === undefined) { | ||
| throw new BadRequestError('Buyer data is required for buyer role'); | ||
| } | ||
| if (data.role === 'seller' && data.sellerData === undefined) { | ||
| throw new BadRequestError('Seller data is required for seller role'); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Clarify role-specific data requirements.
The service layer validation shows that buyerData and sellerData are required based on role, but the entity definition marks them as nullable. This inconsistency should be clarified.
🏁 Script executed:
#!/bin/bash
# Check if there are any existing validations in DTOs that enforce these requirements
echo "=== Checking DTO validation for buyerData/sellerData requirements ==="
rg -A 5 -B 5 "buyerData|sellerData" src/modules/auth/dto/ --type ts
echo -e "\n=== Checking if service enforces these requirements ==="
rg -A 3 -B 3 "buyerData.*required|sellerData.*required" src/modules/auth/services/ --type tsLength of output: 5445
Sync role-specific data requirements between docs and code
The doc snippet in docs/IMPLEMENTATION_GUIDE.md (lines 222–228) shows service-layer checks that throw if buyerData/sellerData are missing for their respective roles, but in the actual code:
- In
src/modules/auth/dto/auth.dto.ts, both properties are marked@IsOptional()and only validated for “allowed” roles via the@IsRoleSpecificDatadecorator. - In the entity they remain nullable.
Choose one of the following to resolve the mismatch:
- Add presence checks in your service methods (e.g. in
AuthService#createUser/updateUser), throwingBadRequestErrorwhenrole==='buyer'&buyerDatais undefined (and similarly for sellers), so the docs snippet is correct. - Update the docs to describe the actual DTO-level validation (
@IsOptional()+@IsRoleSpecificData) and note that presence is not enforced at the service layer.
Please confirm which approach you’ll take and update the code or documentation accordingly.
🤖 Prompt for AI Agents
In docs/IMPLEMENTATION_GUIDE.md around lines 222–228 and corresponding service
code in src/modules/auth/service/auth.service.ts (methods createUser and
updateUser), the docs expect service-layer presence checks for role-specific
data but the code currently relies only on DTO decorators; implement explicit
presence validation in the service: in createUser and updateUser, if data.role
=== 'buyer' and data.buyerData is missing/undefined throw new
BadRequestError('Buyer data is required for buyer role'), and likewise if
data.role === 'seller' and data.sellerData is missing/undefined throw new
BadRequestError('Seller data is required for seller role'); keep existing DTO
decorators as-is and add these checks early in the methods before any
persistence or further validation.
| ### Register a Seller | ||
|
|
||
| ```bash | ||
| curl -X POST 'http://localhost:3000/api/v1/users' \ | ||
| -H 'Content-Type: application/json' \ | ||
| -d '{ | ||
| "walletAddress": "GXYZABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890123456789012345678901234567890", | ||
| "role": "seller", | ||
| "name": "Jane Smith", | ||
| "email": "jane@example.com", | ||
| "location": "Los Angeles", | ||
| "country": "United States", | ||
| "sellerData": { | ||
| "businessName": "Tech Store", | ||
| "categories": ["electronics", "computers"], | ||
| "rating": 4.5 | ||
| } | ||
| }' | ||
| ``` |
There was a problem hiding this comment.
Fix invalid walletAddress example to match documented regex
The seller walletAddress sample exceeds 56 chars and doesn’t match the stated regex ^G[A-Z2-7]{55}$. Provide a valid 56-char Stellar address to avoid confusion.
Apply this diff:
- "walletAddress": "GXYZABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890123456789012345678901234567890",
+ "walletAddress": "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",📝 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.
| ### Register a Seller | |
| ```bash | |
| curl -X POST 'http://localhost:3000/api/v1/users' \ | |
| -H 'Content-Type: application/json' \ | |
| -d '{ | |
| "walletAddress": "GXYZABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890123456789012345678901234567890", | |
| "role": "seller", | |
| "name": "Jane Smith", | |
| "email": "jane@example.com", | |
| "location": "Los Angeles", | |
| "country": "United States", | |
| "sellerData": { | |
| "businessName": "Tech Store", | |
| "categories": ["electronics", "computers"], | |
| "rating": 4.5 | |
| } | |
| }' | |
| ``` | |
| #### Register a Seller | |
🤖 Prompt for AI Agents
In docs/README.md around lines 109 to 127, the example seller walletAddress is
too long and doesn’t match the documented regex ^G[A-Z2-7]{55}$; replace the
current value with a valid 56-character Stellar public key that starts with 'G'
followed by 55 characters from the character set A-Z2-7 (for example generate or
paste a known valid Stellar address), ensuring the example exactly matches the
regex and update the curl JSON accordingly.
| - **🔄 Automatic Store Creation**: Default store created automatically when seller registers | ||
| - **🏪 Multiple Stores**: Sellers can create and manage multiple stores | ||
| - **📊 Rich Store Data**: Comprehensive store information including contact, address, policies | ||
| - **🔍 Advanced Search**: Search stores by name, category, and location | ||
| - **🛡️ Role-Based Access**: Only sellers can create and manage stores | ||
| - **📈 Store Statistics**: Performance metrics and analytics | ||
| - **✅ Admin Controls**: Store approval and status management | ||
|
|
There was a problem hiding this comment.
Polish language in Key Features (user-facing doc).
Minor grammatical fixes improve clarity.
-- **🔄 Automatic Store Creation**: Default store created automatically when seller registers
+- **🔄 Automatic Store Creation**: A default store is created automatically when a seller registers
- **🏪 Multiple Stores**: Sellers can create and manage multiple stores
-- **📊 Rich Store Data**: Comprehensive store information including contact, address, policies
+- **📊 Rich Store Data**: Comprehensive store information, including contact details, address, and policies
-- **🔍 Advanced Search**: Search stores by name, category, and location
+- **🔍 Advanced Search**: Search stores by name, category, and location
-- **🛡️ Role-Based Access**: Only sellers can create and manage stores
+- **🛡️ Role-Based Access**: Only sellers can create and manage stores
-- **📈 Store Statistics**: Performance metrics and analytics
+- **📈 Store Statistics**: Performance metrics and analytics
-- **✅ Admin Controls**: Store approval and status management
+- **✅ Admin Controls**: Store approval and status management📝 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 | |
| - **🏪 Multiple Stores**: Sellers can create and manage multiple stores | |
| - **📊 Rich Store Data**: Comprehensive store information including contact, address, policies | |
| - **🔍 Advanced Search**: Search stores by name, category, and location | |
| - **🛡️ Role-Based Access**: Only sellers can create and manage stores | |
| - **📈 Store Statistics**: Performance metrics and analytics | |
| - **✅ Admin Controls**: Store approval and status management | |
| - **🔄 Automatic Store Creation**: A default store is created automatically when a seller registers | |
| - **🏪 Multiple Stores**: Sellers can create and manage multiple stores | |
| - **📊 Rich Store Data**: Comprehensive store information, including contact details, address, and policies | |
| - **🔍 Advanced Search**: Search stores by name, category, and location | |
| - **🛡️ Role-Based Access**: Only sellers can create and manage stores | |
| - **📈 Store Statistics**: Performance metrics and analytics | |
| - **✅ Admin Controls**: Store approval and status management |
🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: There might be a mistake here.
Context: ...automatically when seller registers - 🏪 Multiple Stores: Sellers can create a...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...create and manage multiple stores - 📊 Rich Store Data: Comprehensive store i...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...nformation including contact, address, policies - 🔍 Advanced Search: Search ...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...uding contact, address, policies - 🔍 Advanced Search: Search stores by name,...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...by name, category, and location - 🛡️ Role-Based Access: Only sellers can cre...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ...s can create and manage stores - 📈 Store Statistics: Performance metrics and...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...formance metrics and analytics - ✅ Admin Controls: Store approval and status ...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In docs/store-system.md around lines 9 to 16, the Key Features list contains
minor grammatical and polish issues; update the bullet points for clearer,
user-facing wording — e.g., change "Default store created automatically when
seller registers" to "A default store is created automatically when a seller
registers", "Only sellers can create and manage stores" to "Only sellers can
create and manage their stores", "Search stores by name, category, and location"
to "Search for stores by name, category, or location", and generally ensure
consistent sentence structure, punctuation, and parallelism across all bullets.
| @PrimaryGeneratedColumn() | ||
| id: number; | ||
|
|
There was a problem hiding this comment.
Align IDs to UUID across docs to match current codebase direction.
Docs currently show numeric IDs (SERIAL, number). PR/AI context references UUID migration for users and broader entities. Recommend using UUID for Store.id and sellerId to avoid confusion and ensure consistency.
Apply these changes:
TypeScript entity:
- @PrimaryGeneratedColumn()
- id: number;
+ @PrimaryGeneratedColumn('uuid')
+ id: string;
@@
- @Column()
- sellerId: number;
+ @Column('uuid')
+ sellerId: string;SQL schema:
-CREATE TABLE stores (
- id SERIAL PRIMARY KEY,
+CREATE TABLE stores (
+ id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
@@
- sellerId INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE,
+ sellerId UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE,Note: gen_random_uuid() requires the pgcrypto extension; alternatively use uuid_generate_v4() with uuid-ossp.
Also applies to: 83-85, 363-386
| async getSellerStores(sellerId: number): Promise<Store[]> { | ||
| return await this.storeRepository.find({ | ||
| where: { sellerId }, | ||
| order: { createdAt: 'DESC' }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align getSellerStores signature with UUID sellerId
Keep sellerId typed as string.
Apply this diff:
- async getSellerStores(sellerId: number): Promise<Store[]> {
+ async getSellerStores(sellerId: string): Promise<Store[]> {📝 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 getSellerStores(sellerId: number): Promise<Store[]> { | |
| return await this.storeRepository.find({ | |
| where: { sellerId }, | |
| order: { createdAt: 'DESC' }, | |
| }); | |
| } | |
| async getSellerStores(sellerId: string): Promise<Store[]> { | |
| return await this.storeRepository.find({ | |
| where: { sellerId }, | |
| order: { createdAt: 'DESC' }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In src/modules/stores/services/store.service.ts around lines 91 to 96, the
getSellerStores method currently types sellerId as number; change the method
signature to accept sellerId: string and update the Promise generic if needed
(Promise<Store[]> remains) so the repository.find call uses the string sellerId
as-is; also scan and update any callers of getSellerStores to pass a string UUID
instead of a number.
| @PrimaryGeneratedColumn('uuid') | ||
| id: string; |
There was a problem hiding this comment.
Verify UUID type consistency across the codebase.
The id field type has been changed from number to string to support UUIDs. Based on the relevant code snippets, I can see several entities still referencing user IDs with inconsistent types.
#!/bin/bash
# Description: Check for type inconsistencies in user ID references across entities
echo "=== Checking for inconsistent user ID type references ==="
# Check Store entity - shows sellerId as number instead of string
echo "Store entity sellerId type:"
rg -n "sellerId.*number" src/modules/stores/entities/store.entity.ts
# Check Product entity - shows sellerId as number
echo -e "\nProduct entity sellerId type:"
rg -n "sellerId.*number" src/modules/products/entities/product.entity.ts
# Check Offer entity - shows sellerId as number
echo -e "\nOffer entity sellerId type:"
rg -n "sellerId.*number" src/modules/offers/entities/offer.entity.ts
# Search for any remaining numeric user ID references
echo -e "\n=== Searching for potential numeric user ID references ==="
rg -n "userId.*number|user_id.*number" --type ts -g "!*.spec.ts" -g "!*.test.ts"🤖 Prompt for AI Agents
In src/modules/users/entities/user.entity.ts around lines 18-19, the id column
was changed from number to string (UUID) but other entities still reference
numeric user IDs; update all related entities (e.g.,
src/modules/stores/entities/store.entity.ts,
src/modules/products/entities/product.entity.ts,
src/modules/offers/entities/offer.entity.ts and any other files found by the
ripgrep) to use string for sellerId/userId fields and matching TypeORM column
types (Column('uuid') or Column({ type: 'uuid' })) and ensure foreign key
relations and types on relations (ManyToOne/OneToMany) use string as the
property type so TypeScript and database mappings remain consistent across the
codebase.
| @Column({ type: 'json', nullable: true }) | ||
| buyerData?: any; | ||
|
|
||
| @Column({ type: 'json', nullable: true }) | ||
| sellerData?: any; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Define specific TypeScript interfaces for buyerData and sellerData.
Using any type for buyerData and sellerData reduces type safety. Consider defining specific interfaces for these JSON fields to ensure data consistency and enable compile-time type checking.
Create type definitions:
// In a types file or at the top of this file
interface BuyerData {
// Define buyer-specific fields
preferences?: Record<string, any>;
// Add other buyer-specific properties
}
interface SellerData {
// Define seller-specific fields
businessInfo?: Record<string, any>;
// Add other seller-specific properties
}Then update the entity:
- @Column({ type: 'json', nullable: true })
- buyerData?: any;
+ @Column({ type: 'json', nullable: true })
+ buyerData?: BuyerData;
- @Column({ type: 'json', nullable: true })
- sellerData?: any;
+ @Column({ type: 'json', nullable: true })
+ sellerData?: SellerData;🤖 Prompt for AI Agents
In src/modules/users/entities/user.entity.ts around lines 37 to 41, the
buyerData and sellerData fields are typed as any which reduces type safety;
define explicit TypeScript interfaces (e.g., BuyerData and SellerData) either at
the top of this file or in a shared types file with fields appropriate for your
domain (e.g., preferences?: Record<string, unknown>, businessInfo?:
Record<string, unknown>, plus other specific properties), then replace the any
types with these interfaces (buyerData?: BuyerData; sellerData?: SellerData) and
keep the @Column({ type: 'json', nullable: true }) decorators unchanged so the
DB mapping stays the same. Ensure to export/import the types if placed in a
separate file.
| @OneToMany(() => Store, (store) => store.seller) | ||
| stores: Store[]; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the Store entity relationship with UUID migration.
The new stores relationship references the Store entity which has a sellerId field defined as number type. This creates a type mismatch since the User id is now a UUID string.
🏁 Script executed:
#!/bin/bash
# Description: Verify Store entity's seller relationship configuration
echo "=== Checking Store entity seller relationship ==="
# Get the full Store entity to verify the seller relationship
ast-grep --pattern 'class Store {
$$$
}'
# Check if there's a migration to update Store's sellerId to UUID
echo -e "\n=== Checking for Store sellerId migration ==="
fd "UpdateForeignKeysToUUID|MigrateStoresSellerId" src/migrations --exec grep -l "sellerId"Length of output: 8349
🏁 Script executed:
#!/bin/bash
echo "=== Searching for migrations referencing seller_id or sellerId ==="
rg -n "seller_id" -g "src/migrations/*"
rg -n "sellerId" -g "src/migrations/*"
echo -e "\n=== Searching for UUID references in migrations ==="
rg -n "uuid" -g "src/migrations/*"Length of output: 9190
Store–User UUID mismatch: update stores.sellerId to UUID
The Store entity still defines
@Column()
sellerId: number;but User.id has been migrated from number → UUID. No migration alters stores.sellerId to UUID, so this foreign‐key relationship will break at runtime.
Please:
- In
src/modules/stores/entities/store.entity.tschange the type ofsellerIdfromnumber→string. - Create a new migration (e.g.
UpdateStoresSellerIdToUUID) that runs:and updates its foreign‐key constraint to referenceALTER TABLE "stores" ALTER COLUMN "sellerId" TYPE UUID USING "sellerId"::uuid;
"users"("id")as UUID. - Verify that the generated SQL is reflected in
src/migrations/*and that tests (if any) cover loading/saving stores.
🤖 Prompt for AI Agents
In src/modules/users/entities/user.entity.ts around lines 55-56: the User.id has
been migrated to UUID but stores.sellerId remains a number; update
src/modules/stores/entities/store.entity.ts to change sellerId type from number
to string (UUID), then add a new migration (e.g. UpdateStoresSellerIdToUUID)
that runs ALTER TABLE "stores" ALTER COLUMN "sellerId" TYPE UUID USING
"sellerId"::uuid; and also drops/recreates the foreign key so it references
"users"("id") as UUID; ensure the generated SQL migration is committed under
src/migrations/* and run/update tests to verify loading/saving Store entities
works with the UUID FK.
| /** | ||
| * Update user using walletAddress as primary identifier | ||
| */ | ||
| async updateUser( | ||
| walletAddress: string, | ||
| data: { | ||
| name?: string; | ||
| email?: string; | ||
| location?: string; | ||
| country?: string; | ||
| buyerData?: any; | ||
| sellerData?: any; | ||
| }, | ||
| ): Promise<User> { | ||
| const user = await this.getUserByWalletAddress(walletAddress); | ||
|
|
||
| if (data.email) { | ||
| const existingUser = await this.userRepository.findOne({ where: { email: data.email } }); | ||
| if (existingUser && existingUser.id !== user.id) { | ||
| throw new BadRequestError('Email already in use'); | ||
| } | ||
| user.email = data.email; | ||
| } | ||
|
|
||
| if (data.name) { | ||
| user.name = data.name; | ||
| } | ||
|
|
||
| if (data.location !== undefined) { | ||
| user.location = data.location; | ||
| } | ||
|
|
||
| if (data.country !== undefined) { | ||
| user.country = data.country; | ||
| } | ||
|
|
||
| if (data.buyerData !== undefined) { | ||
| user.buyerData = data.buyerData; | ||
| } | ||
|
|
||
| if (data.sellerData !== undefined) { | ||
| user.sellerData = data.sellerData; | ||
| } | ||
|
|
||
| return this.userRepository.save(user); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enforce role/data invariants on updates (mirror create-time rules).
createUser prevents buyers from having sellerData and vice versa, but updateUser currently allows setting both. This can violate domain invariants.
Apply this diff to enforce role-specific constraints using the user’s persisted roles:
async updateUser(
walletAddress: string,
data: {
name?: string;
email?: string;
location?: string;
country?: string;
buyerData?: any;
sellerData?: any;
},
): Promise<User> {
const user = await this.getUserByWalletAddress(walletAddress);
+ const roleNames = (user.userRoles || [])
+ .map((ur) => ur.role?.name)
+ .filter(Boolean) as string[];
if (data.email) {
const existingUser = await this.userRepository.findOne({ where: { email: data.email } });
if (existingUser && existingUser.id !== user.id) {
throw new BadRequestError('Email already in use');
}
user.email = data.email;
}
if (data.name) {
user.name = data.name;
}
if (data.location !== undefined) {
user.location = data.location;
}
if (data.country !== undefined) {
user.country = data.country;
}
+ // Enforce role-specific data constraints during updates
+ if (
+ data.sellerData !== undefined &&
+ roleNames.includes('buyer') &&
+ !roleNames.includes('seller')
+ ) {
+ throw new BadRequestError('Buyers cannot have seller data');
+ }
if (data.buyerData !== undefined) {
user.buyerData = data.buyerData;
}
- if (data.sellerData !== undefined) {
+ if (data.sellerData !== undefined) {
user.sellerData = data.sellerData;
}
return this.userRepository.save(user);
}Committable suggestion skipped: line range outside the PR's diff.
| async updateUserById( | ||
| userId: number, | ||
| updateData: { | ||
| name?: string; | ||
| email?: string; | ||
| location?: string; | ||
| country?: string; | ||
| buyerData?: any; | ||
| sellerData?: any; | ||
| }, | ||
| ): Promise<User> { | ||
| const user = await this.userRepository.findOne({ where: { id: userId } }); | ||
| if (!user) { | ||
| throw new BadRequestError('User not found'); | ||
| } | ||
| Object.assign(user, updateData); | ||
| await this.userRepository.save(user); | ||
| return this.getUserByWalletAddress(user.walletAddress); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
updateUserById uses numeric ID but User.id is UUID
This "compat" path will never find users in a UUID world. Either drop it or accept string IDs.
Apply this diff:
- async updateUserById(
- userId: number,
+ async updateUserById(
+ userId: string,
updateData: {
name?: string;
email?: string;
location?: string;
country?: string;
buyerData?: any;
sellerData?: any;
},
): Promise<User> {
- const user = await this.userRepository.findOne({ where: { id: userId } });
+ const user = await this.userRepository.findOne({ where: { id: userId } });
if (!user) {
throw new BadRequestError('User not found');
}
Object.assign(user, updateData);
await this.userRepository.save(user);
return this.getUserByWalletAddress(user.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.
| async updateUserById( | |
| userId: number, | |
| updateData: { | |
| name?: string; | |
| email?: string; | |
| location?: string; | |
| country?: string; | |
| buyerData?: any; | |
| sellerData?: any; | |
| }, | |
| ): Promise<User> { | |
| const user = await this.userRepository.findOne({ where: { id: userId } }); | |
| if (!user) { | |
| throw new BadRequestError('User not found'); | |
| } | |
| Object.assign(user, updateData); | |
| await this.userRepository.save(user); | |
| return this.getUserByWalletAddress(user.walletAddress); | |
| } | |
| async updateUserById( | |
| userId: string, | |
| updateData: { | |
| name?: string; | |
| email?: string; | |
| location?: string; | |
| country?: string; | |
| buyerData?: any; | |
| sellerData?: any; | |
| }, | |
| ): Promise<User> { | |
| const user = await this.userRepository.findOne({ where: { id: userId } }); | |
| if (!user) { | |
| throw new BadRequestError('User not found'); | |
| } | |
| Object.assign(user, updateData); | |
| await this.userRepository.save(user); | |
| return this.getUserByWalletAddress(user.walletAddress); | |
| } |
🤖 Prompt for AI Agents
In src/modules/auth/services/auth.service.ts around lines 243 to 261, the
updateUserById currently takes userId: number but User.id is a UUID string so
the query never matches; change the userId parameter to accept a string (or
string | number if you need backward compatibility), update the method signature
and any callers/tests accordingly, ensure the repository.findOne uses the string
id as before (where: { id: userId }), and run tests to confirm callers were
updated to pass UUID strings (or convert numeric callers to strings) so the
lookup succeeds.
| import { ApiTags, ApiOperation, ApiResponse, ApiBearerAuth } from '@nestjs/swagger'; | ||
| import { StoreService } from '../services/store.service'; | ||
| import { CreateStoreDto, UpdateStoreDto, StoreResponseDto } from '../dto/store.dto'; | ||
| import { JwtAuthGuard } from '../../auth/guards/jwt-auth.guard'; | ||
| import { RolesGuard } from '../../auth/guards/roles.guard'; | ||
| import { Roles } from '../../auth/decorators/roles.decorator'; | ||
| import { Role } from '../../../types/role'; | ||
| import { AuthenticatedRequest } from '../../../types/auth-request.type'; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Import StoreStatus and ParseEnumPipe for proper status validation
To validate status at runtime and avoid string casts, import StoreStatus and ParseEnumPipe.
Apply this diff:
-import { ApiTags, ApiOperation, ApiResponse, ApiBearerAuth } from '@nestjs/swagger';
+import { ApiTags, ApiOperation, ApiResponse, ApiBearerAuth } from '@nestjs/swagger';
+import { ParseEnumPipe } from '@nestjs/common';
+import { StoreStatus } from '../entities/store.entity';📝 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.
| import { ApiTags, ApiOperation, ApiResponse, ApiBearerAuth } from '@nestjs/swagger'; | |
| import { StoreService } from '../services/store.service'; | |
| import { CreateStoreDto, UpdateStoreDto, StoreResponseDto } from '../dto/store.dto'; | |
| import { JwtAuthGuard } from '../../auth/guards/jwt-auth.guard'; | |
| import { RolesGuard } from '../../auth/guards/roles.guard'; | |
| import { Roles } from '../../auth/decorators/roles.decorator'; | |
| import { Role } from '../../../types/role'; | |
| import { AuthenticatedRequest } from '../../../types/auth-request.type'; | |
| import { ApiTags, ApiOperation, ApiResponse, ApiBearerAuth } from '@nestjs/swagger'; | |
| import { ParseEnumPipe } from '@nestjs/common'; | |
| import { StoreStatus } from '../entities/store.entity'; | |
| import { StoreService } from '../services/store.service'; | |
| import { CreateStoreDto, UpdateStoreDto, StoreResponseDto } from '../dto/store.dto'; | |
| import { JwtAuthGuard } from '../../auth/guards/jwt-auth.guard'; | |
| import { RolesGuard } from '../../auth/guards/roles.guard'; | |
| import { Roles } from '../../auth/decorators/roles.decorator'; | |
| import { Role } from '../../../types/role'; | |
| import { AuthenticatedRequest } from '../../../types/auth-request.type'; |
🤖 Prompt for AI Agents
In src/modules/stores/controllers/store.controller.ts around lines 16 to 24, the
controller is missing imports for StoreStatus and Nest's ParseEnumPipe needed
for runtime enum validation; add an import for StoreStatus from the store DTO
module (where StoreStatus is exported, e.g., ../dto/store.dto) and import
ParseEnumPipe from '@nestjs/common', then use ParseEnumPipe(StoreStatus)
wherever status query/body validation occurs to avoid string casts and ensure
proper enum validation at runtime.
| const sellerId = req.user.id as number; | ||
| const store = await this.storeService.createStore(sellerId, createStoreDto); | ||
|
|
There was a problem hiding this comment.
sellerId is a UUID (string), not a number
Casting to number is incorrect and misleading. Use string.
Apply this diff:
- const sellerId = req.user.id as number;
+ const sellerId = req.user.id as string;📝 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.
| const sellerId = req.user.id as number; | |
| const store = await this.storeService.createStore(sellerId, createStoreDto); | |
| const sellerId = req.user.id as string; | |
| const store = await this.storeService.createStore(sellerId, createStoreDto); |
🤖 Prompt for AI Agents
In src/modules/stores/controllers/store.controller.ts around lines 45 to 47, the
code casts req.user.id to a number but sellerId is actually a UUID string;
change the cast to string (e.g., const sellerId = req.user.id as string or
String(req.user.id)) and pass that string to this.storeService.createStore; also
update storeService.createStore’s parameter type/signature (and any downstream
usages) to accept a string sellerId instead of number to keep types consistent.
| const sellerId = req.user.id as number; | ||
| const stores = await this.storeService.getSellerStores(sellerId); | ||
|
|
There was a problem hiding this comment.
Same: use string for sellerId
Apply this diff:
- const sellerId = req.user.id as number;
+ const sellerId = req.user.id as string;📝 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.
| const sellerId = req.user.id as number; | |
| const stores = await this.storeService.getSellerStores(sellerId); | |
| const sellerId = req.user.id as string; | |
| const stores = await this.storeService.getSellerStores(sellerId); |
🤖 Prompt for AI Agents
In src/modules/stores/controllers/store.controller.ts around lines 67 to 69, the
code casts req.user.id to a number for sellerId but the service expects a
string; change the sellerId to use the string form of the id (e.g., obtain
req.user.id as a string or call toString()) and pass that string into
this.storeService.getSellerStores; ensure any downstream types or method
signatures align with string id usage.
| const sellerId = req.user.id as number; | ||
| const store = await this.storeService.updateStore(id, sellerId, updateStoreDto); | ||
|
|
There was a problem hiding this comment.
Same: use string for sellerId in update
Apply this diff:
- const sellerId = req.user.id as number;
+ const sellerId = req.user.id as string;📝 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.
| const sellerId = req.user.id as number; | |
| const store = await this.storeService.updateStore(id, sellerId, updateStoreDto); | |
| const sellerId = req.user.id as string; | |
| const store = await this.storeService.updateStore(id, sellerId, updateStoreDto); |
🤖 Prompt for AI Agents
In src/modules/stores/controllers/store.controller.ts around lines 109 to 111,
the controller passes sellerId as a number to updateStore but the service
expects a string; change how sellerId is derived to convert req.user.id to a
string (e.g., use String(req.user.id) or req.user.id.toString()) and pass that
string value into this.storeService.updateStore(id, sellerId, updateStoreDto).
| async createStore(sellerId: number, createStoreDto: CreateStoreDto): Promise<Store> { | ||
| const seller = await this.userRepository.findOne({ | ||
| where: { id: sellerId }, | ||
| relations: ['userRoles'], | ||
| }); | ||
|
|
||
| if (!seller) { | ||
| throw new NotFoundException('Seller not found'); | ||
| } | ||
|
|
||
| // Check if seller has seller role | ||
| const hasSellerRole = seller.userRoles.some(ur => ur.role.name === 'seller'); | ||
| if (!hasSellerRole) { | ||
| throw new BadRequestException('Only sellers can create stores'); | ||
| } | ||
|
|
||
| const store = this.storeRepository.create({ | ||
| ...createStoreDto, | ||
| sellerId, | ||
| status: StoreStatus.PENDING_APPROVAL, | ||
| }); | ||
|
|
||
| return await this.storeRepository.save(store); | ||
| } |
There was a problem hiding this comment.
Seller role check will always fail; missing nested relation 'userRoles.role'
You query only userRoles, but then check ur.role.name. Without loading the nested role relation, ur.role is undefined. Also, update sellerId parameter to string to align with UUIDs.
Apply this diff:
- async createStore(sellerId: number, createStoreDto: CreateStoreDto): Promise<Store> {
+ async createStore(sellerId: string, createStoreDto: CreateStoreDto): Promise<Store> {
const seller = await this.userRepository.findOne({
where: { id: sellerId },
- relations: ['userRoles'],
+ relations: ['userRoles', 'userRoles.role'],
});
@@
- const hasSellerRole = seller.userRoles.some(ur => ur.role.name === 'seller');
+ const hasSellerRole = seller.userRoles?.some((ur) => ur.role?.name === 'seller');📝 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 createStore(sellerId: number, createStoreDto: CreateStoreDto): Promise<Store> { | |
| const seller = await this.userRepository.findOne({ | |
| where: { id: sellerId }, | |
| relations: ['userRoles'], | |
| }); | |
| if (!seller) { | |
| throw new NotFoundException('Seller not found'); | |
| } | |
| // Check if seller has seller role | |
| const hasSellerRole = seller.userRoles.some(ur => ur.role.name === 'seller'); | |
| if (!hasSellerRole) { | |
| throw new BadRequestException('Only sellers can create stores'); | |
| } | |
| const store = this.storeRepository.create({ | |
| ...createStoreDto, | |
| sellerId, | |
| status: StoreStatus.PENDING_APPROVAL, | |
| }); | |
| return await this.storeRepository.save(store); | |
| } | |
| async createStore(sellerId: string, createStoreDto: CreateStoreDto): Promise<Store> { | |
| const seller = await this.userRepository.findOne({ | |
| where: { id: sellerId }, | |
| relations: ['userRoles', 'userRoles.role'], | |
| }); | |
| if (!seller) { | |
| throw new NotFoundException('Seller not found'); | |
| } | |
| // Check if seller has seller role | |
| const hasSellerRole = seller.userRoles?.some((ur) => ur.role?.name === 'seller'); | |
| if (!hasSellerRole) { | |
| throw new BadRequestException('Only sellers can create stores'); | |
| } | |
| const store = this.storeRepository.create({ | |
| ...createStoreDto, | |
| sellerId, | |
| status: StoreStatus.PENDING_APPROVAL, | |
| }); | |
| return await this.storeRepository.save(store); | |
| } |
🤖 Prompt for AI Agents
In src/modules/stores/services/store.service.ts around lines 63 to 86, the
seller role check fails because the nested relation userRoles.role is not loaded
and sellerId is typed as number while we use UUIDs; update the
userRepository.findOne call to include the nested relation (e.g., relations:
['userRoles', 'userRoles.role']) so ur.role.name is defined, change the
createStore method signature and any internal usage to accept sellerId: string
(UUID) instead of number, and ensure the created store assigns that string
sellerId and any callers are updated to pass a string.
| 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.
Prevent sellers from elevating privileges via UpdateStore; whitelist fields and fix sellerId type
Object.assign(store, updateStoreDto) lets sellers update admin-only fields (status, isVerified, verifiedAt, etc.). Whitelist allowed fields for seller updates, and switch sellerId to string.
Apply this diff:
- async updateStore(storeId: number, sellerId: number, updateStoreDto: UpdateStoreDto): Promise<Store> {
+ async updateStore(storeId: number, sellerId: string, updateStoreDto: UpdateStoreDto): Promise<Store> {
@@
- Object.assign(store, updateStoreDto);
- return await this.storeRepository.save(store);
+ // Whitelist seller-updatable fields only
+ const {
+ status, // ignored (admin only)
+ isVerified, // ignored
+ isFeatured, // ignored
+ verifiedAt, // ignored
+ featuredAt, // ignored
+ sellerId: _ignoredSellerId, // never allow changing owner
+ ...safeUpdates
+ } = updateStoreDto as any;
+
+ Object.assign(store, safeUpdates);
+ return this.storeRepository.save(store);📝 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: string, 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'); | |
| } | |
| // Whitelist seller-updatable fields only | |
| const { | |
| status, // ignored (admin only) | |
| isVerified, // ignored | |
| isFeatured, // ignored | |
| verifiedAt, // ignored | |
| featuredAt, // ignored | |
| sellerId: _ignoredSellerId, // never allow changing owner | |
| ...safeUpdates | |
| } = updateStoreDto as any; | |
| Object.assign(store, safeUpdates); | |
| return this.storeRepository.save(store); | |
| } |
| async deleteStore(storeId: number, sellerId: number): Promise<void> { | ||
| const store = await this.storeRepository.findOne({ | ||
| where: { id: storeId, sellerId }, | ||
| }); | ||
|
|
||
| if (!store) { | ||
| throw new NotFoundException('Store not found or access denied'); | ||
| } | ||
|
|
||
| await this.storeRepository.remove(store); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix sellerId type for deleteStore
Align with UUID.
Apply this diff:
- async deleteStore(storeId: number, sellerId: number): Promise<void> {
+ async deleteStore(storeId: number, sellerId: string): Promise<void> {📝 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 deleteStore(storeId: number, sellerId: number): Promise<void> { | |
| const store = await this.storeRepository.findOne({ | |
| where: { id: storeId, sellerId }, | |
| }); | |
| if (!store) { | |
| throw new NotFoundException('Store not found or access denied'); | |
| } | |
| await this.storeRepository.remove(store); | |
| } | |
| async deleteStore(storeId: number, sellerId: string): Promise<void> { | |
| const store = await this.storeRepository.findOne({ | |
| where: { id: storeId, sellerId }, | |
| }); | |
| if (!store) { | |
| throw new NotFoundException('Store not found or access denied'); | |
| } | |
| await this.storeRepository.remove(store); | |
| } |
🤖 Prompt for AI Agents
In src/modules/stores/services/store.service.ts around lines 133 to 143, the
deleteStore method currently types sellerId as number but the system uses UUIDs;
change the sellerId parameter type to string (UUID) in the method signature,
update any local usages to treat sellerId as a string (the findOne where clause
already uses sellerId so no structural change needed), and update any callers of
deleteStore to pass a UUID string instead of a number. Ensure TypeScript types
and imports are updated accordingly so the method compiles with sellerId as
string.
| @UseGuards(JwtAuthGuard) | ||
| @HttpCode(HttpStatus.OK) | ||
| async updateUser( | ||
| @Param('id', ParseIntPipe) userId: number, | ||
| @Param('walletAddress') walletAddress: string, | ||
| @Body() updateDto: UpdateUserDto, | ||
| @Req() req: Request | ||
| ): Promise<UserUpdateResponse> { | ||
| const currentUserId = req.user?.id; | ||
| const currentUserWalletAddress = req.user?.walletAddress; | ||
| const currentUserRole = req.user?.role?.[0]; | ||
|
|
||
| // Check if user is updating their own profile or is admin | ||
| if (userId !== currentUserId && currentUserRole !== 'admin') { | ||
| if (walletAddress !== currentUserWalletAddress && currentUserRole !== 'admin') { | ||
| throw new UnauthorizedError('You can only update your own profile'); |
There was a problem hiding this comment.
Admin check bug: req.user.role is a string, not an array
JWT payload sets role as a string. Using req.user?.role?.[0] reads the first character (e.g., 'a' from 'admin'), breaking the admin bypass. Normalize role as string (or support both shapes).
Apply this diff:
- const currentUserWalletAddress = req.user?.walletAddress;
- const currentUserRole = req.user?.role?.[0];
+ const currentUserWalletAddress = (req as any).user?.walletAddress;
+ const rawRole = (req as any).user?.role;
+ const currentUserRole = Array.isArray(rawRole) ? rawRole[0] : rawRole;
@@
- if (walletAddress !== currentUserWalletAddress && currentUserRole !== 'admin') {
+ if (walletAddress !== currentUserWalletAddress && currentUserRole !== 'admin') {
throw new UnauthorizedError('You can only update your own profile');
}📝 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.
| @UseGuards(JwtAuthGuard) | |
| @HttpCode(HttpStatus.OK) | |
| async updateUser( | |
| @Param('id', ParseIntPipe) userId: number, | |
| @Param('walletAddress') walletAddress: string, | |
| @Body() updateDto: UpdateUserDto, | |
| @Req() req: Request | |
| ): Promise<UserUpdateResponse> { | |
| const currentUserId = req.user?.id; | |
| const currentUserWalletAddress = req.user?.walletAddress; | |
| const currentUserRole = req.user?.role?.[0]; | |
| // Check if user is updating their own profile or is admin | |
| if (userId !== currentUserId && currentUserRole !== 'admin') { | |
| if (walletAddress !== currentUserWalletAddress && currentUserRole !== 'admin') { | |
| throw new UnauthorizedError('You can only update your own profile'); | |
| @UseGuards(JwtAuthGuard) | |
| @HttpCode(HttpStatus.OK) | |
| async updateUser( | |
| @Param('walletAddress') walletAddress: string, | |
| @Body() updateDto: UpdateUserDto, | |
| @Req() req: Request | |
| ): Promise<UserUpdateResponse> { | |
| const currentUserWalletAddress = (req as any).user?.walletAddress; | |
| const rawRole = (req as any).user?.role; | |
| const currentUserRole = Array.isArray(rawRole) ? rawRole[0] : rawRole; | |
| // Check if user is updating their own profile or is admin | |
| if (walletAddress !== currentUserWalletAddress && currentUserRole !== 'admin') { | |
| throw new UnauthorizedError('You can only update your own profile'); |
🤖 Prompt for AI Agents
In src/modules/users/controllers/user.controller.ts around lines 111 to 123, the
admin check reads req.user?.role?.[0] which treats role as an array and thus
grabs the first character when role is a string; change the check to handle role
as a string (e.g., const currentUserRole = String(req.user?.role) or check
req.user?.role === 'admin') or normalize req.user.role to an array before
indexing so the condition correctly detects admins and preserves existing
behavior for array-shaped roles.
| async getUserByWalletAddress( | ||
| @Param('walletAddress') walletAddress: string, | ||
| @Req() req: Request | ||
| ): Promise<UserUpdateResponse> { | ||
| const currentUserId = req.user?.id; | ||
| const currentUserWalletAddress = req.user?.walletAddress; | ||
| const currentUserRole = req.user?.role?.[0]; | ||
|
|
||
| // Check if user is accessing their own profile or is admin | ||
| if (userId !== currentUserId && currentUserRole !== 'admin') { | ||
| if (walletAddress !== currentUserWalletAddress && currentUserRole !== 'admin') { | ||
| throw new UnauthorizedError('Access denied'); |
There was a problem hiding this comment.
Same admin check bug in getUserByWalletAddress
Normalize role handling here as well.
Apply this diff:
- const currentUserWalletAddress = req.user?.walletAddress;
- const currentUserRole = req.user?.role?.[0];
+ const currentUserWalletAddress = (req as any).user?.walletAddress;
+ const rawRole = (req as any).user?.role;
+ const currentUserRole = Array.isArray(rawRole) ? rawRole[0] : rawRole;
@@
- if (walletAddress !== currentUserWalletAddress && currentUserRole !== 'admin') {
+ if (walletAddress !== currentUserWalletAddress && currentUserRole !== 'admin') {
throw new UnauthorizedError('Access denied');
}🤖 Prompt for AI Agents
In src/modules/users/controllers/user.controller.ts around lines 151 to 160, the
admin check currently reads the role as req.user?.role?.[0] which fails when
role is a string or a different shape; replace it with a normalized check that
handles both string and array forms and is case-insensitive — e.g., derive roles
as an array (if req.user?.role is a string wrap it, if already an array use it),
map to lower-case trimmed values, then allow access if walletAddress matches
current user's wallet OR roles includes 'admin'; update the conditional and
error path accordingly.
…entation files; remove outdated implementation guides and summaries from docs
2 participants
🚀 StarShop Pull Request
Mark with an
xall the checkboxes that apply (like[x])📌 Type of Change
📝 Changes description
🎯 Auto-Expire Offers After 7 Days - Feature Implementation
This PR implements automatic expiration of offers that haven't been accepted after 7 days, keeping the system clean and ensuring timely responses.
Core Changes:
Database Schema Updates
expires_attimestamp field toofferstablecreatedAt + 7 daysService Layer Enhancements
API Endpoints
POST /offers/expire-manual- Manual testing triggerGET /offers/:id/expired- Check expiration statusGET /offers/expiring-soon- Monitor expiring offersCron Job Configuration
PENDINGtoREJECTEDTesting & Quality
Documentation
Technical Details:
expires_atPENDINGoffers can be expired📸 Evidence (A photo is required as evidence)
Implementation Evidence:
✅ Database Migration Created:
✅ Cron Job Service Implemented:
✅ Comprehensive Testing:
offer-expiration.service.spec.tsoffer.service.expiration.spec.tsoffer-expiration.integration.spec.ts✅ Complete Documentation:
docs/offers-auto-expiration.mddocs/IMPLEMENTATION_SUMMARY.md⏰ Time spent breakdown
Development Timeline:
Total Time: ~6.5 hours
🌌 Comments
Implementation Highlights:
�� All Requirements Met:
�� Production Ready:
🔒 Security & Data Integrity:
📚 Documentation & Support:
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
API Changes
Documentation