-
Notifications
You must be signed in to change notification settings - Fork 98
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: bulk docs prop #1568
Merged
Merged
feat: bulk docs prop #1568
Changes from 32 commits
Commits
Show all changes
43 commits
Select commit
Hold shift + click to select a range
79b3f8d
refactor: simplify sync request handling in DocsEditViewPanel
AdiGajbhiye 4055ae7
refactor: optimize loading logic in DocumentationPropagationButton
AdiGajbhiye 4d86e95
feat: add Loader component and integrate into DocumentationPropagatio…
AdiGajbhiye 8d9b624
feat: implement cancellation logic in documentation propagation and i…
AdiGajbhiye b76807b
refactor: streamline column handling in DocumentationPropagationButto…
AdiGajbhiye 3b55605
feat: add bulk documentation propagation panel and update state manag…
AdiGajbhiye 647d7ad
feat: enhance DocumentationPropagationButton with Card component for …
AdiGajbhiye d783a30
feat: refactor DocumentationPropagationButton to use SingleColumnCard…
AdiGajbhiye 45c0d1d
feat: enhance BulkDocumentationPropagationPanel with loading state an…
AdiGajbhiye 1428e23
refactor: simplify column handling in BulkDocumentationPropagationPan…
AdiGajbhiye b11fe97
refactor: extract logic into useDocumentationPropagation hook for bet…
AdiGajbhiye 6bdc0f3
feat: implement mergeDocItems function and enhance useDocumentationPr…
AdiGajbhiye 52896fd
feat: add Select All and Unselect All buttons for column selection in…
AdiGajbhiye e99d753
feat: add dispatch for closing drawer and canceling column lineage in…
AdiGajbhiye 137c99b
refactor: streamline cancellation token handling in DbtLineageService…
AdiGajbhiye 3e14b4f
feat: reorganize layout and styling for BulkDocumentationPropagationP…
AdiGajbhiye d6d2e38
feat: enhance BulkDocumentationPropagationPanel with sorting and impr…
AdiGajbhiye 2ec0d7f
feat: conditionally render description in SingleColumnCard and adjust…
AdiGajbhiye e7947f2
feat: add loading state for column lineage in BulkDocumentationPropag…
AdiGajbhiye a39901d
feat: implement expandable rows in SingleColumnCard with loading stat…
AdiGajbhiye d9ea7d6
refactor: simplify state management by replacing reduce with Object.f…
AdiGajbhiye 12dae29
refactor: remove unused cancellation token logging in NewLineagePanel
AdiGajbhiye ba57bd2
feat: add telemetry event for bulk documentation save action
AdiGajbhiye d0ae564
feat: enhance DocumentationPropagationButton with data-testid attribu…
AdiGajbhiye 09ed21a
feat: add data-testid attributes to SingleColumnCard for improved tes…
AdiGajbhiye 3def681
feat: update propagateDocumentation calls to close drawer after execu…
AdiGajbhiye f81ee4e
feat: enhance bulk documentation save with success message for saved …
AdiGajbhiye 065e9eb
feat: add bulkDocsPropCredit method and telemetry for successful docu…
AdiGajbhiye 79df631
feat: remove isSaved state and related UI elements from documentation…
AdiGajbhiye 97cfa01
feat: remove isSaved state and success message from DocumentationProp…
AdiGajbhiye 311ade3
feat: update bulk documentation save to include number of columns in …
AdiGajbhiye bbb5b4e
fixes
mdesmet b558082
feat: refactor setSelectedColumns to use functional update for improv…
AdiGajbhiye 90d6b91
feat: remove SAVE_PROCEED action and related logic from Documentation…
AdiGajbhiye 363c1e0
feat: integrate bulk documentation properties management in Documenta…
AdiGajbhiye 6453333
feat: add support for single documentation properties panel in Docume…
AdiGajbhiye 41e6fc7
feat: simplify warning message in DocumentationProvider for unsaved c…
AdiGajbhiye 0edcce6
feat: enhance documentation propagation components with improved stat…
AdiGajbhiye baedd3f
feat: update documentation propagation logic to reset state and impro…
AdiGajbhiye 3dc39c7
feat: streamline documentation saving process by removing unnecessary…
AdiGajbhiye 502e26c
feat: reset tests and selected columns metadata in documentation prop…
AdiGajbhiye 408f172
feat: remove unnecessary await from information message display in Do…
AdiGajbhiye defe7f9
Fix string
mdesmet File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
import { existsSync, readFileSync, writeFileSync } from "fs"; | ||
import { | ||
CancellationToken, | ||
CancellationTokenSource, | ||
ColorThemeKind, | ||
Disposable, | ||
ProgressLocation, | ||
|
@@ -13,6 +14,7 @@ import { | |
WebviewViewResolveContext, | ||
window, | ||
workspace, | ||
env, | ||
} from "vscode"; | ||
import { DBTProjectContainer } from "../manifest/dbtProjectContainer"; | ||
import { | ||
|
@@ -110,6 +112,7 @@ export class DocsEditViewPanel implements WebviewViewProvider { | |
private eventMap: Map<string, ManifestCacheProjectAddedEvent> = new Map(); | ||
private _disposables: Disposable[] = []; | ||
private onMessageDisposable: Disposable | undefined; | ||
private cancellationTokenSource: CancellationTokenSource | undefined; | ||
|
||
public constructor( | ||
private dbtProjectContainer: DBTProjectContainer, | ||
|
@@ -682,15 +685,6 @@ export class DocsEditViewPanel implements WebviewViewProvider { | |
panel: this._panel, | ||
}); | ||
break; | ||
case "columnLineageBase": { | ||
this.dbtLineageService.handleColumnLineage(params, () => { | ||
this._panel?.webview.postMessage({ | ||
command: "columnLineage", | ||
args: { event: CllEvents.CANCEL }, | ||
}); | ||
}); | ||
break; | ||
} | ||
case "getDownstreamColumns": { | ||
const targets = params.targets as [string, string][]; | ||
const testsResult = await Promise.all( | ||
|
@@ -720,7 +714,7 @@ export class DocsEditViewPanel implements WebviewViewProvider { | |
this.handleSyncRequestFromWebview( | ||
syncRequestId, | ||
() => ({ column_lineage: [], tables: [], tests }), | ||
command, | ||
"response", | ||
); | ||
return; | ||
} | ||
|
@@ -729,23 +723,29 @@ export class DocsEditViewPanel implements WebviewViewProvider { | |
name: params.column as string, | ||
}; | ||
const currAnd1HopTables = [...tables, ...targets.map((t) => t[0])]; | ||
const columns = await this.dbtLineageService.getConnectedColumns({ | ||
targets, | ||
currAnd1HopTables, | ||
selectedColumn, | ||
upstreamExpansion: true, | ||
showIndirectEdges: false, | ||
eventType: "documentation_propagation", | ||
}); | ||
this.cancellationTokenSource = new CancellationTokenSource(); | ||
const columns = await this.dbtLineageService.getConnectedColumns( | ||
{ | ||
targets, | ||
currAnd1HopTables, | ||
selectedColumn, | ||
upstreamExpansion: true, | ||
showIndirectEdges: false, | ||
eventType: "documentation_propagation", | ||
}, | ||
this.cancellationTokenSource!, | ||
); | ||
this.handleSyncRequestFromWebview( | ||
syncRequestId, | ||
() => { | ||
return { ...columns, tables: _tables, tests }; | ||
}, | ||
command, | ||
() => ({ ...columns, tables: _tables, tests }), | ||
"response", | ||
); | ||
break; | ||
} | ||
case "cancelColumnLineage": { | ||
this.cancellationTokenSource?.cancel(); | ||
break; | ||
} | ||
case "saveDocumentation": | ||
this.telemetry.sendTelemetryEvent( | ||
TelemetryEvents["DocumentationEditor/SaveClick"], | ||
|
@@ -761,20 +761,37 @@ export class DocsEditViewPanel implements WebviewViewProvider { | |
break; | ||
case "saveDocumentationBulk": { | ||
this.telemetry.sendTelemetryEvent( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The previous implementation used window.withProgress to provide visual feedback during the bulk save operation. Removing this might affect user experience in cases where saving takes longer. Consider if a progress indicator is still needed. |
||
TelemetryEvents["DocumentationEditor/SaveClick"], | ||
TelemetryEvents["DocumentationEditor/SaveBulk"], | ||
); | ||
window.withProgress( | ||
const successfulSaves = await window.withProgress( | ||
{ | ||
title: "Saving documentation", | ||
location: ProgressLocation.Notification, | ||
cancellable: false, | ||
}, | ||
async () => { | ||
const successfulSaves: string[] = []; | ||
for (const item of message.models) { | ||
await this.saveDocumentation(item, syncRequestId); | ||
const model = await this.saveDocumentation( | ||
item, | ||
syncRequestId, | ||
); | ||
if (model) { | ||
successfulSaves.push(model); | ||
} | ||
} | ||
return successfulSaves; | ||
}, | ||
); | ||
if (successfulSaves.length > 0) { | ||
await window.showInformationMessage( | ||
`Successfully saved: ${Array.from(new Set(successfulSaves)).join(", ")}`, | ||
); | ||
this.altimateRequest.bulkDocsPropCredit({ | ||
num_columns: message.numColumns, | ||
session_id: env.sessionId, | ||
}); | ||
} | ||
break; | ||
} | ||
} | ||
|
@@ -983,6 +1000,7 @@ export class DocsEditViewPanel implements WebviewViewProvider { | |
}, | ||
}); | ||
} | ||
return this.documentation?.name; | ||
} catch (error) { | ||
this.transmitError(); | ||
this.telemetry.sendTelemetryError( | ||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add disposal of cancellation token source.
While the cancellation token implementation is good, the token source should be disposed of after use to prevent memory leaks.
Apply this diff to add proper disposal:
Also applies to: 745-748