-
Notifications
You must be signed in to change notification settings - Fork 120
fix: resolve project context for untitled query files #1842
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
base: master
Are you sure you want to change the base?
Changes from all commits
45646e1
a32285f
36c3bd0
f8e0a3f
a301aa9
859f43b
26dc36e
67d0516
5869473
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,11 +3,16 @@ import { RunModelType } from "@altimateai/dbt-integration"; | |
| import { Uri, window } from "vscode"; | ||
| import { GenerateModelFromSourceParams } from "../code_lens_provider/sourceModelCreationCodeLensProvider"; | ||
| import { DBTProjectContainer } from "../dbt_client/dbtProjectContainer"; | ||
| import { toProjectQuickPickItem } from "../quickpick/projectQuickPick"; | ||
| import { QueryManifestService } from "../services/queryManifestService"; | ||
| import { NodeTreeItem } from "../treeview_provider/modelTreeviewProvider"; | ||
| import { extendErrorWithSupportLinks } from "../utils"; | ||
|
|
||
| export class RunModel { | ||
| constructor(private dbtProjectContainer: DBTProjectContainer) {} | ||
| constructor( | ||
| private dbtProjectContainer: DBTProjectContainer, | ||
| private queryManifestService: QueryManifestService, | ||
| ) {} | ||
|
|
||
| runModelOnActiveWindow(type?: RunModelType) { | ||
| if (!window.activeTextEditor) { | ||
|
|
@@ -41,17 +46,63 @@ export class RunModel { | |
| this.compileDBTModel(fullPath); | ||
| } | ||
|
|
||
| compileQueryOnActiveWindow() { | ||
| async compileQueryOnActiveWindow() { | ||
| if (!window.activeTextEditor) { | ||
| return; | ||
| } | ||
| const fullPath = window.activeTextEditor.document.uri; | ||
| if (fullPath.scheme === "untitled") { | ||
| const resolved = await this.ensureProjectForUntitledUri(); | ||
| if (!resolved) { | ||
| return; | ||
| } | ||
| } | ||
| const query = window.activeTextEditor.document.getText(); | ||
| if (query !== undefined) { | ||
| this.compileDBTQuery(fullPath, query); | ||
| } | ||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * Ensures a dbt project is stored in workspace state for untitled files. | ||
| * If no project is stored yet — or the stored project is stale (removed | ||
| * between sessions) — falls back to getOrPickProjectFromWorkspace() which | ||
| * auto-selects the single project or prompts the user to pick one. | ||
| * Returns true if a project is available, false if the user cancelled or | ||
| * no projects exist. | ||
| */ | ||
| private async ensureProjectForUntitledUri(): Promise<boolean> { | ||
| const selectedProject = this.dbtProjectContainer.getFromWorkspaceState( | ||
| "dbtPowerUser.projectSelected", | ||
| ); | ||
| if (selectedProject?.uri) { | ||
| // Validate the stored project still exists in the workspace | ||
| const raw = selectedProject.uri; | ||
| // Workspace state deserializes Uri as a plain object — .fsPath is a | ||
| // getter on the Uri prototype and won't survive, so use .path. | ||
| const storedUri = Uri.file(raw.path); | ||
| if (this.dbtProjectContainer.findDBTProject(storedUri)) { | ||
| return true; | ||
| } | ||
| // Stale — clear it and fall through to re-pick | ||
| this.dbtProjectContainer.setToWorkspaceState( | ||
| "dbtPowerUser.projectSelected", | ||
| undefined, | ||
| ); | ||
| } | ||
shreyastelkar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| const project = | ||
| await this.queryManifestService.getOrPickProjectFromWorkspace(); | ||
| if (!project) { | ||
| window.showErrorMessage("Unable to find dbt project for this query."); | ||
| return false; | ||
| } | ||
| this.dbtProjectContainer.setToWorkspaceState( | ||
| "dbtPowerUser.projectSelected", | ||
| toProjectQuickPickItem(project), | ||
| ); | ||
| return true; | ||
| } | ||
|
|
||
| private getQuery() { | ||
| if (!window.activeTextEditor) { | ||
| return; | ||
|
|
@@ -62,16 +113,26 @@ export class RunModel { | |
| ); | ||
| } | ||
|
|
||
| executeQueryOnActiveWindow() { | ||
| async executeQueryOnActiveWindow() { | ||
| const query = this.getQuery(); | ||
| if (query === undefined) { | ||
| return; | ||
| } | ||
| const modelPath = window.activeTextEditor?.document.uri; | ||
| if (modelPath) { | ||
| const modelName = path.basename(modelPath.fsPath, ".sql"); | ||
| this.executeSQL(window.activeTextEditor!.document.uri, query, modelName); | ||
| if (!modelPath) { | ||
| return; | ||
| } | ||
| if (modelPath.scheme === "untitled") { | ||
| const resolved = await this.ensureProjectForUntitledUri(); | ||
| if (!resolved) { | ||
| return; | ||
| } | ||
| } | ||
| const modelName = | ||
| modelPath.scheme === "untitled" | ||
| ? "untitled" | ||
| : path.basename(modelPath.fsPath, ".sql"); | ||
| this.executeSQL(modelPath, query, modelName); | ||
| } | ||
|
|
||
| runModelOnNodeTreeItem(type: RunModelType) { | ||
|
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. nit: For untitled URIs, Not a correctness issue (execution still works), but could be a minor UX nit. Consider a fallback like: const modelName = modelPath.scheme === "untitled"
? "untitled"
: path.basename(modelPath.fsPath, ".sql");
Contributor
Author
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. Already addressed — see lines 129-132 in the current code: const modelName =
modelPath.scheme === "untitled"
? "untitled"
: path.basename(modelPath.fsPath, ".sql"); |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -102,6 +102,7 @@ export class DBTProjectContainer implements Disposable { | |
| this.projects.set(event.root, event.name); | ||
| } else { | ||
| this.projects.delete(event.root); | ||
| this.clearStaleProjectSelection(event.root); | ||
| } | ||
| }); | ||
| } | ||
|
|
@@ -240,14 +241,6 @@ export class DBTProjectContainer implements Disposable { | |
| } | ||
|
|
||
| executeSQL(uri: Uri, query: string, modelName: string): void { | ||
| if (uri.scheme === "untitled") { | ||
| const selectedProject = this.getFromWorkspaceState( | ||
| "dbtPowerUser.projectSelected", | ||
| ); | ||
| if (selectedProject) { | ||
| uri = selectedProject.uri; | ||
| } | ||
| } | ||
| this.findDBTProject(uri)?.executeSQLOnQueryPanel(query, modelName); | ||
| } | ||
|
|
||
|
|
@@ -302,7 +295,8 @@ export class DBTProjectContainer implements Disposable { | |
| } | ||
|
|
||
| findDBTProject(uri: Uri): DBTProject | undefined { | ||
| return this.findDBTWorkspaceFolder(uri)?.findDBTProject(uri); | ||
| const resolved = this.resolveProjectUri(uri); | ||
| return this.findDBTWorkspaceFolder(resolved)?.findDBTProject(resolved); | ||
| } | ||
|
|
||
| getProjects(): DBTProject[] { | ||
|
|
@@ -479,6 +473,48 @@ export class DBTProjectContainer implements Disposable { | |
| return this.dbtWorkspaceFolders.find((folder) => folder.contains(uri)); | ||
| } | ||
|
|
||
| /** | ||
| * Resolves an untitled document URI to the selected project's URI. | ||
| * When users create ad-hoc query files (via "New query" or manually), | ||
| * the document has an `untitled:` scheme that can't be matched to a | ||
| * project directory. This method looks up the stored project selection | ||
| * from workspace state and returns a real file URI that findDBTProject | ||
| * can resolve. | ||
| * | ||
| * Workspace state stores Uri as a plain JSON object (not a Uri instance), | ||
| * so we reconstruct it via Uri.file() to ensure .fsPath works correctly. | ||
| */ | ||
| private resolveProjectUri(uri: Uri): Uri { | ||
| if (uri.scheme === "untitled") { | ||
| const selectedProject = this.getFromWorkspaceState( | ||
| "dbtPowerUser.projectSelected", | ||
| ); | ||
| if (selectedProject?.uri) { | ||
| // Workspace state deserializes Uri as a plain object — .fsPath is a | ||
| // getter on the Uri prototype and won't survive, so use .path. | ||
| return Uri.file(selectedProject.uri.path); | ||
| } | ||
| } | ||
| return uri; | ||
| } | ||
|
|
||
| /** | ||
| * Clear the stored project selection if it points to a project that was | ||
| * just unregistered. Prevents stale state from causing silent failures | ||
| * when the user later opens an untitled query file. | ||
| */ | ||
| private clearStaleProjectSelection(removedRoot: Uri): void { | ||
| const selectedProject = this.getFromWorkspaceState( | ||
| "dbtPowerUser.projectSelected", | ||
| ); | ||
| if (!selectedProject?.uri) { | ||
| return; | ||
| } | ||
| if (Uri.file(selectedProject.uri.path).fsPath === removedRoot.fsPath) { | ||
| this.setToWorkspaceState("dbtPowerUser.projectSelected", undefined); | ||
| } | ||
| } | ||
|
|
||
|
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. thought: I verified that
Contributor
Author
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. Good catch documenting this. The exact === match on fsPath is intentional and consistent with how DBTWorkspaceFolder.contains() keys projects. |
||
| async checkIfAltimateDatapilotInstalled() { | ||
| const datapilotVersion = | ||
| await this.altimateDatapilot.checkIfAltimateDatapilotInstalled(); | ||
|
|
||
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.
This already has some significance in other screens. It was not required before. Any way to avoid this?
I thought it worked like this before:
if single project. -> use that project, if multiple projects -> show dropdown