Skip to content

fix: don't error remote database status command if database was not enabled yet to provide more meaningful output #8218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pieh wants to merge 4 commits into
base: main
Choose a base branch
from
+29 −2
Open

fix: don't error remote database status command if database was not enabled yet to provide more meaningful output #8218

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ae8c1fc
fix: don't error remote database status if database was not enabled y…
pieh Apr 28, 2026
4ea63a0
test: adjust not throwing assertion
pieh Apr 28, 2026
d0e7e94
Merge branch 'main' into fix/database-status-branch-not-yet-created
pieh Apr 28, 2026
d908cbd
Merge branch 'main' into fix/database-status-branch-not-yet-created
pieh Apr 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/commands/database/db-status.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,9 +410,16 @@ export const statusDb = async (options: DatabaseStatusOptions, command: BaseComm
throw new Error(`You must be logged in with ${netlifyCommand()} login to target a remote branch.`)
}

connectionString = await fetchBranchConnectionString({ siteId, accessToken, basePath }, branch)
fetchApplied = remoteAppliedMigrations({ siteId, accessToken, basePath, branch })
branchLabel = branch
if (enabled) {
connectionString = await fetchBranchConnectionString({ siteId, accessToken, basePath }, branch)
fetchApplied = remoteAppliedMigrations({ siteId, accessToken, basePath, branch })
} else {
// When the database isn't enabled, skip the remote fetch — the branch endpoint would 404 with
// a non-actionable error. The pending-migrations hint below directs users to install
// @netlify/database and deploy, which is the actionable next step.
fetchApplied = () => Promise.resolve([])
Comment on lines +418 to +421
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 28, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove explanatory inline comments and keep intent in code shape

This block introduces comments that explain behavior; please keep this self-explanatory via code and remove these comments.

Suggested cleanup 
-    } else {
-      // When the database isn't enabled, skip the remote fetch — the branch endpoint would 404 with
-      // a non-actionable error. The pending-migrations hint below directs users to install
-      // `@netlify/database` and deploy, which is the actionable next step.
-      fetchApplied = () => Promise.resolve([])
-    }
+    } else {
+      fetchApplied = async () => []
+    }

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/db-status.ts` around lines 418 - 421, Remove the
explanatory inline comments and make the intent explicit in code: replace the
commented assignment with a clearly named noop function (e.g., const
noopFetchApplied = async () => []; ) and assign fetchApplied = noopFetchApplied
(or directly use fetchApplied = async () => []), removing the comment lines
entirely so the code reads self-documentingly; update any nearby references to
use the new symbol if introduced.

}
} else {
isLocal = true
branchLabel = 'local'
Expand Down
20 changes: 20 additions & 0 deletions tests/unit/commands/database/db-status.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,26 @@ describe('statusDb', () => {
expect(jsonMessages[0]).toMatchObject({ target: 'feature-x' })
})

test('does not fetch branch connection string or remote applied migrations when database is not enabled', async () => {
setupFetchRouter({ siteDatabase: null })

await expect(statusDb({ branch: 'feature-x' }, createMockCommand())).resolves.not.toThrow()
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

const fetchedUrls = mockFetch.mock.calls.map((c) => {
const u = c[0] as URL | string
return typeof u === 'string' ? u : u.toString()
})
expect(fetchedUrls.some((u) => u.includes('/database/branch/'))).toBe(false)
expect(fetchedUrls.some((u) => u.includes('/database/migrations'))).toBe(false)
expect(mockConnectToDatabase).not.toHaveBeenCalled()

const output = logMessages.join('\n')
expect(output).toContain('Netlify Database is not enabled for this project')
expect(output).toContain('Applied migrations (0)')
expect(output).toContain('Pending migrations (0)')
expect(output).not.toContain('Connected to database branch')
})

test('throws a helpful error when the branch endpoint 404s', async () => {
setupFetchRouter({ siteDatabase: { connection_string: PROD_CONN } })

Expand Down
Loading