refactor: data init #178
refactor: data init #178
Conversation
github-actions
bot
added
the
refactoring
WalkthroughThe changes introduce a new Initializer pattern to replace direct service instantiation in AppModule. Five initializer classes—MenuInitializer, RoleInit, PermissionInit, UserInit, and ApplicationInit—manage seeding of menus, permissions, roles, users, and applications respectively. A new menu-data.json file provides hierarchical menu configuration, replacing legacy static menu data. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/cmd docker --check |
🐳 Docker Check Status
Status: ⏳ Building... Preparing environment and running Docker health checks. |
✅ Docker Check Status
Status: SUCCESS! Service is built, running, and responded to health check on |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@template/nestJs/src/application/application.init.ts`:
- Around line 13-22: The loop over applicationData in run() incorrectly uses
return when an existing application is found, which exits the entire method and
skips remaining items; change that return to continue so the loop skips only the
current app and proceeds. Update the logic around this.application.findOne(...)
to continue on a truthy result and ensure subsequent calls to
this.application.create(app) and this.application.save(...) still execute for
other entries.
- Around line 7-11: The ApplicationInit class lacks the NestJS `@Injectable`()
decorator, so import Injectable from '@nestjs/common' and annotate the class
with `@Injectable`() above the declaration of export class ApplicationInit
implements Initializer<Promise<void>>; this ensures Nest's DI can instantiate
the class that injects Repository<Application> via the constructor with
`@InjectRepository`(Application).
In `@template/nestJs/src/menu/menu.initializer.ts`:
- Around line 48-50: The dfs function is async but recursive calls to dfs for
each child (inside run when iterating data.children and passing res.id) are not
awaited, allowing run() to finish before nested menus persist; update the loop
in the run()/initializer context to await each dfs(child, 0, res.id) (or gather
Promises and await Promise.all) so that all recursive persists complete before
run() returns, ensuring the full menu tree is persisted for downstream
initializers.
In `@template/nestJs/src/permission/permission.initalizer.ts`:
- Around line 27-33: The wildcard permission '*' is unconditionally pushed into
permissionEntites causing duplicates on repeated runs; update the seeding in
run() to check for existence via this.permission.findOne({ where: { name: '*' }
}) before calling this.permission.create(...) and pushing to permissionEntites
so it mirrors the other permission checks and prevents duplicate entries on
repeated runs (ensure you reference the same this.permission.create and
this.permission.findOne usage and the permissionEntites array and save() call).
- Around line 1-10: The class PermissionInit is missing the `@Injectable`()
decorator so Nest's DI won't inject the `@InjectRepository`(Permission)
repository; add an import for Injectable from "@nestjs/common" and annotate the
PermissionInit class with `@Injectable`() (preserving the existing constructor and
Repository<Permission> injection) so DI works like the other initializers (e.g.,
MenuInitializer, RoleInit).
In `@template/nestJs/src/user/user.initalizer.ts`:
- Around line 39-42: The Logger calls in user.initalizer.ts currently print the
admin password in plaintext; update the code that logs after creating the admin
(see Logger.log calls for "[APP]: create admin user success", "[APP]: email:
${user.email}", and the line that logs "password: 'admin'") to stop outputting
the password — either remove the password log entirely or wrap it in a
development-only guard (e.g., check NODE_ENV === 'development' or a dedicated
feature flag) so production logs never contain credentials; ensure only
non-sensitive info (email, success) is logged and any dev-only logging is
clearly gated.
- Around line 8-15: The UserInit class is missing the required NestJS decorator;
add the `@Injectable`() decorator above the exported class UserInit so Nest's DI
can resolve the constructor-injected dependencies (Repository<User>,
UserService, Repository<Role>) used in Initializer<Promise<void>>; also import
Injectable from '@nestjs/common' if not already imported and ensure the
decorator is exported with the class.
🧹 Nitpick comments (3)
template/nestJs/src/initalizer.interface.ts (1)
1-3: Typo in filename:initalizershould beinitializer.The interface design is clean and appropriate. However, the filename
initalizer.interface.tshas a typo (missing 'i'). This inconsistency is already propagating to imports across multiple files.Suggested rename
Rename the file from
initalizer.interface.tstoinitializer.interface.tsand update all imports accordingly.template/nestJs/src/user/user.initalizer.ts (1)
1-6: Filename typo:user.initalizer.tsshould beuser.initializer.ts.Consistent with the interface file, but the typo should be corrected across all files for maintainability.
template/nestJs/src/role/role.initializer.ts (1)
20-26: Check for existing admin role before heavy queries.You can avoid extra DB work on every startup by short-circuiting before fetching permissions/menus.
♻️ Proposed refactor
async run(): Promise<void> { - const permission = await this.permission.find({ - where:{ - name: '*' - } - }) - const menus = await this.menu.find(); if (await this.role.findOneBy({name: 'admin'})) { return; } + const permission = await this.permission.find({ + where:{ + name: '*' + } + }) + const menus = await this.menu.find(); await this.role.save({ name: 'admin', permission: permission, menus }); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
template/nestJs/menu-data.jsontemplate/nestJs/src/app.module.tstemplate/nestJs/src/application/application.init.tstemplate/nestJs/src/application/application.module.tstemplate/nestJs/src/initalizer.interface.tstemplate/nestJs/src/menu/init/menuData.tstemplate/nestJs/src/menu/menu.initializer.tstemplate/nestJs/src/menu/menu.module.tstemplate/nestJs/src/menu/menu.service.tstemplate/nestJs/src/permission/permission.initalizer.tstemplate/nestJs/src/permission/permission.module.tstemplate/nestJs/src/role/role.initializer.tstemplate/nestJs/src/role/role.module.tstemplate/nestJs/src/user/user.initalizer.tstemplate/nestJs/src/user/user.module.ts
💤 Files with no reviewable changes (2)
- template/nestJs/src/menu/menu.service.ts
- template/nestJs/src/menu/init/menuData.ts
🧰 Additional context used
🧬 Code graph analysis (8)
template/nestJs/src/role/role.module.ts (5)
template/nestJs/src/app.module.ts (1)
Module(50-206)template/nestJs/src/application/application.module.ts (1)
Module(8-14)template/nestJs/src/menu/menu.module.ts (1)
Module(8-14)template/nestJs/src/permission/permission.module.ts (1)
Module(8-14)template/nestJs/src/user/user.module.ts (1)
Module(11-17)
template/nestJs/src/user/user.initalizer.ts (1)
template/nestJs/src/initalizer.interface.ts (1)
Initializer(1-3)
template/nestJs/src/application/application.module.ts (6)
template/nestJs/src/app.module.ts (1)
Module(50-206)template/nestJs/src/menu/menu.module.ts (1)
Module(8-14)template/nestJs/src/permission/permission.module.ts (1)
Module(8-14)template/nestJs/src/role/role.module.ts (1)
Module(8-14)template/nestJs/src/user/user.module.ts (1)
Module(11-17)template/nestJs/src/application/application.init.ts (1)
ApplicationInit(7-24)
template/nestJs/src/permission/permission.initalizer.ts (2)
template/nestJs/src/initalizer.interface.ts (1)
Initializer(1-3)template/tinyvue/src/api/permission.ts (1)
Permission(3-7)
template/nestJs/src/user/user.module.ts (2)
template/nestJs/src/app.module.ts (1)
Module(50-206)template/nestJs/src/user/user.initalizer.ts (1)
UserInit(8-44)
template/nestJs/src/role/role.initializer.ts (3)
template/nestJs/src/menu/menu.initializer.ts (1)
Injectable(20-56)template/nestJs/src/initalizer.interface.ts (1)
Initializer(1-3)template/tinyvue/src/api/permission.ts (1)
Permission(3-7)
template/nestJs/src/application/application.init.ts (2)
template/nestJs/src/initalizer.interface.ts (1)
Initializer(1-3)template/nestJs/src/application/init/data.ts (1)
applicationData(1-88)
template/nestJs/src/menu/menu.initializer.ts (2)
template/nestJs/src/role/role.initializer.ts (1)
Injectable(7-35)template/nestJs/src/initalizer.interface.ts (1)
Initializer(1-3)
🔇 Additional comments (8)
template/nestJs/menu-data.json (2)
247-255: Verify: EmptymenuTypefor "Local" menu item.The "Local" menu item has an empty
menuTypewhile all other items have either"normal"or"admin". This may be intentional, but please verify this is the expected behavior.
1-262: LGTM!The JSON structure is well-formed and consistent. The hierarchical menu configuration is clean and follows a predictable pattern.
template/nestJs/src/application/application.module.ts (1)
6-12: LGTM!The module correctly imports, provides, and exports
ApplicationInit, following the same pattern established in other modules (RoleModule,MenuModule,PermissionModule,UserModule).template/nestJs/src/user/user.module.ts (1)
9-15: Initializer wiring looks consistent.Adding
UserInitto providers/exports matches the initializer-based bootstrap flow and keeps module boundaries clear.template/nestJs/src/permission/permission.module.ts (1)
6-12: Initializer wiring looks consistent.Registering and exporting
PermissionInitaligns with the new initialization flow.template/nestJs/src/role/role.module.ts (1)
6-12: Initializer wiring looks consistent.Exposing
RoleInitvia providers/exports fits the new initializer-based bootstrap design.template/nestJs/src/menu/menu.module.ts (1)
6-12: Initializer wiring looks consistent.Providing and exporting
MenuInitializerkeeps the module aligned with the new bootstrap flow.template/nestJs/src/app.module.ts (1)
41-45: Initializer wiring and sequencing look consistent.Imports, constructor injection, and run order align with the new initializer pattern.
Also applies to: 103-108, 190-194
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| export class ApplicationInit implements Initializer<Promise<void>> { | ||
| constructor( | ||
| @InjectRepository(Application) | ||
| private application: Repository<Application> | ||
| ){} |
There was a problem hiding this comment.
Missing @Injectable() decorator.
NestJS requires the @Injectable() decorator for classes that use dependency injection. Without it, the DI container may not properly instantiate this class.
Proposed fix
+import { Injectable } from "@nestjs/common";
import { Initializer } from "../initalizer.interface";
import { applicationData } from './init/data';
import { Repository } from "typeorm";
import { Application } from "@app/models";
import { InjectRepository } from "@nestjs/typeorm";
+@Injectable()
export class ApplicationInit implements Initializer<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.
| export class ApplicationInit implements Initializer<Promise<void>> { | |
| constructor( | |
| @InjectRepository(Application) | |
| private application: Repository<Application> | |
| ){} | |
| import { Injectable } from "@nestjs/common"; | |
| import { Initializer } from "../initalizer.interface"; | |
| import { applicationData } from './init/data'; | |
| import { Repository } from "typeorm"; | |
| import { Application } from "@app/models"; | |
| import { InjectRepository } from "@nestjs/typeorm"; | |
| `@Injectable`() | |
| export class ApplicationInit implements Initializer<Promise<void>> { | |
| constructor( | |
| `@InjectRepository`(Application) | |
| private application: Repository<Application> | |
| ){} |
🤖 Prompt for AI Agents
In `@template/nestJs/src/application/application.init.ts` around lines 7 - 11, The
ApplicationInit class lacks the NestJS `@Injectable`() decorator, so import
Injectable from '@nestjs/common' and annotate the class with `@Injectable`() above
the declaration of export class ApplicationInit implements
Initializer<Promise<void>>; this ensures Nest's DI can instantiate the class
that injects Repository<Application> via the constructor with
`@InjectRepository`(Application).
| for (const app of applicationData) { | ||
| if ( | ||
| await this.application.findOne({where:{name:app.name}}) | ||
| ) { | ||
| return; | ||
| } | ||
| await this.application.save( | ||
| this.application.create(app) | ||
| ) | ||
| } |
There was a problem hiding this comment.
Logic error: return exits early, skipping remaining applications.
When an existing application is found, return exits the entire run() method, preventing any subsequent applications in applicationData from being processed. This should be continue to skip only the current item.
Proposed fix
async run(): Promise<void> {
for (const app of applicationData) {
if (
await this.application.findOne({where:{name:app.name}})
) {
- return;
+ continue;
}
await this.application.save(
this.application.create(app)
)
}
}📝 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.
| for (const app of applicationData) { | |
| if ( | |
| await this.application.findOne({where:{name:app.name}}) | |
| ) { | |
| return; | |
| } | |
| await this.application.save( | |
| this.application.create(app) | |
| ) | |
| } | |
| for (const app of applicationData) { | |
| if ( | |
| await this.application.findOne({where:{name:app.name}}) | |
| ) { | |
| continue; | |
| } | |
| await this.application.save( | |
| this.application.create(app) | |
| ) | |
| } |
🤖 Prompt for AI Agents
In `@template/nestJs/src/application/application.init.ts` around lines 13 - 22,
The loop over applicationData in run() incorrectly uses return when an existing
application is found, which exits the entire method and skips remaining items;
change that return to continue so the loop skips only the current app and
proceeds. Update the logic around this.application.findOne(...) to continue on a
truthy result and ensure subsequent calls to this.application.create(app) and
this.application.save(...) still execute for other entries.
| for (const child of data.children) { | ||
| dfs(child, 0, res.id) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "menu.initializer.ts" | head -5Repository: opentiny/tiny-pro
Length of output: 107
🏁 Script executed:
cat -n ./template/nestJs/src/menu/menu.initializer.tsRepository: opentiny/tiny-pro
Length of output: 1966
🏁 Script executed:
# Check if there are other initializers that depend on MenuInitializer
find . -type f -name "*.ts" -path "*/initializer*" | head -10Repository: opentiny/tiny-pro
Length of output: 43
🏁 Script executed:
find . -type f -name "*initializer*" -o -name "*Initializer*" | grep -E "\.(ts|js)$" | head -20Repository: opentiny/tiny-pro
Length of output: 154
🏁 Script executed:
cat -n ./template/nestJs/src/role/role.initializer.tsRepository: opentiny/tiny-pro
Length of output: 1210
Await child DFS recursion to ensure complete persistence.
The dfs function is async, but child calls at lines 48–50 lack await. This allows run() to complete before nested menus persist, causing downstream initializers like RoleInit to read incomplete menu trees.
🐛 Suggested fix
for (const child of data.children) {
- dfs(child, 0, res.id)
+ await dfs(child, 0, res.id)
}📝 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.
| for (const child of data.children) { | |
| dfs(child, 0, res.id) | |
| } | |
| for (const child of data.children) { | |
| await dfs(child, 0, res.id) | |
| } |
🤖 Prompt for AI Agents
In `@template/nestJs/src/menu/menu.initializer.ts` around lines 48 - 50, The dfs
function is async but recursive calls to dfs for each child (inside run when
iterating data.children and passing res.id) are not awaited, allowing run() to
finish before nested menus persist; update the loop in the run()/initializer
context to await each dfs(child, 0, res.id) (or gather Promises and await
Promise.all) so that all recursive persists complete before run() returns,
ensuring the full menu tree is persisted for downstream initializers.
| import { InjectRepository } from "@nestjs/typeorm"; | ||
| import { Initializer } from "../initalizer.interface"; | ||
| import { Permission } from "@app/models"; | ||
| import { Repository } from "typeorm"; | ||
|
|
||
| export class PermissionInit implements Initializer<Promise<void>>{ | ||
| constructor( | ||
| @InjectRepository(Permission) | ||
| private permission: Repository<Permission>, | ||
| ){} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find other initializer implementations to see the pattern
fd -e ts -e js | xargs grep -l "implements Initializer" 2>/dev/null | head -20Repository: opentiny/tiny-pro
Length of output: 178
🏁 Script executed:
# Search for other classes with `@InjectRepository` in the codebase
rg "@InjectRepository" --type ts -B 3 -A 3 | head -100Repository: opentiny/tiny-pro
Length of output: 6781
🏁 Script executed:
# Check if `@Injectable` is used with `@InjectRepository` in similar patterns
rg "@Injectable\(\)" --type ts -B 2 -A 8 | head -150Repository: opentiny/tiny-pro
Length of output: 10183
🌐 Web query:
NestJS @Injectabledecorator requirement with@InjectRepository dependency injection
💡 Result:
Short answer: yes — the class that receives an @InjectRepository(...) in its constructor must be a Nest provider (e.g., decorated with @Injectable() or another Nest decorator that registers it as a provider) so the DI container can resolve the repository.
Why: @Injectable() (or another provider-declaring decorator) ensures the class is registered with Nest’s IoC container and (via TypeScript's emitDecoratorMetadata / reflect-metadata) provides the constructor metadata Nest uses to resolve constructor parameters like the repository. If you register a plain class in a module’s providers array with no decorator and it has no ctor deps, it may still be instantiable — but when you depend on things provided by TypeOrmModule.forFeature (injected via @InjectRepository) you need the metadata that @Injectable() (or an equivalent) triggers. [1][2][3]
Practical steps:
- Add
@Injectable() to your service (or ensure it’s otherwise registered as a provider). - Import TypeOrmModule.forFeature([...]) in the module that declares the service so the repository token is provided.
- Ensure tsconfig has "emitDecoratorMetadata": true and you import 'reflect-metadata' (standard Nest setup). [1][2]
Sources:
- NestJS providers / DI docs. [1]
- NestJS SQL / TypeORM recipe showing
@InjectRepositoryusage. [2] - Community explanation of
@Injectableand metadata necessity. [3]
References:
[1] NestJS — Providers / Custom providers (dependency injection)
[2] NestJS — Database (TypeORM) / forFeature and @InjectRepository example
[3] Discussion: why @Injectable is needed (emitDecoratorMetadata explanation)
Add @Injectable() to ensure Nest DI works.
Without @Injectable(), constructor injection (including @InjectRepository) will fail at runtime during module bootstrap. Other initializers in the codebase (MenuInitializer, RoleInit) use this decorator; PermissionInit should follow the same pattern.
🛠️ Suggested fix
import { InjectRepository } from "@nestjs/typeorm";
+import { Injectable } from "@nestjs/common";
import { Initializer } from "../initalizer.interface";
import { Permission } from "@app/models";
import { Repository } from "typeorm";
+@Injectable()
export class PermissionInit implements Initializer<Promise<void>>{
constructor(
`@InjectRepository`(Permission)
private permission: Repository<Permission>,
){}📝 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 { InjectRepository } from "@nestjs/typeorm"; | |
| import { Initializer } from "../initalizer.interface"; | |
| import { Permission } from "@app/models"; | |
| import { Repository } from "typeorm"; | |
| export class PermissionInit implements Initializer<Promise<void>>{ | |
| constructor( | |
| @InjectRepository(Permission) | |
| private permission: Repository<Permission>, | |
| ){} | |
| import { InjectRepository } from "@nestjs/typeorm"; | |
| import { Injectable } from "@nestjs/common"; | |
| import { Initializer } from "../initalizer.interface"; | |
| import { Permission } from "@app/models"; | |
| import { Repository } from "typeorm"; | |
| `@Injectable`() | |
| export class PermissionInit implements Initializer<Promise<void>>{ | |
| constructor( | |
| `@InjectRepository`(Permission) | |
| private permission: Repository<Permission>, | |
| ){} |
🤖 Prompt for AI Agents
In `@template/nestJs/src/permission/permission.initalizer.ts` around lines 1 - 10,
The class PermissionInit is missing the `@Injectable`() decorator so Nest's DI
won't inject the `@InjectRepository`(Permission) repository; add an import for
Injectable from "@nestjs/common" and annotate the PermissionInit class with
`@Injectable`() (preserving the existing constructor and Repository<Permission>
injection) so DI works like the other initializers (e.g., MenuInitializer,
RoleInit).
| const permissionEntites:Permission[] = []; | ||
| permissionEntites.push( | ||
| this.permission.create({ | ||
| name: '*', | ||
| desc: '' | ||
| }) | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "*permission*initializer*" -o -name "*initializer*permission*" | head -20Repository: opentiny/tiny-pro
Length of output: 43
🏁 Script executed:
find . -path "*/permission/*" -name "*.ts" | grep -E "(initializer|initalizer)" | head -20Repository: opentiny/tiny-pro
Length of output: 118
🏁 Script executed:
find . -type f -name "*.ts" | xargs grep -l "permission\.initalizer\|permission\.initializer" | head -10Repository: opentiny/tiny-pro
Length of output: 150
🏁 Script executed:
cat -n ./template/nestJs/src/permission/permission.initalizer.tsRepository: opentiny/tiny-pro
Length of output: 1948
🏁 Script executed:
find . -path "*/permission/*" -name "*.ts" | head -20Repository: opentiny/tiny-pro
Length of output: 721
🏁 Script executed:
find . -path "*models*permission*" -o -path "*entity*permission*" | grep -E "\.ts$" | head -20Repository: opentiny/tiny-pro
Length of output: 176
🏁 Script executed:
find . -type f -name "*.ts" | xargs grep -l "class Permission" | head -10Repository: opentiny/tiny-pro
Length of output: 386
🏁 Script executed:
cat -n ./template/nestJs/libs/models/src/permission.tsRepository: opentiny/tiny-pro
Length of output: 625
🏁 Script executed:
find . -path "*/migrations/*" -name "*.ts" | xargs grep -l "permission\|Permission" 2>/dev/null | head -10Repository: opentiny/tiny-pro
Length of output: 43
🏁 Script executed:
cat -n ./template/nestJs/src/permission/permission.service.tsRepository: opentiny/tiny-pro
Length of output: 3007
🏁 Script executed:
find . -type f -name "*.ts" | xargs grep -l "unique\|UNIQUE" | grep -iE "(migration|schema|permission)" | head -10Repository: opentiny/tiny-pro
Length of output: 43
🏁 Script executed:
grep -r "unique" ./template/nestJs/libs/models/src/permission.ts 2>/dev/nullRepository: opentiny/tiny-pro
Length of output: 43
🏁 Script executed:
cd ./template/nestJs && find . -type f -name "*.ts" | xargs grep -B5 -A5 "@Unique\|unique.*true" 2>/dev/null | grep -iE "(permission|name)" | head -20Repository: opentiny/tiny-pro
Length of output: 43
🏁 Script executed:
grep -r "PermissionInit\|permission.*initalizer\|run()" ./template/nestJs/src --include="*.ts" | grep -v node_modules | head -20Repository: opentiny/tiny-pro
Length of output: 1501
🏁 Script executed:
cat -n ./template/nestJs/src/permission/permission.module.tsRepository: opentiny/tiny-pro
Length of output: 707
Make wildcard permission seeding idempotent.
The '*' permission is added unconditionally while all other permissions in the same initializer (lines 34–50) check for existence before creation. If run() is called multiple times during retry or partial installs, the wildcard will be added to the array repeatedly, causing save() to fail. Guard it with the same findOne() check used for other permissions.
🛠️ Suggested fix
const permissionEntites:Permission[] = [];
+ const existingWildcard = await this.permission.findOne({
+ where: { name: '*' },
+ });
+ if (!existingWildcard) {
permissionEntites.push(
this.permission.create({
name: '*',
desc: ''
})
);
+ }📝 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 permissionEntites:Permission[] = []; | |
| permissionEntites.push( | |
| this.permission.create({ | |
| name: '*', | |
| desc: '' | |
| }) | |
| ); | |
| const permissionEntites:Permission[] = []; | |
| const existingWildcard = await this.permission.findOne({ | |
| where: { name: '*' }, | |
| }); | |
| if (!existingWildcard) { | |
| permissionEntites.push( | |
| this.permission.create({ | |
| name: '*', | |
| desc: '' | |
| }) | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@template/nestJs/src/permission/permission.initalizer.ts` around lines 27 -
33, The wildcard permission '*' is unconditionally pushed into permissionEntites
causing duplicates on repeated runs; update the seeding in run() to check for
existence via this.permission.findOne({ where: { name: '*' } }) before calling
this.permission.create(...) and pushing to permissionEntites so it mirrors the
other permission checks and prevents duplicate entries on repeated runs (ensure
you reference the same this.permission.create and this.permission.findOne usage
and the permissionEntites array and save() call).
| export class UserInit implements Initializer<Promise<void>> { | ||
| constructor( | ||
| @InjectRepository(User) | ||
| private userRepo: Repository<User>, | ||
| private user: UserService, | ||
| @InjectRepository(Role) | ||
| private role: Repository<Role> | ||
| ){} |
There was a problem hiding this comment.
Missing @Injectable() decorator.
Same issue as ApplicationInit - NestJS requires @Injectable() for proper dependency injection.
Proposed fix
import { InjectRepository } from "@nestjs/typeorm";
import { Initializer } from "../initalizer.interface";
import { Role, User } from "@app/models";
import { Repository } from "typeorm";
import { UserService } from "./user.service";
-import { Logger } from "@nestjs/common";
+import { Injectable, Logger } from "@nestjs/common";
+@Injectable()
export class UserInit implements Initializer<Promise<void>> {🤖 Prompt for AI Agents
In `@template/nestJs/src/user/user.initalizer.ts` around lines 8 - 15, The
UserInit class is missing the required NestJS decorator; add the `@Injectable`()
decorator above the exported class UserInit so Nest's DI can resolve the
constructor-injected dependencies (Repository<User>, UserService,
Repository<Role>) used in Initializer<Promise<void>>; also import Injectable
from '@nestjs/common' if not already imported and ensure the decorator is
exported with the class.
| Logger.log(`[APP]: create admin user success`); | ||
| Logger.log(`[APP]: email: ${user.email}`); | ||
| Logger.log(`[APP]: password: 'admin'`); | ||
| Logger.log('Enjoy!'); |
There was a problem hiding this comment.
Avoid logging plaintext passwords.
Logging the password 'admin' to console could expose credentials in production logs. Consider removing the password log or only logging it in development mode.
Proposed fix
Logger.log(`[APP]: create admin user success`);
Logger.log(`[APP]: email: ${user.email}`);
- Logger.log(`[APP]: password: 'admin'`);
+ Logger.log(`[APP]: password: <redacted> (see seed configuration)`);
Logger.log('Enjoy!');📝 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.
| Logger.log(`[APP]: create admin user success`); | |
| Logger.log(`[APP]: email: ${user.email}`); | |
| Logger.log(`[APP]: password: 'admin'`); | |
| Logger.log('Enjoy!'); | |
| Logger.log(`[APP]: create admin user success`); | |
| Logger.log(`[APP]: email: ${user.email}`); | |
| Logger.log(`[APP]: password: <redacted> (see seed configuration)`); | |
| Logger.log('Enjoy!'); |
🤖 Prompt for AI Agents
In `@template/nestJs/src/user/user.initalizer.ts` around lines 39 - 42, The Logger
calls in user.initalizer.ts currently print the admin password in plaintext;
update the code that logs after creating the admin (see Logger.log calls for
"[APP]: create admin user success", "[APP]: email: ${user.email}", and the line
that logs "password: 'admin'") to stop outputting the password — either remove
the password log entirely or wrap it in a development-only guard (e.g., check
NODE_ENV === 'development' or a dedicated feature flag) so production logs never
contain credentials; ensure only non-sensitive info (email, success) is logged
and any dev-only logging is clearly gated.
|
@GaoNeng-wWw 重构之后比之前直观多了👍 |
2 participants
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.