Fix duplicated chats (#87)

* Add the file path to the chat section widget, and use the path as
section name

* Avoid opening duplicate chat in side panel

* Add tests

Author: brichet

packages/jupyterlab-collaborative-chat/src/widget.tsx

@@ -151,7 +151,7 @@ export class ChatPanel extends SidePanel {
    * @param model - the model of the chat widget
    * @param name - the name of the chat.
    */
-  addChat(model: IChatModel, name: string): void {
+  addChat(model: IChatModel, name: string, path: string): void {
     // Collapse all chats
     const content = this.content as AccordionPanel;
     for (let i = 0; i < this.widgets.length; i++) {
@@ -168,22 +168,45 @@ export class ChatPanel extends SidePanel {
       themeManager: this._themeManager,
       autocompletionRegistry: this._autocompletionRegistry
     });
-    this.addWidget(new ChatSection({ name, widget, commands: this._commands }));
+    this.addWidget(
+      new ChatSection({ name, widget, commands: this._commands, path })
+    );
   }
 
+  /**
+   * Update the list of available chats in the root directory of the drive.
+   */
   updateChatNames = async (): Promise<void> => {
     const extension = chatFileType.extensions[0];
     this._drive
       .get('.')
-      .then(model => {
-        const chatsName = (model.content as any[])
+      .then(contentModel => {
+        const chatsNames: { [name: string]: string } = {};
+        (contentModel.content as any[])
           .filter(f => f.type === 'file' && f.name.endsWith(extension))
-          .map(f => PathExt.basename(f.name, extension));
-        this._chatNamesChanged.emit(chatsName);
+          .forEach(
+            f => (chatsNames[PathExt.basename(f.name, extension)] = f.name)
+          );
+
+        this._chatNamesChanged.emit(chatsNames);
       })
       .catch(e => console.error('Error getting the chat files from drive', e));
   };
 
+  /**
+   * Open a chat if it exists in the side panel.
+   *
+   * @param path - the path of the chat.
+   * @returns a boolean, whether the chat existed in the side panel or not.
+   */
+  openIfExists(path: string): boolean {
+    const index = this._getChatIndex(path);
+    if (index > -1) {
+      this._expandChat(index);
+    }
+    return index > -1;
+  }
+
   /**
    * A message handler invoked on an `'after-show'` message.
    */
@@ -192,28 +215,41 @@ export class ChatPanel extends SidePanel {
     this._openChat.renderPromise?.then(() => this.updateChatNames());
   }
 
+  /**
+   * Return the index of the chat in the list (-1 if not opened).
+   *
+   * @param name - the chat name.
+   */
+  private _getChatIndex(path: string) {
+    return this.widgets.findIndex(w => (w as ChatSection).path === path);
+  }
+
+  /**
+   * Expand the chat from its index.
+   */
+  private _expandChat(index: number): void {
+    if (!this.widgets[index].isVisible) {
+      (this.content as AccordionPanel).expand(index);
+    }
+  }
+
   /**
    * Handle `change` events for the HTMLSelect component.
    */
   private _chatSelected = (
     event: React.ChangeEvent<HTMLSelectElement>
   ): void => {
-    const value = event.target.value;
-    if (value === '-') {
+    const select = event.target;
+    const path = select.value;
+    const name = select.options[select.selectedIndex].textContent;
+    if (name === '-') {
       return;
     }
 
-    const index = this.widgets.findIndex(
-      w => (w as ChatSection).name === value
-    );
-    if (index === -1) {
-      this._commands.execute(CommandIDs.openChat, {
-        filepath: `${value}${chatFileType.extensions[0]}`,
-        inSidePanel: true
-      });
-    } else if (!this.widgets[index].isVisible) {
-      (this.content as AccordionPanel).expand(index);
-    }
+    this._commands.execute(CommandIDs.openChat, {
+      filepath: path,
+      inSidePanel: true
+    });
     event.target.selectedIndex = 0;
   };
 
@@ -232,7 +268,9 @@ export class ChatPanel extends SidePanel {
     }
   }
 
-  private _chatNamesChanged = new Signal<this, string[]>(this);
+  private _chatNamesChanged = new Signal<this, { [name: string]: string }>(
+    this
+  );
   private _commands: CommandRegistry;
   private _config: IConfig = {};
   private _drive: ICollaborativeDrive;
@@ -269,8 +307,9 @@ class ChatSection extends PanelWithToolbar {
     super(options);
     this.addClass(SECTION_CLASS);
     this._name = options.name;
+    this._path = options.path;
     this.title.label = this._name;
-    this.title.caption = this._name;
+    this.title.caption = this._path;
     this.toolbar.addClass(TOOLBAR_CLASS);
 
     this._markAsRead = new ToolbarButton({
@@ -316,6 +355,13 @@ class ChatSection extends PanelWithToolbar {
     options.widget.node.style.height = '100%';
   }
 
+  /**
+   * The path of the chat.
+   */
+  get path(): string {
+    return this._path;
+  }
+
   /**
    * The name of the chat.
    */
@@ -353,6 +399,7 @@ class ChatSection extends PanelWithToolbar {
 
   private _name: string;
   private _markAsRead: ToolbarButton;
+  private _path: string;
 }
 
 /**
@@ -366,11 +413,12 @@ export namespace ChatSection {
     commands: CommandRegistry;
     name: string;
     widget: ChatWidget;
+    path: string;
   }
 }
 
 type ChatSelectProps = {
-  chatNamesChanged: ISignal<ChatPanel, string[]>;
+  chatNamesChanged: ISignal<ChatPanel, { [name: string]: string }>;
   handleChange: (event: React.ChangeEvent<HTMLSelectElement>) => void;
 };
 
@@ -381,7 +429,9 @@ function ChatSelect({
   chatNamesChanged,
   handleChange
 }: ChatSelectProps): JSX.Element {
-  const [chatNames, setChatNames] = useState<string[]>([]);
+  // An object associating a chat name to its path. Both are purely indicative, the name
+  // is the section title and the path is used as caption.
+  const [chatNames, setChatNames] = useState<{ [name: string]: string }>({});
 
   // Update the chat list.
   chatNamesChanged.connect((_, chatNames) => {
@@ -391,8 +441,8 @@ function ChatSelect({
   return (
     <HTMLSelect onChange={handleChange}>
       <option value="-">Open a chat</option>
-      {chatNames.map(name => (
-        <option value={name}>{name}</option>
+      {Object.keys(chatNames).map(name => (
+        <option value={chatNames[name]}>{name}</option>
       ))}
     </HTMLSelect>
   );

python/jupyterlab-collaborative-chat/src/index.ts

@@ -465,6 +465,11 @@ const chatCommands: JupyterFrontEndPlugin<void> = {
             if (inSidePanel && chatPanel) {
               // The chat is opened in the chat panel.
               app.shell.activateById(chatPanel.id);
+
+              if (chatPanel.openIfExists(filepath)) {
+                return;
+              }
+
               const model = await drive.get(filepath);
 
               /**
@@ -494,7 +499,11 @@ const chatCommands: JupyterFrontEndPlugin<void> = {
                */
               chatPanel.addChat(
                 chat,
-                PathExt.basename(model.name, chatFileType.extensions[0])
+                PathExt.join(
+                  PathExt.dirname(model.path),
+                  PathExt.basename(model.name, chatFileType.extensions[0])
+                ),
+                model.path
               );
             } else {
               // The chat is opened in the main area

python/jupyterlab-collaborative-chat/ui-tests/tests/side-panel.spec.ts

@@ -58,10 +58,11 @@ test.describe('#sidepanel', () => {
     const name = FILENAME.replace('.chat', '');
     let panel: Locator;
     let dialog: Locator;
+    let addButton: Locator;
 
     test.beforeEach(async ({ page }) => {
       panel = await openSidePanel(page);
-      const addButton = panel.locator(
+      addButton = panel.locator(
         '.jp-SidePanel-toolbar .jp-Toolbar-item.jp-collab-chat-add'
       );
       await addButton.click();
@@ -89,6 +90,7 @@ test.describe('#sidepanel', () => {
         '.jp-SidePanel-content .jp-AccordionPanel-title'
       );
       await expect(chatTitle).toHaveCount(1);
+      await expect(chatTitle).toHaveClass(/lm-mod-expanded/);
       await expect(
         chatTitle.locator('.lm-AccordionPanel-titleLabel')
       ).toHaveText(name);
@@ -117,6 +119,39 @@ test.describe('#sidepanel', () => {
       const content = panel.locator('.jp-SidePanel-content');
       await expect(content).toBeEmpty();
     });
+
+    test('should reveal the existing chat when creating again', async ({
+      page
+    }) => {
+      await dialog.locator('input[type="text"]').pressSequentially(name);
+      await dialog.getByRole('button').getByText('Ok').click();
+      await page.waitForCondition(
+        async () => await page.filebrowser.contents.fileExists(FILENAME)
+      );
+
+      const chatTitle = panel.locator(
+        '.jp-SidePanel-content .jp-AccordionPanel-title'
+      );
+      await expect(chatTitle).toHaveCount(1);
+      await expect(
+        chatTitle.locator('.lm-AccordionPanel-titleLabel')
+      ).toHaveText(name);
+
+      // Collapse the chat.
+      await chatTitle.click();
+      await expect(chatTitle).not.toHaveClass(/lm-mod-expanded/);
+
+      await addButton.click();
+
+      // try to recreate the same chat.
+      dialog = page.locator('.jp-Dialog');
+      await dialog.waitFor();
+      await dialog.locator('input[type="text"]').pressSequentially(name);
+      await dialog.getByRole('button').getByText('Ok').click();
+
+      // the chat should be expanded.
+      await expect(chatTitle).toHaveClass(/lm-mod-expanded/);
+    });
   });
 
   test.describe('#openingClosing', () => {
@@ -207,6 +242,56 @@ test.describe('#sidepanel', () => {
       ).toHaveText(FILENAME.split('.')[0]);
     });
 
+    test('should reveal the existing chat in side panel when moving again', async ({
+      page
+    }) => {
+      let chatPanel = await openChat(page, FILENAME);
+      const button = chatPanel.getByTitle('Move the chat to the side panel');
+      await button.click();
+      await expect(chatPanel).not.toBeAttached();
+
+      const sidePanel = page.locator('.jp-SidePanel.jp-collab-chat-sidepanel');
+      await expect(sidePanel).toBeVisible();
+      const chatTitle = sidePanel.locator(
+        '.jp-SidePanel-content .jp-AccordionPanel-title'
+      );
+      await expect(chatTitle).toHaveCount(1);
+      await expect(
+        chatTitle.locator('.lm-AccordionPanel-titleLabel')
+      ).toHaveText(FILENAME.split('.')[0]);
+
+      await chatTitle.click();
+      await expect(chatTitle).not.toHaveClass(/lm-mod-expanded/);
+
+      chatPanel = await openChat(page, FILENAME);
+      await button.click();
+      await expect(chatPanel).not.toBeAttached();
+      await expect(chatTitle).toHaveClass(/lm-mod-expanded/);
+    });
+
+    test('chat section should contain the file path', async ({ page }) => {
+      // Create a nested chat file and open it.
+      const dirName = 'my-dir';
+      const nestedPath = [dirName, FILENAME].join('/');
+      await page.filebrowser.contents.createDirectory(dirName);
+      await page.filebrowser.contents.uploadContent('{}', 'text', nestedPath);
+
+      const chatPanel = await openChat(page, nestedPath);
+      const button = chatPanel.getByTitle('Move the chat to the side panel');
+      await button.click();
+      await expect(chatPanel).not.toBeAttached();
+
+      // Move the chat to the side panel.
+      const sidePanel = page.locator('.jp-SidePanel.jp-collab-chat-sidepanel');
+      await expect(sidePanel).toBeVisible();
+      const chatTitle = sidePanel.locator(
+        '.jp-SidePanel-content .jp-AccordionPanel-title'
+      );
+      await expect(
+        chatTitle.locator('.lm-AccordionPanel-titleLabel')
+      ).toHaveText(nestedPath.split('.')[0]);
+    });
+
     test('side panel should contain a button to move the chat', async ({
       page
     }) => {

python/jupyterlab-collaborative-chat/ui-tests/tests/test-utils.ts

@@ -33,10 +33,12 @@ export const openChat = async (
       filepath
     });
   }, filename);
+  const splitPath = filename.split('/');
+  const tabName = splitPath[splitPath.length - 1];
   await page.waitForCondition(
-    async () => await page.activity.isTabActive(filename)
+    async () => await page.activity.isTabActive(tabName)
   );
-  return (await page.activity.getPanelLocator(filename)) as Locator;
+  return (await page.activity.getPanelLocator(tabName)) as Locator;
 };
 
 export const openChatToSide = async (