[Patch agent] Logic for when to show workspace selection dialog

- Persist selected workspace in a setting
- Automatically select previously selected workspace
- If no previously selected workspace -> don't automatically select
  any workspace
- Track potential workspace deletion
- Only show 'change' button if a workspace is selected
- On 'apply' button if no workspace selected yet, show selection
  dialog and then immediately progress to applying changes

Bug: 393266352
Change-Id: If20bd4eda982e5279319d6a9a08d949d31ac147a
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6348458
Reviewed-by: Ergün Erdoğmuş <[email protected]>
Commit-Queue: Wolfgang Beyer <[email protected]>

Author: wolfib

front_end/panels/ai_assistance/PatchWidget.test.ts

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import * as Common from '../../core/common/common.js';
+import type * as Persistence from '../../models/persistence/persistence.js';
 import * as Workspace from '../../models/workspace/workspace.js';
 import * as PanelCommon from '../../panels/common/common.js';
 import {
@@ -15,6 +16,8 @@ import {
 import {updateHostConfig} from '../../testing/EnvironmentHelpers.js';
 import {describeWithMockConnection} from '../../testing/MockConnection.js';
 
+import * as AiAssistance from './ai_assistance.js';
+
 describeWithMockConnection('PatchWidget', () => {
   let showFreDialogStub: sinon.SinonStub<Parameters<typeof PanelCommon.FreDialog.show>, Promise<boolean>>;
   beforeEach(() => {
@@ -31,6 +34,7 @@ describeWithMockConnection('PatchWidget', () => {
   describe('applyToWorkspace', () => {
     beforeEach(() => {
       createTestFilesystem('file://test');
+      Common.Settings.Settings.instance().createSetting('ai-assistance-patching-selected-project-id', 'file://test');
       updateHostConfig({
         devToolsFreestyler: {
           enabled: true,
@@ -79,44 +83,97 @@ Files:
   });
 
   describe('workspace', () => {
-    it('does not report a workspace project if disabled', async () => {
-      createTestFilesystem('file://test');
+    let project: Persistence.FileSystemWorkspaceBinding.FileSystem;
+
+    beforeEach(() => {
+      project = createTestFilesystem('file://test').project;
       updateHostConfig({
         devToolsFreestyler: {
           enabled: true,
-          patching: false,
+          patching: true,
         },
       });
-      const {view} = await createPatchWidget();
-      assert.isUndefined(view.input.projectName);
     });
 
-    it('reports a current workspace project', async () => {
-      createTestFilesystem('file://test');
+    it('does not select a workspace project if patching is disabled', async () => {
       updateHostConfig({
         devToolsFreestyler: {
           enabled: true,
-          patching: true,
+          patching: false,
         },
       });
       const {view} = await createPatchWidget();
+      assert.isUndefined(view.input.projectName);
+    });
+
+    it('does not select a workspace project if setting does not exist', async () => {
+      const {view} = await createPatchWidget();
+      assert.isUndefined(view.input.projectName);
+    });
+
+    it('selects a workspace project matching the setting', async () => {
+      Common.Settings.Settings.instance().createSetting('ai-assistance-patching-selected-project-id', 'file://test');
+      const {view} = await createPatchWidget();
       assert.strictEqual(view.input.projectName, 'test');
     });
 
-    it('reports an updated project', async () => {
-      const {project} = createTestFilesystem('file://test');
-      updateHostConfig({
-        devToolsFreestyler: {
-          enabled: true,
-          patching: true,
-        },
-      });
+    it('removes a selected workspace project upon workspace removal', async () => {
+      Common.Settings.Settings.instance().createSetting('ai-assistance-patching-selected-project-id', 'file://test');
       const {view} = await createPatchWidget();
       assert.strictEqual(view.input.projectName, 'test');
 
       Workspace.Workspace.WorkspaceImpl.instance().removeProject(project);
-      createTestFilesystem('file://test2');
-      assert.strictEqual((await view.nextInput).projectName, 'test2');
+      const input = await view.nextInput;
+      assert.isUndefined(input.projectName);
+    });
+
+    it('selection is triggered by applyToWorkspace click if no workspace is (pre-)selected', async () => {
+      let handler: (project: Workspace.Workspace.Project) => void = () => {};
+      const showSelectWorkspaceDialogStub =
+          sinon.stub(AiAssistance.SelectWorkspaceDialog, 'show').callsFake((handleProjectSelected, _project) => {
+            handler = handleProjectSelected;
+          });
+      const {view, panel} = await createPatchWidget({aidaClient: mockAidaClient([[{explanation: 'suggested patch'}]])});
+      panel.changeSummary = 'body { background-color: red; }';
+      assert.isUndefined(view.input.projectName);
+
+      // Simulate clicking the "Apply to workspace" button
+      view.input.onApplyToWorkspace();
+      await new Promise(resolve => setTimeout(resolve, 0));
+      assert.isTrue(showSelectWorkspaceDialogStub.calledOnce);
+
+      // Simulate selecting a workspace with the SelectWorkspaceDialog
+      handler(project);
+      const input = await view.nextInput;
+
+      // Assert that a patch has been generated and a project has been selected
+      assert.strictEqual(input.patchSuggestion, 'suggested patch');
+      assert.strictEqual(input.projectName, 'test');
+    });
+
+    it('selection is triggered by the "change"-button if a workspace is already (pre-)selected', async () => {
+      const {project: project2} = createTestFilesystem('file://test2');
+      Common.Settings.Settings.instance().createSetting('ai-assistance-patching-selected-project-id', 'file://test');
+      let handler: (project: Workspace.Workspace.Project) => void = () => {};
+      const showSelectWorkspaceDialogStub =
+          sinon.stub(AiAssistance.SelectWorkspaceDialog, 'show').callsFake((onProjectSelected, _project) => {
+            handler = onProjectSelected;
+          });
+      const {view, panel} = await createPatchWidget();
+      panel.changeSummary = 'body { background-color: red; }';
+      assert.strictEqual(view.input.projectName, 'test');
+
+      // Simulate clicking the "Change" button
+      assert.isTrue(showSelectWorkspaceDialogStub.notCalled);
+      view.input.onChangeWorkspaceClick();
+      assert.isTrue(showSelectWorkspaceDialogStub.calledOnce);
+
+      // Simulate selecting a different workspace with the SelectWorkspaceDialog
+      handler(project2);
+      const input = await view.nextInput;
+
+      // Assert that the project has been updated
+      assert.strictEqual(input.projectName, 'test2');
     });
   });
 });

front_end/panels/ai_assistance/PatchWidget.ts

@@ -139,6 +139,8 @@ export class PatchWidget extends UI.Widget.Widget {
   // Whether the user completed first run experience dialog or not.
   #aiPatchingFreCompletedSetting =
       Common.Settings.Settings.instance().createSetting('ai-assistance-patching-fre-completed', false);
+  #projectIdSetting =
+      Common.Settings.Settings.instance().createSetting('ai-assistance-patching-selected-project-id', '');
   #view: View;
   #viewOutput: ViewOutput = {};
   #aidaClient: Host.AidaClient.AidaClient;
@@ -156,6 +158,7 @@ export class PatchWidget extends UI.Widget.Widget {
   }) {
     super(false, false, element);
     this.#aidaClient = opts?.aidaClient ?? new Host.AidaClient.AidaClient();
+
     // clang-format off
     this.#view = view ?? ((input, output, target) => {
       if (!input.changeSummary) {
@@ -257,17 +260,19 @@ export class PatchWidget extends UI.Widget.Widget {
 
         return html`
         <div class="footer">
-          <div class="change-workspace">
-            <div class="selected-folder">
-              <devtools-icon .name=${'folder'}></devtools-icon> <span title=${input.projectPath}>${input.projectName}</span>
+          ${input.projectName ? html`
+            <div class="change-workspace">
+              <div class="selected-folder">
+                <devtools-icon .name=${'folder'}></devtools-icon> <span title=${input.projectPath}>${input.projectName}</span>
+              </div>
+              <devtools-button
+                @click=${input.onChangeWorkspaceClick}
+                .jslogContext=${'change-workspace'}
+                .variant=${Buttons.Button.Variant.TEXT}>
+                  ${lockedString(UIStringsNotTranslate.change)}
+              </devtools-button>
             </div>
-            <devtools-button
-              @click=${input.onChangeWorkspaceClick}
-              .jslogContext=${'change-workspace'}
-              .variant=${Buttons.Button.Variant.TEXT}>
-                ${lockedString(UIStringsNotTranslate.change)}
-            </devtools-button>
-          </div>
+          ` : nothing}
           <div class="apply-to-workspace-container">
             ${input.patchSuggestionLoading ? html`
               <div class="loading-text-container">
@@ -333,22 +338,6 @@ export class PatchWidget extends UI.Widget.Widget {
     this.requestUpdate();
   }
 
-  #onChangeWorkspaceClick(): void {
-    const dialog = new UI.Dialog.Dialog('select-workspace');
-    dialog.setMaxContentSize(new UI.Geometry.Size(384, 340));
-    dialog.setSizeBehavior(UI.GlassPane.SizeBehavior.SET_EXACT_WIDTH_MAX_HEIGHT);
-    dialog.setDimmed(true);
-
-    const handleProjectSelected = (project: Workspace.Workspace.Project): void => {
-      this.#project = project;
-      this.requestUpdate();
-    };
-
-    new SelectWorkspaceDialog({dialog, handleProjectSelected, currentProject: this.#project})
-        .show(dialog.contentElement);
-    dialog.show();
-  }
-
   #onLearnMoreTooltipClick(): void {
     this.#viewOutput.tooltipRef?.value?.hidePopover();
     void UI.ViewManager.ViewManager.instance().showView('chrome-ai');
@@ -373,7 +362,7 @@ export class PatchWidget extends UI.Widget.Widget {
           },
           onDiscard: this.#onDiscard.bind(this),
           onSaveAll: this.#onSaveAll.bind(this),
-          onChangeWorkspaceClick: this.#onChangeWorkspaceClick.bind(this),
+          onChangeWorkspaceClick: this.#showSelectWorkspaceDialog.bind(this, {applyPatch: false}),
         },
         this.#viewOutput, this.contentElement);
   }
@@ -383,8 +372,7 @@ export class PatchWidget extends UI.Widget.Widget {
     this.#selectDefaultProject();
 
     if (isAiAssistancePatchingEnabled()) {
-      this.#workspace.addEventListener(Workspace.Workspace.Events.ProjectAdded, this.#onProjectAddedOrRemoved, this);
-      this.#workspace.addEventListener(Workspace.Workspace.Events.ProjectRemoved, this.#onProjectAddedOrRemoved, this);
+      this.#workspace.addEventListener(Workspace.Workspace.Events.ProjectRemoved, this.#onProjectRemoved, this);
 
       // @ts-expect-error temporary global function for local testing.
       window.aiAssistanceTestPatchPrompt = async (changeSummary: string) => {
@@ -395,9 +383,7 @@ export class PatchWidget extends UI.Widget.Widget {
 
   override willHide(): void {
     if (isAiAssistancePatchingEnabled()) {
-      this.#workspace.removeEventListener(Workspace.Workspace.Events.ProjectAdded, this.#onProjectAddedOrRemoved, this);
-      this.#workspace.removeEventListener(
-          Workspace.Workspace.Events.ProjectRemoved, this.#onProjectAddedOrRemoved, this);
+      this.#workspace.removeEventListener(Workspace.Workspace.Events.ProjectRemoved, this.#onProjectRemoved, this);
     }
   }
 
@@ -443,41 +429,42 @@ export class PatchWidget extends UI.Widget.Widget {
   }
 
   #selectDefaultProject(): void {
-    if (isAiAssistancePatchingEnabled()) {
-      // TODO: this is temporary code that should be replaced with
-      // workflow selection flow. For now it picks the first Workspace
-      // project that is not Snippets.
-      const projects = this.#workspace.projectsForType(Workspace.Workspace.projectTypes.FileSystem);
+    const project = this.#workspace.project(this.#projectIdSetting.get());
+    if (project) {
+      this.#project = project;
+    } else {
       this.#project = undefined;
-      for (const project of projects) {
-        // This is for TypeScript to narrow the types. projectsForType()
-        // probably only returns instances of
-        // Persistence.FileSystemWorkspaceBinding.FileSystem.
-        if (!(project instanceof Persistence.FileSystemWorkspaceBinding.FileSystem)) {
-          continue;
-        }
-        if (project.fileSystem().type() !== Persistence.PlatformFileSystem.PlatformFileSystemType.WORKSPACE_PROJECT) {
-          continue;
-        }
-        this.#project = project;
-        this.requestUpdate();
-        break;
-      }
+      this.#projectIdSetting.set('');
     }
+    this.requestUpdate();
   }
 
-  #onProjectAddedOrRemoved(): void {
-    this.#selectDefaultProject();
+  #onProjectRemoved(): void {
+    if (this.#project && !this.#workspace.project(this.#project.id())) {
+      this.#projectIdSetting.set('');
+      this.#project = undefined;
+      this.requestUpdate();
+    }
+  }
+
+  #showSelectWorkspaceDialog(options: {applyPatch: boolean} = {applyPatch: false}): void {
+    const onProjectSelected = (project: Workspace.Workspace.Project): void => {
+      this.#project = project;
+      this.#projectIdSetting.set(project.id());
+      if (options.applyPatch) {
+        void this.#applyPatchAndUpdateUI();
+      } else {
+        this.requestUpdate();
+      }
+    };
+
+    SelectWorkspaceDialog.show(onProjectSelected, this.#project);
   }
 
   async #onApplyToWorkspace(): Promise<void> {
     if (!isAiAssistancePatchingEnabled()) {
       return;
     }
-    const changeSummary = this.changeSummary;
-    if (!changeSummary) {
-      throw new Error('Change summary does not exist');
-    }
 
     // Show the FRE dialog if needed and only continue when
     // the user accepted the disclaimer.
@@ -486,6 +473,19 @@ export class PatchWidget extends UI.Widget.Widget {
       return;
     }
 
+    if (this.#project) {
+      await this.#applyPatchAndUpdateUI();
+    } else {
+      this.#showSelectWorkspaceDialog({applyPatch: true});
+    }
+  }
+
+  async #applyPatchAndUpdateUI(): Promise<void> {
+    const changeSummary = this.changeSummary;
+    if (!changeSummary) {
+      throw new Error('Change summary does not exist');
+    }
+
     this.#patchSuggestionLoading = true;
     this.requestUpdate();
     const {response, processedFiles} = await this.#applyPatch(changeSummary);

front_end/panels/ai_assistance/SelectWorkspaceDialog.test.ts

@@ -16,7 +16,7 @@ describeWithEnvironment('SelectWorkspaceDialog', () => {
   function createComponent(): {
     view: ViewFunctionStub<typeof AiAssistance.SelectWorkspaceDialog>,
     component: AiAssistance.SelectWorkspaceDialog,
-    handleProjectSelected: sinon.SinonSpy<[Workspace.Workspace.Project], void>,
+    onProjectSelected: sinon.SinonSpy<[Workspace.Workspace.Project], void>,
     hideDialogSpy: sinon.SinonSpy<[], void>,
     project: Persistence.FileSystemWorkspaceBinding.FileSystem,
   } {
@@ -26,37 +26,37 @@ describeWithEnvironment('SelectWorkspaceDialog', () => {
     const hideDialogSpy = sinon.spy(dialog, 'hide');
 
     const view = createViewFunctionStub(AiAssistance.SelectWorkspaceDialog);
-    const handleProjectSelected = sinon.spy() as sinon.SinonSpy<[Workspace.Workspace.Project], void>;
-    const component = new AiAssistance.SelectWorkspaceDialog({dialog, handleProjectSelected}, view);
+    const onProjectSelected = sinon.spy() as sinon.SinonSpy<[Workspace.Workspace.Project], void>;
+    const component = new AiAssistance.SelectWorkspaceDialog({dialog, onProjectSelected}, view);
     component.markAsRoot();
     component.show(document.body);
     assert.strictEqual(view.callCount, 1);
     assert.strictEqual(view.input.selectedIndex, 0);
 
-    return {view, component, handleProjectSelected, hideDialogSpy, project};
+    return {view, component, onProjectSelected, hideDialogSpy, project};
   }
 
   it('selects a project', async () => {
-    const {view, handleProjectSelected, hideDialogSpy, project} = createComponent();
+    const {view, onProjectSelected, hideDialogSpy, project} = createComponent();
     view.input.onProjectSelected(1);
     const input = await view.nextInput;
     assert.strictEqual(view.callCount, 2);
     assert.strictEqual(input.selectedIndex, 1);
 
     view.input.onSelectButtonClick();
-    assert.isTrue(handleProjectSelected.calledOnceWith(project));
+    assert.isTrue(onProjectSelected.calledOnceWith(project));
     assert.isTrue(hideDialogSpy.calledOnce);
   });
 
   it('can be canceled', async () => {
-    const {view, handleProjectSelected, hideDialogSpy} = createComponent();
+    const {view, onProjectSelected, hideDialogSpy} = createComponent();
     view.input.onProjectSelected(1);
     const input = await view.nextInput;
     assert.strictEqual(view.callCount, 2);
     assert.strictEqual(input.selectedIndex, 1);
 
     view.input.onCancelButtonClick();
-    assert.isTrue(handleProjectSelected.notCalled);
+    assert.isTrue(onProjectSelected.notCalled);
     assert.isTrue(hideDialogSpy.calledOnce);
   });