From ec8e6f79b3d3afe337e671e0e284723bd9eb0085 Mon Sep 17 00:00:00 2001
From: Chenlei Hu <huchenlei@proton.me>
Date: Thu, 10 Oct 2024 15:56:00 -0400
Subject: [PATCH] Fix create group node command error states (#1209)

* Fix edge cases

* Add playwright test

* nit
---
 browser_tests/ComfyPage.ts       |  4 ++++
 browser_tests/groupNode.spec.ts  | 16 +++++++++++++++
 src/extensions/core/groupNode.ts | 35 ++++++++++++++++++--------------
 3 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/browser_tests/ComfyPage.ts b/browser_tests/ComfyPage.ts
index 8bc2cca07..7c4d13644 100644
--- a/browser_tests/ComfyPage.ts
+++ b/browser_tests/ComfyPage.ts
@@ -575,6 +575,10 @@ export class ComfyPage {
     await this.nextFrame()
   }
 
+  async getVisibleToastCount() {
+    return await this.page.locator('.p-toast:visible').count()
+  }
+
   async clickTextEncodeNode1() {
     await this.canvas.click({
       position: {
diff --git a/browser_tests/groupNode.spec.ts b/browser_tests/groupNode.spec.ts
index e24255ce0..c72245473 100644
--- a/browser_tests/groupNode.spec.ts
+++ b/browser_tests/groupNode.spec.ts
@@ -252,4 +252,20 @@ test.describe('Group Node', () => {
       })
     })
   })
+
+  test.describe('Keybindings', () => {
+    test('Convert to group node, no selection', async ({ comfyPage }) => {
+      expect(await comfyPage.getVisibleToastCount()).toBe(0)
+      await comfyPage.page.keyboard.press('Alt+g')
+      await comfyPage.page.waitForTimeout(300)
+      expect(await comfyPage.getVisibleToastCount()).toBe(1)
+    })
+    test('Convert to group node, selected 1 node', async ({ comfyPage }) => {
+      expect(await comfyPage.getVisibleToastCount()).toBe(0)
+      await comfyPage.clickTextEncodeNode1()
+      await comfyPage.page.keyboard.press('Alt+g')
+      await comfyPage.page.waitForTimeout(300)
+      expect(await comfyPage.getVisibleToastCount()).toBe(1)
+    })
+  })
 })
diff --git a/src/extensions/core/groupNode.ts b/src/extensions/core/groupNode.ts
index b8518b2fa..492a21653 100644
--- a/src/extensions/core/groupNode.ts
+++ b/src/extensions/core/groupNode.ts
@@ -1450,22 +1450,27 @@ const replaceLegacySeparators = (nodes: ComfyNode[]): void => {
   }
 }
 
-function convertSelectedNodesToGroupNode() {
-  if (
-    !app.canvas.selected_nodes ||
-    Object.keys(app.canvas.selected_nodes).length === 0
-  ) {
-    useToastStore().add({
-      severity: 'error',
-      summary: 'No nodes selected',
-      detail: 'Please select nodes to convert to group node',
-      life: 3000
-    })
-    return
+/**
+ * Convert selected nodes to a group node
+ * @throws {Error} if no nodes are selected
+ * @throws {Error} if a group node is already selected
+ * @throws {Error} if a group node is selected
+ *
+ * The context menu item should not be available if any of the above conditions are met.
+ * The error is automatically handled by the commandStore when the command is executed.
+ */
+async function convertSelectedNodesToGroupNode() {
+  const nodes = Object.values(app.canvas.selected_nodes ?? {})
+  if (nodes.length === 0) {
+    throw new Error('No nodes selected')
   }
-
-  const nodes = Object.values(app.canvas.selected_nodes)
-  return GroupNodeHandler.fromNodes(nodes)
+  if (nodes.length === 1) {
+    throw new Error('Please select multiple nodes to convert to group node')
+  }
+  if (nodes.some((n) => GroupNodeHandler.isGroupNode(n))) {
+    throw new Error('Selected nodes contain a group node')
+  }
+  return await GroupNodeHandler.fromNodes(nodes)
 }
 
 function ungroupSelectedGroupNodes() {