Skip to content

Publish Dialog - Adding Target field functionality #20217

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

Jump to bottom
Merged
merged 66 commits into from
Oct 20, 2025
Merged

Publish Dialog - Adding Target field functionality #20217

merged 66 commits into from
Oct 20, 2025

Conversation

ssreerama
Copy link
Contributor

@ssreerama ssreerama commented Sep 23, 2025 •
edited
Loading

Pull Request Template – vscode-mssql

Description

This pull request adds the target field appearance and validations on target option selection.

  • When the local container option is selected, four new fields appears with validations
    • Port, Admin password, confirm admin password and docker image
  • Added tests more maintainable and focused.
  • Additionally, new form fields, validation messages, and labels have been added for container and Azure SQL server scenarios.
  • toggle enable and disable Publish and script generation buttons based on target field selection and with successful validation

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

image

1 TargetSection

@ssreerama
intial changes to load empty dialog
a4159d7
@ssreerama
UI intial changes
cc2167f
@ssreerama
advanced options added
94f8dae
@ssreerama
basic UI final changes
463b2e8
@ssreerama
UI changes with formwebviewcontroller
f75c54c
@ssreerama
Basic UI final changes
a6c2545
@ssreerama
Merge branch 'sai/vscodePublishDialog' of https://github.com/microsof…
90844a1 
…t/vscode-mssql into sai/vscodePublishDialog
@ssreerama
removed Advanced related changes from the current set
6e1970c
@ssreerama
converting const to static class
13b93a9
@ssreerama
localized files
28fc897
@ssreerama
copilot comments adjusted
0c24390
@ssreerama
using new webview provider
6c85bdd
@ssreerama
updating the code
c091477
@ssreerama
Adding publish dialog basic test file
8f586c8
@ssreerama
Merge branch 'main' into sai/vscodePublishDialog
0e24de1
@ssreerama
adding target field changes
6e5aa69
@ssreerama
revert service instnace update and add loc fiels
bba6d85
@ssreerama
addressing code review comments
3f7eeff
@ssreerama
Merge branch 'main' into sai/vscodePublishDialog
7d95c74
@ssreerama
Merge branch 'sai/vscodePublishDialog' into sai/vscodePublishDialog_1…
4415ad2 
…Target
@ssreerama
adding tests that covers the target field
db5f691
@ssreerama ssreerama requested a review from Copilot September 23, 2025 05:43
Benjin
Benjin reviewed Oct 17, 2025
View reviewed changes
Benjin
Benjin reviewed Oct 17, 2025
View reviewed changes
Benjin
Benjin reviewed Oct 17, 2025
View reviewed changes
Benjin
Benjin reviewed Oct 17, 2025
View reviewed changes
Benjin
Benjin reviewed Oct 17, 2025
View reviewed changes
Benjin
Benjin reviewed Oct 17, 2025
View reviewed changes
ssreerama
ssreerama commented Oct 20, 2025
View reviewed changes
@Benjin Benjin self-requested a review October 20, 2025 17:59
Benjin
Benjin reviewed Oct 20, 2025
View reviewed changes
@ssreerama ssreerama requested review from Copilot October 20, 2025 18:02
Copilot
Copilot AI reviewed Oct 20, 2025
View reviewed changes
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Benjin
Benjin reviewed Oct 20, 2025
View reviewed changes
src/deployment/dockerUtils.ts
* @param targetVersion - The SQL Server version (e.g., "160" for SQL Server 2022)
* @returns Sorted array of tags filtered by version and organized by year
*/
export async function getSqlServerContainerTagsForTargetVersion(
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also be confirmed with Iqra and (just as importantly) @croblesm. In the container deployment UX, we are intentionally only allowing users to select the latest/GAed tags for each version because we're told we should not be providing a way for users to install version of SQL that Microsoft doesn't provide support for. By that understanding, we shouldn't be pulling all the tags, and should just provide options for GA versions, which means this can likely just be hardcoded.

Benjin
Benjin reviewed Oct 20, 2025
View reviewed changes
src/publishProject/projectUtils.ts
export function getPublishServerName(target: string) {
return target ===
targetPlatformToVersion.get("Azure SQL Database" /* SqlTargetPlatform.sqlAzure */)
return target === targetPlatformToVersion.get(SqlTargetPlatform.sqlAzure)
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, what is the target platform value for Fabric? Is that still sqlAzure?

Benjin
Benjin reviewed Oct 20, 2025
View reviewed changes
src/publishProject/publishProjectWebViewController.ts
// Use the deployment UI function with target version filtering
const tagComponent = this.state.formComponents[PublishFormFields.ContainerImageTag];
if (tagComponent) {
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should end up being removed to just use -latest for all, but waiting PM confirmation.

Benjin
Benjin previously approved these changes Oct 20, 2025
View reviewed changes
Copilot
Copilot AI reviewed Oct 20, 2025
View reviewed changes
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@ssreerama ssreerama merged commit b310cc9 into main Oct 20, 2025
3 checks passed
@ssreerama ssreerama deleted the sai/vscodePublishDialog_1Target branch October 20, 2025 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@aasimkhan30 aasimkhan30 aasimkhan30 approved these changes

@Benjin Benjin Benjin left review comments

@kburtram kburtram Awaiting requested review from kburtram kburtram is a code owner

@caohai caohai Awaiting requested review from caohai caohai is a code owner

@laurenastrid1 laurenastrid1 Awaiting requested review from laurenastrid1 laurenastrid1 is a code owner

@allancascante allancascante Awaiting requested review from allancascante allancascante is a code owner

@manujoseph85 manujoseph85 Awaiting requested review from manujoseph85 manujoseph85 is a code owner

@lewis-sanchez lewis-sanchez Awaiting requested review from lewis-sanchez lewis-sanchez is a code owner

Assignees

@ssreerama ssreerama

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ssreerama @Benjin @codecov-commenter @aasimkhan30 @lewis-sanchez