Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 9 additions & 6 deletions src/commands/database/status-db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -361,24 +361,27 @@ export const statusDb = async (options: DatabaseStatusOptions, command: BaseComm
}
const enabled = Boolean(envUrl) || siteHasDatabase

// `--branch` can also be supplied via the NETLIFY_DB_BRANCH env var.
const branch = options.branch ?? process.env.NETLIFY_DB_BRANCH
Comment on lines +364 to +365
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 | 🟡 Minor

Remove explanatory inline behavior comment.

Line 364-365 documents what the code does; this should be expressed by code only.

Suggested cleanup
-  // `--branch` can also be supplied via the NETLIFY_DB_BRANCH env var.
   const branch = options.branch ?? process.env.NETLIFY_DB_BRANCH

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

📝 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.

Suggested change
// `--branch` can also be supplied via the NETLIFY_DB_BRANCH env var.
const branch = options.branch ?? process.env.NETLIFY_DB_BRANCH
const branch = options.branch ?? process.env.NETLIFY_DB_BRANCH
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/database/status-db.ts` around lines 364 - 365, Remove the
explanatory inline comment that duplicates behavior for branch resolution;
delete the comment line preceding the declaration so the code reads simply as
const branch = options.branch ?? process.env.NETLIFY_DB_BRANCH (no comment
needed). Locate the comment that says "`--branch` can also be supplied via the
NETLIFY_DB_BRANCH env var." immediately above the branch assignment and remove
it, leaving the assignment as the self-explanatory implementation.


// Resolve what database we're looking at and how to fetch applied migrations.
let branchLabel: string
let connectionString: string | null = null
let fetchApplied: AppliedMigrationsFetcher
let cleanup: (() => Promise<void>) | undefined
let isLocal = false

if (options.branch) {
if (branch) {
if (!siteId) {
throw new Error(`The project must be linked with ${netlifyCommand()} link to use --branch.`)
throw new Error(`The project must be linked with ${netlifyCommand()} link to target a remote branch.`)
}
if (!accessToken) {
throw new Error(`You must be logged in with ${netlifyCommand()} login to use --branch.`)
throw new Error(`You must be logged in with ${netlifyCommand()} login to target a remote branch.`)
}

connectionString = await fetchBranchConnectionString({ siteId, accessToken, basePath }, options.branch)
fetchApplied = remoteAppliedMigrations({ siteId, accessToken, basePath, branch: options.branch })
branchLabel = options.branch
connectionString = await fetchBranchConnectionString({ siteId, accessToken, basePath }, branch)
fetchApplied = remoteAppliedMigrations({ siteId, accessToken, basePath, branch })
branchLabel = branch
} else {
isLocal = true
branchLabel = 'local'
Expand Down
29 changes: 29 additions & 0 deletions tests/unit/commands/database/status-db.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,12 @@ beforeEach(() => {
mockPackageJson({ dependencies: { '@netlify/database': '^1.0.0' } })
setLocalDatabaseRunning(LOCAL_CONN_WITH_CREDS)
delete process.env.NETLIFY_DB_URL
delete process.env.NETLIFY_DB_BRANCH
})

afterEach(() => {
delete process.env.NETLIFY_DB_URL
delete process.env.NETLIFY_DB_BRANCH
})

describe('statusDb', () => {
Expand Down Expand Up @@ -632,5 +634,32 @@ describe('statusDb', () => {

expect(logMessages.join('\n')).not.toContain('netlify db migrations apply')
})

test('falls back to NETLIFY_DB_BRANCH env var when --branch is not passed', async () => {
process.env.NETLIFY_DB_BRANCH = 'feature-env'
setupFetchRouter({
siteDatabase: { connection_string: PROD_CONN },
branch: { 'feature-env': { connection_string: BRANCH_CONN } },
migrations: { 'feature-env': [] },
})

await statusDb({ json: true }, createMockCommand())

expect(jsonMessages[0]).toMatchObject({ target: 'feature-env' })
expect(mockConnectToDatabase).not.toHaveBeenCalled()
})

test('--branch wins over NETLIFY_DB_BRANCH when both are set', async () => {
process.env.NETLIFY_DB_BRANCH = 'env-branch'
setupFetchRouter({
siteDatabase: { connection_string: PROD_CONN },
branch: { 'flag-branch': { connection_string: BRANCH_CONN } },
migrations: { 'flag-branch': [] },
})

await statusDb({ branch: 'flag-branch', json: true }, createMockCommand())

expect(jsonMessages[0]).toMatchObject({ target: 'flag-branch' })
})
})
})
Loading