feat(cli): db seed command #428
Conversation
|
Claude Code is working… I'll analyze this and get back to you. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new CLI "db seed" action that reads a seed script from package.json, executes it via execa, integrates optional automatic seeding into migrate reset/dev flows (with a --skip-seed flag), extends getPkgJsonConfig to return seed, and adds tests covering seeding behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)packages/cli/src/index.ts (1)
🔇 Additional comments (2)
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 |
|
fixes #387 |
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new db seed command to the ZenStack CLI that allows users to seed their database using a custom script defined in package.json. The seeding functionality is also integrated into the migrate reset command with an option to skip it.
Key Changes:
- New
zen db seedcommand that executes a seed script from package.json configuration - Integration with
migrate resetto automatically run seeding after reset (with--skip-seedopt-out) - Both
migrate devandmigrate resetnow pass--skip-seedto Prisma to use ZenStack's custom seeding instead
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds execa ^9.6.0 dependency for command execution |
| packages/cli/package.json | Adds execa dependency to the CLI package |
| packages/cli/src/actions/seed.ts | New seed action that executes custom seed scripts from package.json |
| packages/cli/src/actions/index.ts | Exports the new seed action |
| packages/cli/src/actions/action-utils.ts | Extends getPkgJsonConfig to support seed script configuration |
| packages/cli/src/actions/migrate.ts | Integrates seeding into migrate reset with skip-seed option |
| packages/cli/src/index.ts | Registers new db seed command and adds skip-seed option to migrate reset |
| packages/cli/test/db.test.ts | Adds comprehensive tests for seeding functionality |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/cli/src/actions/action-utils.ts (1)
80-112: Inconsistent path handling for seed configuration.Lines 100-107 resolve
schemaandoutputto absolute paths usingpath.resolve, but Line 108 treatsseedas a raw string without path resolution. This inconsistency could be confusing.However, this appears intentional since
seedis a shell command (e.g.,"node seed.js"or"npm run seed"), not a file path. Consider adding a comment to clarify this distinction.Add a clarifying comment:
result.seed = typeof pkgJson.zenstack.seed === 'string' && pkgJson.zenstack.seed; + // Note: seed is a shell command string, not a file path, so it's not resolved }Additionally, consider refactoring the logical AND pattern for better clarity:
if (pkgJson.zenstack && typeof pkgJson.zenstack === 'object') { - result.schema = - pkgJson.zenstack.schema && - typeof pkgJson.zenstack.schema === 'string' && - path.resolve(path.dirname(pkgJsonFile), pkgJson.zenstack.schema); - result.output = - pkgJson.zenstack.output && - typeof pkgJson.zenstack.output === 'string' && - path.resolve(path.dirname(pkgJsonFile), pkgJson.zenstack.output); - result.seed = typeof pkgJson.zenstack.seed === 'string' && pkgJson.zenstack.seed; + if (pkgJson.zenstack.schema && typeof pkgJson.zenstack.schema === 'string') { + result.schema = path.resolve(path.dirname(pkgJsonFile), pkgJson.zenstack.schema); + } + if (pkgJson.zenstack.output && typeof pkgJson.zenstack.output === 'string') { + result.output = path.resolve(path.dirname(pkgJsonFile), pkgJson.zenstack.output); + } + if (typeof pkgJson.zenstack.seed === 'string') { + result.seed = pkgJson.zenstack.seed; + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
packages/cli/package.json(1 hunks)packages/cli/src/actions/action-utils.ts(2 hunks)packages/cli/src/actions/index.ts(1 hunks)packages/cli/src/actions/migrate.ts(3 hunks)packages/cli/src/actions/seed.ts(1 hunks)packages/cli/src/index.ts(3 hunks)packages/cli/test/db.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
packages/cli/src/actions/seed.ts (2)
packages/cli/src/actions/action-utils.ts (1)
getPkgJsonConfig(80-112)packages/cli/src/cli-error.ts (1)
CliError(4-4)
packages/cli/src/actions/migrate.ts (2)
packages/cli/src/utils/exec-utils.ts (1)
execPrisma(32-53)packages/cli/src/actions/action-utils.ts (1)
handleSubProcessError(58-64)
packages/cli/src/index.ts (1)
packages/cli/src/telemetry.ts (1)
telemetry(139-139)
packages/cli/test/db.test.ts (1)
packages/cli/test/utils.ts (2)
createProject(12-18)runCli(20-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Agent
- GitHub Check: build-test (20.x, sqlite)
- GitHub Check: build-test (20.x, postgresql)
- GitHub Check: claude-review
🔇 Additional comments (10)
packages/cli/src/actions/index.ts (1)
8-10: LGTM!The seed action export follows the existing pattern and integrates cleanly with the other action exports.
packages/cli/src/actions/seed.ts (1)
14-21: Clean early return handling.The early return when no seed script is defined is well-structured, with appropriate warning suppression via the
noWarningsoption.packages/cli/src/index.ts (2)
37-39: LGTM!The seedAction follows the established pattern for action wrappers and correctly tracks the command as 'db seed' for telemetry.
94-94: Good integration with migrate reset workflow.The
--skip-seedoption provides appropriate control over the seeding behavior during reset operations.packages/cli/test/db.test.ts (1)
57-60: LGTM!The test correctly validates that the CLI handles the absence of a seed script gracefully without errors.
packages/cli/src/actions/migrate.ts (4)
6-6: Good integration with seed action.Importing the seed runner enables post-migration seeding in the reset workflow.
11-11: LGTM!The
skipSeedoption is appropriately added to CommonOptions to control seeding behavior across migrate commands.
69-83: Clarify seeding behavior in dev command.The
devcommand unconditionally passes--skip-seedto Prisma (Line 75), but there's no corresponding ZenStack seed execution after the migration like in theresetcommand. This means users cannot seed duringmigrate devworkflow.Is this intentional? Consider whether the dev command should also respect the
skipSeedoption and conditionally run the ZenStack seed after migration, similar to the reset command.Please clarify if:
- Seeding is intentionally not supported during
migrate dev- If supported, should it follow the same pattern as reset (Lines 99-101)?
// Potential enhancement for consistency: async function runDev(prismaSchemaFile: string, options: DevOptions) { try { const cmd = [ 'migrate dev', ` --schema "${prismaSchemaFile}"`, ' --skip-generate', ' --skip-seed', // Skip Prisma's seed options.name ? ` --name "${options.name}"` : '', options.createOnly ? ' --create-only' : '', ].join(''); execPrisma(cmd); } catch (err) { handleSubProcessError(err); } // Should we add this? if (!options.skipSeed && !options.createOnly) { await runSeed({ noWarnings: true, printStatus: true }, []); } }
85-102: Well-implemented post-reset seeding.The async conversion of
runResetand the conditional seeding logic (Lines 99-101) is clean and follows good practices. ThenoWarnings: trueandprintStatus: trueoptions are appropriate for this context.packages/cli/package.json (1)
37-37: Execa version 9.6.0 has no known security vulnerabilities; however, compatibility verification is incomplete.The security check passed with no known vulnerabilities for execa 9.6.0. The package requires Node.js "^18.19.0 || >=20.5.0", which are modern versions. However, the project does not declare explicit Node.js version requirements in
packages/cli/package.jsonor the rootpackage.json. Confirm that the project's actual runtime environment supports one of execa's required Node.js versions.
|
Claude Code is working… I'll analyze this and get back to you. |
Summary by CodeRabbit
New Features
db seedcommand to run seed scripts configured in package.json.--skip-seedto disable.--.Tests