DacPac Dialog Bug Fixes and Feature Flag Implementation by allancascante · Pull Request #20505 · microsoft/vscode-mssql

allancascante · 2025-11-08T02:48:46Z

Description

This PR addresses critical bug fixes identified during code review and implements a feature flag to gate the Data-tier Application dialog behind experimental features. This allows for controlled rollout and user opt-in while ensuring stability of the core extension.

Changes in this PR:

Feature Flag Implementation: This allows users to opt into the feature while ensuring it doesn't impact users who prefer stability over new features.
Enable the Feature:

{ "mssql.enableExperimentalFeatures": true}
After enabling, reload VS Code. The "Data-tier Applications..." menu item will appear in Object Explorer for Server and Database nodes. And the operations for the quick lunch for Data-tier will be available also.

Fixed System Databases in Dropdown ([Bug]: remove system databases from database dropdown #20461): Filters out master, tempdb, model, msdb from database dropdown
Fixed Success Notification and File Reveal ([Bug]: Success message needs to 'reveal in explorer' on export #20468): Added "Reveal in Explorer" button to Extract and Export success notifications
Shows filename in notification message for clarity
Users can quickly locate exported files without searching file system
Fixed "Learn More" Documentation Link ([Bug]: adding 'learn more' link #20466): Added link to Microsoft documentation after dialog subtitle
Fixed Operation Label Consistency ([Bug]: consistency in operation labels #20467): Standardized operation labels across UI
Consistent terminology: "Deploy", "Extract", "Import", "Export"
Fixed Database Dropdown Ordering ([Bug]: in export and extract interface, DB dropdown should be right after server #20460): Database dropdown now appears immediately after server selection
Fixed application version field does not do any semver validation ([Bug]: application version field does not do any semver validation #20459): application version field does semver validation now as required by the service.
Fixed database name field changes after selecting package file in deploy ([Bug]: database name field changes after selecting package file in deploy #20458):

Code Changes Checklist

New or updated unit tests added
All existing tests pass (npm run test)
Code follows contributing guidelines
Telemetry/logging updated if relevant
No regressions or UX breakage

Reviewers: Please read our reviewer guidelines

…rm.tsx Co-authored-by: Copilot <[email protected]>

Co-authored-by: Copilot <[email protected]>

Copilot

Pull Request Overview

This PR enhances the DACPAC/BACPAC dialog functionality by implementing experimental feature gating, improving user experience with application version validation and success notifications, and updating terminology from "Deploy" to "Publish" for consistency.

Implements experimental feature flag to conditionally register and display DACPAC/BACPAC commands
Adds application version format validation (n.n.n or n.n.n.n) for Extract operations
Introduces success notifications with "Reveal in Explorer" actions for Extract/Export operations

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/controllers/mainController.ts	Conditionally registers DACPAC commands based on experimental features flag and sets VS Code context for menu visibility
src/controllers/dacpacDialogWebviewController.ts	Adds success notifications for all operations, filters system databases from listings, and updates terminology in comments
src/reactviews/pages/DacpacDialog/dacpacDialogForm.tsx	Implements application version validation, reorders UI sections for better UX, and adds Learn More link
src/reactviews/pages/DacpacDialog/ApplicationInfoSection.tsx	Adds validation support with error display for application version field
src/reactviews/common/locConstants.ts	Updates terminology from "Deploy DACPAC" to "Publish DACPAC" and adds validation message
src/constants/locConstants.ts	Adds new localized success messages and validation error message
package.json	Updates menu visibility condition to include experimental features check
test/unit/mainController.test.ts	Updates tests to handle conditional command registration based on experimental features
test/unit/dacpacDialogWebviewController.test.ts	Updates test descriptions to use "Publish" terminology and adds stub for notification messages
localization/xliff/vscode-mssql.xlf	Updates localization entries for terminology changes and new messages
localization/l10n/bundle.l10n.json	Updates localization bundle with new and updated strings

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-actions · 2025-11-08T03:01:18Z

PR Changes

Category	Target Branch	PR Branch	Difference
Code Coverage	59.62%	59.62%	⚪ 0.00%
VSIX Size	5233 KB	5234 KB	⚪ 1 KB ( 0% )
Webview Bundle Size	5252 KB	5252 KB	⚪ 0 KB ( 0% )

codecov-commenter · 2025-11-08T03:01:43Z

Codecov Report

❌ Patch coverage is 67.64706% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.60%. Comparing base (609dc9a) to head (39ba016).

Files with missing lines	Patch %	Lines
src/controllers/mainController.ts	0.00%	7 Missing ⚠️
src/controllers/dacpacDialogWebviewController.ts	76.47%	4 Missing ⚠️

❌ Your patch status has failed because the patch coverage (67.64%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #20505   +/-   ##
=======================================
  Coverage   57.59%   57.60%           
=======================================
  Files         207      207           
  Lines       18798    18826   +28     
  Branches     1175     1178    +3     
=======================================
+ Hits        10827    10845   +18     
- Misses       7971     7981   +10

Files with missing lines	Coverage Δ
src/constants/locConstants.ts	`83.26% <100.00%> (+0.18%)`	⬆️
src/controllers/dacpacDialogWebviewController.ts	`91.40% <76.47%> (-0.96%)`	⬇️
src/controllers/mainController.ts	`16.86% <0.00%> (-0.41%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Benjin · 2025-11-10T18:05:42Z

+                        LocConstants.DacpacDialog.RevealInExplorer,
+                    )
+                    .then((selection) => {
+                        if (selection === LocConstants.DacpacDialog.RevealInExplorer) {


This should be "Reveal in Finder" on macOS and "Open Containing Folder" on Linux. #20511

Benjin · 2025-11-10T18:15:36Z

+            registerDacPacCommand(Constants.cmdDacpacDialog, DacPacDialogOperationType.Deploy);
+            registerDacPacCommand(Constants.cmdDeployDacpac, DacPacDialogOperationType.Deploy);
+            registerDacPacCommand(Constants.cmdExtractDacpac, DacPacDialogOperationType.Extract);
+            registerDacPacCommand(Constants.cmdImportBacpac, DacPacDialogOperationType.Import);
+            registerDacPacCommand(Constants.cmdExportBacpac, DacPacDialogOperationType.Export);


These commands need to be added to package.json's contributes/menus/commandPalette section with:

{ "command": "mssql.launchDacpacDialog", "when": "mssql.enableExperimentalFeatures" },

As is, these still show up in the command palette, but error out because the flag being off means they aren't actually registered.

#20512

Benjin · 2025-11-10T18:17:44Z

@@ -570,7 +570,7 @@
                },
                {


For follow-up: we should rename the commands so that they're grouped as a feature, e.g.:

mssql.launchDacpacDialog -> mssql.dacpacDialog.launch
mssql.deployDacpac -> mssql.dacpacDialog.deploy

#20513

Benjin · 2025-11-10T18:25:26Z

+        await vscode.commands.executeCommand(
+            "setContext",
+            "mssql.experimentalFeaturesEnabled",
+            this.isExperimentalEnabled,
+        );


This shouldn't be necessary for boolean config options. Look in package.json for "when": "mssql.enableRichExperiences"

this is needed to hide the menu option on OE

Benjin · 2025-11-10T18:27:22Z

+            <Field
+                label={locConstants.dacpacDialog.applicationVersionLabel}
+                validationMessage={versionValidation?.message}
+                validationState={versionValidation?.severity === "error" ? "error" : "none"}>


Why not just validationState={versionValidation?.severity}? All of versionValidation?.severity's potential values (error, warning, undefined) are valid for validationState.

Benjin · 2025-11-10T18:33:48Z

+        // Regex to match n.n.n.n format where n is one or more digits
+        // Allows 3 or 4 parts (1.0.0 or 1.0.0.0)
+        const versionRegex = /^\d+\.\d+\.\d+(\.\d+)?$/;


Can you double-check this validation with @llali? At the very least, I know 1.1 is also valid (but just 1 is not, nor is 1.1.1.1.1.1.1.1.1.1.1.1)

#20514

Benjin · 2025-11-10T18:35:52Z

-                    <div className={classes.description}>{locConstants.dacpacDialog.subtitle}</div>
+                    <div className={classes.description}>
+                        {locConstants.dacpacDialog.subtitle}{" "}
+                        <Link href="https://learn.microsoft.com/en-us/sql/tools/sql-database-projects/concepts/data-tier-applications/overview?view=sql-server-ver17">


We should extract this to src\reactviews\common\constants.ts

allancascante and others added 30 commits October 20, 2025 15:35

proof of concept

1b8d66b

small fix to enable button after success export

9b9e982

fix on select database by default

9d48496

fix on connecting when lunching from database

e79fedc

small fixes in ui

f2b99dd

making app and version optional

68f0924

small text change

9ca59e4

loc changes

d726761

Update src/reactviews/pages/DataTierApplication/dataTierApplicationFo…

ad3ce87

…rm.tsx Co-authored-by: Copilot <[email protected]>

Update src/controllers/dataTierApplicationWebviewController.ts

03f5260

Co-authored-by: Copilot <[email protected]>

fix on validation

3914a99

fixing test failing in pipeline

ab77938

small improvement in validation

fa3e245

clearing form after succesfully completing the operation

b74be0c

removing messages panel

c1ce2c4

removing message panel

21ac90a

changes on operation component

ea0ee8e

missing localization changes

35e4398

Merge branch 'main' into dev/allancascante/data-tier-application

fcd13a7

localization after merge

8ec36a3

refactor to move logic to controller

f219588

fix to clear databases when selected an non-active server

b2fad95

fixes in connection management

7ee8fd7

updates to localization

a9c794c

unit tests updates

6056efd

adding telemetry and aria-label to form

df8fc7a

updating test

f0b6a2a

adding extra tests

5e59d0c

Merge branch 'main' into dev/allancascante/data-tier-application

96ba664

adding support to suggest names

390ef10

allancascante added 3 commits November 7, 2025 17:11

fixes after merge

d584e7b

use feature flag to hide the dacpac dialog

1cbf119

Merge branch 'main' into dev/allancascante/dacpac_dialog_fixes

39ba016

allancascante requested review from Benjin, aasimkhan30, caohai, kburtram, laurenastrid1, lewis-sanchez, manujoseph85 and ssreerama as code owners November 8, 2025 02:48

allancascante requested a review from Copilot November 8, 2025 02:54

Copilot AI reviewed Nov 8, 2025

View reviewed changes

Comment thread localization/l10n/bundle.l10n.json

Comment thread src/reactviews/pages/DacpacDialog/dacpacDialogForm.tsx

Comment thread src/constants/locConstants.ts

Comment thread localization/xliff/vscode-mssql.xlf

Benjin approved these changes Nov 10, 2025

View reviewed changes

allancascante merged commit a86d4d7 into main Nov 10, 2025
9 checks passed

allancascante deleted the dev/allancascante/dacpac_dialog_fixes branch November 10, 2025 18:44

allancascante mentioned this pull request Nov 10, 2025

Follow up fixes after PR merge #20519

Merged

5 tasks

Conversation

allancascante commented Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Code Changes Checklist

Reviewers: Please read our reviewer guidelines

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Nov 8, 2025

PR Changes

Uh oh!

codecov-commenter commented Nov 8, 2025

Codecov Report

Uh oh!

Benjin Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Benjin Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Benjin Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Benjin Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

allancascante Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Benjin Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Benjin Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Benjin Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

allancascante commented Nov 8, 2025 •

edited

Loading

Benjin Nov 10, 2025 •

edited

Loading

Benjin Nov 10, 2025 •

edited

Loading

Benjin Nov 10, 2025 •

edited

Loading

Benjin Nov 10, 2025 •

edited

Loading