Skip to content

feat: restructure db command files#8175

Merged
eduardoboucas merged 1 commit intomainfrom
feat/db-command-restructure
Apr 20, 2026
Merged

feat: restructure db command files#8175
eduardoboucas merged 1 commit intomainfrom
feat/db-command-restructure

Conversation

@eduardoboucas
Copy link
Copy Markdown
Member

Separates the legacy db commands from the new ones.

@eduardoboucas eduardoboucas requested a review from a team as a code owner April 20, 2026 21:48
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 20, 2026

📝 Walkthrough

Summary by CodeRabbit

Release Notes

  • Refactor
    • Reorganized database command module structure for improved maintainability, moving shared utilities to dedicated directories.
    • Updated default migrations directory path to netlify/database/migrations (from netlify/db/migrations).

Walkthrough

This pull request refactors the database command module structure by reorganizing files into legacy/ and util/ subdirectories. The main database.ts file's type exports (Extension and SiteInfo) are moved to a new legacy/types.ts file. The resolveMigrationsDirectory function and migration-related constants are extracted from individual command files into shared utilities (util/migrations-path.ts and util/constants.ts). Database command files are renamed with a db- prefix for consistency. Multiple import paths are updated throughout to reflect the new structure, and the default migrations directory path is standardized to netlify/database/migrations. Test files are updated to reference the new file paths and mock targets.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: restructuring database command files by separating legacy from new commands, as evidenced by the creation of legacy/ subdirectory and reorganization of imports throughout the codebase.
Description check ✅ Passed The description directly relates to the changeset, stating the intent to separate legacy db commands from new ones, which is reflected in the file reorganization and new legacy/ directory structure.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/db-command-restructure

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

📊 Benchmark results

Comparing with 5647420

  • Dependency count: 1,061 (no change)
  • Package size: 355 MB ⬆️ 0.00% increase vs. 5647420
  • Number of ts-expect-error directives: 356 (no change)

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
src/commands/database/util/migrations-path.ts (1)

5-11: Remove explanatory comments and rely on naming.

Lines 5-11 describe behavior inline; this conflicts with the repository style rule for TS/JS files.

♻️ Suggested cleanup
-// Default on-disk location for migration files, relative to the project root.
-// Can be overridden by setting `db.migrations.path` in `netlify.toml`.
 export const DEFAULT_MIGRATIONS_PATH = 'netlify/database/migrations'
-
-// Resolves the absolute path of the project's migrations directory. Prefers
-// the `db.migrations.path` override from `netlify.toml` when present; falls
-// back to `<project-root>/netlify/database/migrations`.
 export const resolveMigrationsDirectory = (command: BaseCommand): string => {

As per coding guidelines, **/*.{ts,tsx,js,jsx}: Never write comments on what the code does; make the code clean and self-explanatory instead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/database/util/migrations-path.ts` around lines 5 - 11, Remove
the explanatory block comments above DEFAULT_MIGRATIONS_PATH and the resolver
function so the file relies on self-documenting names; specifically delete the
multi-line comment describing default location and resolution behavior and keep
only the exported constant DEFAULT_MIGRATIONS_PATH (and the resolver code if
present) so the implementation reads cleanly without inline "what it does"
comments.
tests/unit/commands/database/db-status.test.ts (1)

90-105: Consider adding one explicit fallback-path test.

Because the helper defaults migrationsPath into config, most tests won’t exercise resolveMigrationsDirectory fallback behavior. Add one case with migrationsPath: null and assert the default path (/project/netlify/database/migrations) is used.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/commands/database/db-status.test.ts` around lines 90 - 105, The
tests never exercise the fallback in resolveMigrationsDirectory because
createMockCommand seeds config.migrations by default; add one unit test that
calls createMockCommand({ migrationsPath: null }) and invokes the code path that
calls resolveMigrationsDirectory (the db-status command test harness) and assert
the resolved migrations directory equals '/project/netlify/database/migrations';
reference the helper createMockCommand and the resolver
resolveMigrationsDirectory (or the db-status execution function) so the new test
explicitly verifies the fallback behavior.
tests/unit/commands/database/db-migrate.test.ts (1)

82-89: Error-message assertion can be tightened for this path change.

This test currently only matches 'No migrations directory found'. Consider also asserting 'netlify/database/migrations' so the new standardized default-path guidance is protected against regression.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/commands/database/db-migrate.test.ts` around lines 82 - 89, Update
the unit test in db-migrate.test.ts that calls migrate({}, command) so the
rejection assertion checks for the more specific error message including the
standardized path; change the expect(...).rejects.toThrow('No migrations
directory found') to assert the message includes both 'No migrations directory
found' and the default path string 'netlify/database/migrations' (e.g., toThrow
matching that combined text) so the failure protects against regressions to the
default path guidance in the migrate function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/commands/database/db-migrate.ts`:
- Around line 20-24: The error message is misleading because the code only uses
command.netlify.config.db?.migrations?.path (migrationsDirectory) and throws
even when the DEFAULT_MIGRATIONS_PATH exists; update the logic in db-migrate.ts
to resolve a fallback by checking the filesystem for DEFAULT_MIGRATIONS_PATH
when command.netlify.config.db?.migrations?.path is undefined or empty, set
migrationsDirectory to that default if it exists, and only throw the current
Error (mentioning DEFAULT_MIGRATIONS_PATH) if neither the configured path nor
the default folder is present; reference the migrationsDirectory variable,
DEFAULT_MIGRATIONS_PATH constant, and command.netlify.config access to locate
where to implement the filesystem existence check and fallback assignment.

---

Nitpick comments:
In `@src/commands/database/util/migrations-path.ts`:
- Around line 5-11: Remove the explanatory block comments above
DEFAULT_MIGRATIONS_PATH and the resolver function so the file relies on
self-documenting names; specifically delete the multi-line comment describing
default location and resolution behavior and keep only the exported constant
DEFAULT_MIGRATIONS_PATH (and the resolver code if present) so the implementation
reads cleanly without inline "what it does" comments.

In `@tests/unit/commands/database/db-migrate.test.ts`:
- Around line 82-89: Update the unit test in db-migrate.test.ts that calls
migrate({}, command) so the rejection assertion checks for the more specific
error message including the standardized path; change the
expect(...).rejects.toThrow('No migrations directory found') to assert the
message includes both 'No migrations directory found' and the default path
string 'netlify/database/migrations' (e.g., toThrow matching that combined text)
so the failure protects against regressions to the default path guidance in the
migrate function.

In `@tests/unit/commands/database/db-status.test.ts`:
- Around line 90-105: The tests never exercise the fallback in
resolveMigrationsDirectory because createMockCommand seeds config.migrations by
default; add one unit test that calls createMockCommand({ migrationsPath: null
}) and invokes the code path that calls resolveMigrationsDirectory (the
db-status command test harness) and assert the resolved migrations directory
equals '/project/netlify/database/migrations'; reference the helper
createMockCommand and the resolver resolveMigrationsDirectory (or the db-status
execution function) so the new test explicitly verifies the fallback behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 52f889cf-7bd6-42ff-b21a-343bc042cba0

📥 Commits

Reviewing files that changed from the base of the PR and between 5647420 and d2d40a7.

📒 Files selected for processing (25)
  • src/commands/database/database.ts
  • src/commands/database/db-connect.ts
  • src/commands/database/db-migrate.ts
  • src/commands/database/db-migration-new.ts
  • src/commands/database/db-migration-pull.ts
  • src/commands/database/db-reset.ts
  • src/commands/database/db-status.ts
  • src/commands/database/legacy/constants.ts
  • src/commands/database/legacy/db-init.ts
  • src/commands/database/legacy/db-status.ts
  • src/commands/database/legacy/drizzle.ts
  • src/commands/database/legacy/types.ts
  • src/commands/database/legacy/utils.ts
  • src/commands/database/util/applied-migrations.ts
  • src/commands/database/util/constants.ts
  • src/commands/database/util/db-connection.ts
  • src/commands/database/util/meta-commands.ts
  • src/commands/database/util/migrations-path.ts
  • src/commands/database/util/pg-client-executor.ts
  • src/commands/database/util/psql-formatter.ts
  • tests/unit/commands/database/db-migrate.test.ts
  • tests/unit/commands/database/db-migration-new.test.ts
  • tests/unit/commands/database/db-migration-pull.test.ts
  • tests/unit/commands/database/db-reset.test.ts
  • tests/unit/commands/database/db-status.test.ts
💤 Files with no reviewable changes (1)
  • src/commands/database/legacy/constants.ts

Comment on lines 20 to 24
const migrationsDirectory = command.netlify.config.db?.migrations?.path
if (!migrationsDirectory) {
throw new Error(
'No migrations directory found. Create a directory at netlify/db/migrations or set `db.migrations.path` in `netlify.toml`.',
`No migrations directory found. Create a directory at ${DEFAULT_MIGRATIONS_PATH} or set \`db.migrations.path\` in \`netlify.toml\`.`,
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Default-path guidance is misleading without actual fallback resolution.

At Line 20, the command only reads db.migrations.path from config, but Line 23 tells users to create ${DEFAULT_MIGRATIONS_PATH}. A project with only the default folder (and no config key) still errors here.

Suggested fix
-import { DEFAULT_MIGRATIONS_PATH } from './util/migrations-path.js'
+import { resolveMigrationsDirectory } from './util/migrations-path.js'
@@
-  const migrationsDirectory = command.netlify.config.db?.migrations?.path
-  if (!migrationsDirectory) {
-    throw new Error(
-      `No migrations directory found. Create a directory at ${DEFAULT_MIGRATIONS_PATH} or set \`db.migrations.path\` in \`netlify.toml\`.`,
-    )
-  }
+  const migrationsDirectory = resolveMigrationsDirectory(command)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/database/db-migrate.ts` around lines 20 - 24, The error message
is misleading because the code only uses
command.netlify.config.db?.migrations?.path (migrationsDirectory) and throws
even when the DEFAULT_MIGRATIONS_PATH exists; update the logic in db-migrate.ts
to resolve a fallback by checking the filesystem for DEFAULT_MIGRATIONS_PATH
when command.netlify.config.db?.migrations?.path is undefined or empty, set
migrationsDirectory to that default if it exists, and only throw the current
Error (mentioning DEFAULT_MIGRATIONS_PATH) if neither the configured path nor
the default folder is present; reference the migrationsDirectory variable,
DEFAULT_MIGRATIONS_PATH constant, and command.netlify.config access to locate
where to implement the filesystem existence check and fallback assignment.

@eduardoboucas eduardoboucas merged commit 794c2e0 into main Apr 20, 2026
69 checks passed
@eduardoboucas eduardoboucas deleted the feat/db-command-restructure branch April 20, 2026 22:40
eduardoboucas pushed a commit that referenced this pull request Apr 21, 2026
🤖 I have created a release *beep* *boop*
---


## [25.1.0](v25.0.1...v25.1.0)
(2026-04-21)


### Features

* add `db migrations reset` command
([#8177](#8177))
([3dd0f38](3dd0f38))
* add `db status` command
([#8173](#8173))
([9bccaf9](9bccaf9))
* Add deploy_source to CLI deploy requests (EX-2032)
([#8155](#8155))
([289933d](289933d))
* restructure db command files
([#8175](#8175))
([794c2e0](794c2e0))
* support `NETLIFY_DB_BRANCH` env var in `db status` command
([#8174](#8174))
([5647420](5647420))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: token-generator-app[bot] <82042599+token-generator-app[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants