Conversation
WalkthroughRemoved the centralized TypeORM entities array from TypeOrmModule.forRoot in AppModule. Entities are now expected to be registered via TypeOrmModule.forFeature within feature modules. Connection options (e.g., synchronize=false, logging) remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant N as NestJS Runtime
participant A as AppModule
participant T as TypeOrmModule.forRoot
participant F as FeatureModule(s)
participant R as Repository Providers
N->>A: Bootstrap application
A->>T: Initialize DB connection (no global entities)
Note right of T: Connection opts unchanged<br/>synchronize=false, logging as before
A-->>F: Load feature modules
F->>T: TypeOrmModule.forFeature(EntityX, EntityY, ...)
T-->>R: Register repositories for feature entities
N-->>F: Inject repositories into services/controllers
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/app.module.ts (5)
21-21: Critical: Duplicate EscrowModule import will cause a runtime error.
EscrowModuleis imported twice at lines 19 and 21, and also added to the imports array twice at lines 78 and 80. Additionally, there's anEscrowsModuleimported at line 50 and added at line 82. This suggests a conflict between two different escrow implementations.Remove the duplicate imports and determine which escrow module should be used:
import { OffersModule } from './modules/offers/offers.module'; -import { EscrowModule } from './modules/escrow/escrow.module'; import { SupabaseModule } from './modules/supabase/supabase.module'; import { EscrowModule } from './modules/escrow/escrow.module';And in the imports array:
OffersModule, EscrowModule, SupabaseModule, - EscrowModule, StoresModule, EscrowsModule,Note: Verify whether you need both
EscrowModuleandEscrowsModule, or if one should be removed entirely.Also applies to: 80-80
45-45: Critical: Duplicate Escrow entity imports from different paths.
Escrowentity is imported from both./modules/escrow/entities/escrow.entity(line 45) and./modules/escrows/entities/escrow.entity(line 48). This will cause a naming conflict and prevent the file from compiling.Remove one of the duplicate imports after determining which escrow implementation is correct:
-import { Escrow } from './modules/escrow/entities/escrow.entity'; import { EscrowFundingTx } from './modules/escrow/entities/escrow-funding-tx.entity'; import { Store } from './modules/stores/entities/store.entity'; import { Escrow } from './modules/escrows/entities/escrow.entity';Also applies to: 48-48
44-44: Critical: Duplicate Milestone entity imports from different paths.
Milestoneentity is imported from both./modules/escrow/entities/milestone.entity(line 44) and./modules/escrows/entities/milestone.entity(line 49). This creates a naming conflict.Remove one of the duplicate imports:
-import { Milestone } from './modules/escrow/entities/milestone.entity'; import { Escrow } from './modules/escrow/entities/escrow.entity'; import { EscrowFundingTx } from './modules/escrow/entities/escrow-funding-tx.entity'; import { Store } from './modules/stores/entities/store.entity'; import { Escrow } from './modules/escrows/entities/escrow.entity'; import { Milestone } from './modules/escrows/entities/milestone.entity';Also applies to: 49-49
25-50: Remove unused entity imports.All entity imports (lines 25-50) are now unused since entities are loaded via
TypeOrmModule.forFeaturein feature modules. These unused imports add unnecessary clutter and increase the module's parsing time.As per coding guidelines, remove all unused imports:
-// Entities -import { User } from './modules/users/entities/user.entity'; -import { Order } from './modules/orders/entities/order.entity'; -import { OrderItem } from './modules/orders/entities/order-item.entity'; -import { UserRole } from './modules/auth/entities/user-role.entity'; -import { Role } from './modules/auth/entities/role.entity'; -import { Notification } from './modules/notifications/entities/notification.entity'; -import { Wishlist } from './modules/wishlist/entities/wishlist.entity'; -import { Product } from './modules/products/entities/product.entity'; -import { ProductType } from './modules/productTypes/entities/productTypes.entity'; -import { ProductVariant } from './modules/productVariants/entities/productVariants.entity'; -import { Attribute } from './modules/attributes/entities/attribute.entity'; -import { AttributeValue } from './modules/attributes/entities/attribute-value.entity'; -import { Coupon } from './modules/coupons/entities/coupon.entity'; -import { CouponUsage } from './modules/coupons/entities/coupon-usage.entity'; -import { BuyerRequest } from './modules/buyer-requests/entities/buyer-request.entity'; -import { Offer } from './modules/offers/entities/offer.entity'; -import { OfferAttachment } from './modules/offers/entities/offer-attachment.entity'; -import { EscrowAccount } from './modules/escrow/entities/escrow-account.entity'; -import { Milestone } from './modules/escrow/entities/milestone.entity'; -import { Escrow } from './modules/escrow/entities/escrow.entity'; -import { EscrowFundingTx } from './modules/escrow/entities/escrow-funding-tx.entity'; -import { Store } from './modules/stores/entities/store.entity'; -import { Escrow } from './modules/escrows/entities/escrow.entity'; -import { Milestone } from './modules/escrows/entities/milestone.entity'; -import { EscrowsModule } from './modules/escrows/escrows.module';
57-64: Add missing TypeOrmModule.forFeature import in notifications module
The notifications feature module currently lacks aTypeOrmModule.forFeature([...])call, so its entities aren’t registered and their repositories will fail at runtime. Insrc/modules/notifications/notifications.module.ts, add:imports: [ TypeOrmModule.forFeature([NotificationEntity /*, …other entities*/]), // …other imports ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
src/app.module.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.ts
📄 CodeRabbit inference engine (.cursorrules)
src/**/*.ts: Do not access environment variables via process.env directly; import and use the config object from src/config/env
Provide explicit return types for all functions
Do not use the any type; use unknown or specific types instead
Create interfaces for complex object shapes
Type all function parameters explicitly
Do not use console.log; use proper logging (e.g., NestJS Logger)
Remove unused imports
Do not leave commented-out code in commits
Wrap risky operations in try-catch and handle errors appropriately
Add JSDoc comments for complex logic
Files:
src/app.module.ts
🧠 Learnings (1)
📚 Learning: 2025-09-29T16:06:54.363Z
Learnt from: CR
PR: StarShopCr/StarShop-Backend#0
File: .cursorrules:0-0
Timestamp: 2025-09-29T16:06:54.363Z
Learning: Applies to src/modules/**/entities/**/*.entity.ts : Define persistence entities in entities/*.entity.ts
Applied to files:
src/app.module.ts
| url: process.env.DATABASE_URL, | ||
| ssl: process.env.DB_SSL === 'true' ? { rejectUnauthorized: false } : undefined, |
There was a problem hiding this comment.
Replace direct process.env access with config object.
Direct access to process.env violates the coding guidelines. Environment variables should be accessed via the config object from src/config/env.
As per coding guidelines, import and use the config object:
+import { config } from './config/env';
+
import './config/crypto-global';
import { Module } from '@nestjs/common';Then update the TypeOrmModule configuration:
TypeOrmModule.forRoot({
type: 'postgres',
- url: process.env.DATABASE_URL,
- ssl: process.env.DB_SSL === 'true' ? { rejectUnauthorized: false } : undefined,
+ url: config.DATABASE_URL,
+ ssl: config.DB_SSL === 'true' ? { rejectUnauthorized: false } : undefined,
// entities property removed for modularization. Now entities are loaded via TypeOrmModule.forFeature in each feature module.
synchronize: false,
- logging: process.env.NODE_ENV === 'development',
+ logging: config.NODE_ENV === 'development',
}),Also applies to: 63-63
🤖 Prompt for AI Agents
In src/app.module.ts around lines 59-60 (also apply to line 63), replace direct
process.env usage with the project's config object from src/config/env: import
the config instance at the top of the file, then use config.get('DATABASE_URL')
in place of process.env.DATABASE_URL and use config.get('DB_SSL') === 'true' (or
a boolean helper from the config if available) to decide the ssl value instead
of process.env.DB_SSL; update the TypeOrmModule configuration accordingly so ssl
is set to { rejectUnauthorized: false } when the config indicates true and
undefined otherwise, and make the same replacement for the other occurrence on
line 63.
1 participant
🚀 StarShop Pull Request – Issue #198
Mark with an
xall the checkboxes that apply (like[x]).✅ Checklist
npm run test:e2e)npm run format/npm run lint:fix)📌 Type of Change
📝 Changes Description
Implemented entitys loading
⏰ Time Spent Breakdown
1 hour
Summary by CodeRabbit