From 77bb9780ecb826cea2f4dfdbde85ef6680b46283 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Tue, 10 Oct 2023 16:44:40 +0200 Subject: [PATCH] Fix confusion between modeling store an view states This fixes three bugs related to the modeling store and view states: - In the model editor view, when `setModeledMethods` was called, it would do it on the active database, instead of the database that the view was showing. This should not result in any visible bugs since the active database is always the one that is being shown (in theory), but I can imagine that it could cause issues if showing multiple model editors next to each other. - In the method modeling panel, the "reveal in editor" button would always show the already active model editor. Therefore, if you had multiple open and were still viewing the method of the first one, it would always show the second one. - In the method modeling panel, the same bug would cause the incorrect modeled methods to be updated. --- .../method-modeling-view-provider.ts | 25 ++++++++----------- .../src/model-editor/model-editor-view.ts | 9 ++----- .../src/model-editor/modeling-store.ts | 1 + 3 files changed, 13 insertions(+), 22 deletions(-) diff --git a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts index 362b53ba5..a3fb6bbcc 100644 --- a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts +++ b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts @@ -8,7 +8,7 @@ import { extLogger } from "../../common/logging/vscode/loggers"; import { App } from "../../common/app"; import { redactableError } from "../../common/errors"; import { Method } from "../method"; -import { DbModelingState, ModelingStore } from "../modeling-store"; +import { ModelingStore } from "../modeling-store"; import { AbstractWebviewViewProvider } from "../../common/vscode/abstract-webview-view-provider"; import { assertNever } from "../../common/helpers-pure"; import { ModelEditorViewTracker } from "../model-editor-view-tracker"; @@ -111,15 +111,17 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< break; case "setModeledMethod": { - const activeState = this.ensureActiveState(); + if (!this.databaseItem) { + return; + } this.modelingStore.updateModeledMethods( - activeState.databaseItem, + this.databaseItem, msg.method.signature, convertFromLegacyModeledMethod(msg.method), ); this.modelingStore.addModifiedMethod( - activeState.databaseItem, + this.databaseItem, msg.method.signature, ); break; @@ -140,10 +142,12 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< } private async revealInModelEditor(method: Method): Promise { - const activeState = this.ensureActiveState(); + if (!this.databaseItem) { + return; + } const views = this.editorViewTracker.getViews( - activeState.databaseItem.databaseUri.toString(), + this.databaseItem.databaseUri.toString(), ); if (views.length === 0) { return; @@ -152,15 +156,6 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< await Promise.all(views.map((view) => view.revealMethod(method))); } - private ensureActiveState(): DbModelingState { - const activeState = this.modelingStore.getStateForActiveDb(); - if (!activeState) { - throw new Error("No active state found in modeling store"); - } - - return activeState; - } - private registerToModelingStoreEvents(): void { this.push( this.modelingStore.onModeledMethodsChanged(async (e) => { diff --git a/extensions/ql-vscode/src/model-editor/model-editor-view.ts b/extensions/ql-vscode/src/model-editor/model-editor-view.ts index 50b24fdb2..eb1ab2ddb 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-view.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-view.ts @@ -664,16 +664,11 @@ export class ModelEditorView extends AbstractWebview< } private setModeledMethods(signature: string, methods: ModeledMethod[]) { - const state = this.modelingStore.getStateForActiveDb(); - if (!state) { - throw new Error("Attempting to set modeled method without active db"); - } - this.modelingStore.updateModeledMethods( - state.databaseItem, + this.databaseItem, signature, methods, ); - this.modelingStore.addModifiedMethod(state.databaseItem, signature); + this.modelingStore.addModifiedMethod(this.databaseItem, signature); } } diff --git a/extensions/ql-vscode/src/model-editor/modeling-store.ts b/extensions/ql-vscode/src/model-editor/modeling-store.ts index 6563d8b72..93cd48d25 100644 --- a/extensions/ql-vscode/src/model-editor/modeling-store.ts +++ b/extensions/ql-vscode/src/model-editor/modeling-store.ts @@ -347,6 +347,7 @@ export class ModelingStore extends DisposableObject { } return { + databaseItem: dbState.databaseItem, method: selectedMethod, usage: dbState.selectedUsage, modeledMethods: dbState.modeledMethods[selectedMethod.signature] ?? [],