Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Changes to allow reporting success with an action on sqlTasksService#20651

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.

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

Already on GitHub?Sign in to your account

Open
allancascante wants to merge6 commits intomain
base:main
Choose a base branch
Loading
fromdev/allancascante/notification_fixes

Conversation

@allancascante
Copy link
Contributor

@allancascanteallancascante commentedNov 20, 2025•
edited
Loading

Description

This PR refactors the task completion notification system to eliminate duplicate notifications for DacFx operations (Export BACPAC, Extract DACPAC, Import BACPAC, Deploy DACPAC) and provides a generic, configurable solution for custom task completion handlers, as reported in thisbug.

Problem
Previously, DacFx operations showed two notifications:

A custom notification with action button from dacpacDialogWebviewController
A generic completion notification from sqlTasksService

Solution

Centralized all task completion notification logic in sqlTasksService with a configurable handler registration system.

Code Changes Checklist

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

Reviewers:Please read our reviewer guidelines

@github-actions
Copy link

github-actionsbot commentedNov 20, 2025•
edited
Loading

PR Changes

CategoryTarget BranchPR BranchDifference
vscode-mssql VSIX5262 KB5262 KB⚪ 0 KB ( 0% )
sql-database-projects VSIX5627 KB5627 KB⚪ 0 KB ( 0% )

@codecov-commenter
Copy link

codecov-commenter commentedNov 20, 2025•
edited
Loading

Codecov Report

❌ Patch coverage is98.66667% with1 line in your changes missing coverage. Please review.
✅ Project coverage is 62.94%. Comparing base (3109cca) to head (fc1a170).
⚠️ Report is 2 commits behind head on main.

Files with missing linesPatch %Lines
extensions/mssql/src/controllers/mainController.ts0.00%1 Missing⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@##             main   #20651      +/-   ##==========================================+ Coverage   62.65%   62.94%   +0.29%==========================================  Files         209      209                Lines       19350    19390      +40       Branches     2461     2464       +3     ==========================================+ Hits        12124    12206      +82+ Misses       7146     7102      -44- Partials       80       82       +2
Files with missing linesCoverage Δ
extensions/mssql/src/constants/constants.ts100.00% <100.00%> (ø)
...l/src/controllers/dacpacDialogWebviewController.ts92.16% <ø> (+0.64%)⬆️
extensions/mssql/src/services/dacFxService.ts53.94% <100.00%> (+48.68%)⬆️
extensions/mssql/src/services/sqlTasksService.ts82.35% <100.00%> (+45.88%)⬆️
extensions/mssql/src/controllers/mainController.ts16.88% <0.00%> (ø)

... and1 file with indirect coverage changes

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

/**
* Helper method to register task completion handlers for DacPac operations
*/
privateregisterTaskCompletionHandlers():void{
Copy link
Member

Choose a reason for hiding this comment

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

If this method is only used for DACPAC-related completion handlers, could we rename it to something more specific like registerDacpacTaskCompletionHandlers?

Also, I’m wondering if we should start moving some of this logic out of mainController into feature-specific modules (e.g. a DACPAC-specific file) and just import/register them here. This file is starting to get quite large, and splitting by feature area might make it easier to maintain.

Benjin reacted with thumbs up emoji
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking this registration should be done by the dacpacDialogWebviewController when it launches the first time. It wouldn't need to unregister upon webview disposal because that would revert to the less helpful task completion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or maybe the dacfxService constructor would be even better.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@kburtramkburtramkburtram left review comments

@BenjinBenjinBenjin left review comments

@aasimkhan30aasimkhan30Awaiting requested review from aasimkhan30aasimkhan30 is a code owner

@caohaicaohaiAwaiting requested review from caohaicaohai is a code owner

@laurenastrid1laurenastrid1Awaiting requested review from laurenastrid1laurenastrid1 is a code owner

@lewis-sanchezlewis-sanchezAwaiting requested review from lewis-sanchezlewis-sanchez is a code owner

@ssreeramassreeramaAwaiting requested review from ssreeramassreerama is a code owner

@manujoseph85manujoseph85Awaiting requested review from manujoseph85manujoseph85 is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Data-tier Application dialog has duplicate notifications

4 participants

@allancascante@codecov-commenter@kburtram@Benjin
[8]ページ先頭
©2009-2025 Movatter.jp