From cd92dfec47428e38780808dd2f4b9a0882f3bfb8 Mon Sep 17 00:00:00 2001
From: jamesgeorge007 <jamesgeorge998001@gmail.com>
Date: Tue, 23 Apr 2024 22:21:13 +0530
Subject: [PATCH] refactor: introduce writable handles to signify updates to
 handle references

A special list of writable handles is compiled in a list while issuing handles (request/collection creation, etc). Instead of manually computing the tab and toggling the dirty state, the writable handle is updated (changing the type to invalid on request deletion) and the tab with the request open can infer it via the update reflected in the request handle under the tab save context (reactive update trigger).
---
 .../components/new-collections/rest/index.vue |  15 --
 .../hoppscotch-common/src/pages/index.vue     |  64 ++++---
 .../src/services/inspection/index.ts          |   5 +
 .../src/services/new-workspace/handle.ts      |   9 +-
 .../providers/personal.workspace.ts           | 160 +++++++++++-------
 .../src/services/tab/rest.ts                  |   6 +-
 .../hoppscotch-common/src/services/tab/tab.ts |  81 +++++++--
 7 files changed, 230 insertions(+), 110 deletions(-)

diff --git a/packages/hoppscotch-common/src/components/new-collections/rest/index.vue b/packages/hoppscotch-common/src/components/new-collections/rest/index.vue
index d5a3de324..700cb8b1d 100644
--- a/packages/hoppscotch-common/src/components/new-collections/rest/index.vue
+++ b/packages/hoppscotch-common/src/components/new-collections/rest/index.vue
@@ -1118,15 +1118,6 @@ const onRemoveRequest = async () => {
     return
   }
 
-  const { providerID, requestID, workspaceID } = requestHandle.value.data
-
-  const possibleTab = tabs.getTabRefWithSaveContext({
-    originLocation: "workspace-user-collection",
-    workspaceID,
-    providerID,
-    requestID,
-  })
-
   if (
     isSelected({
       requestIndex: parseInt(requestIndexPath.split("/").pop() ?? ""),
@@ -1143,12 +1134,6 @@ const onRemoveRequest = async () => {
     return
   }
 
-  // If there is a tab attached to this request, dissociate its state and mark it dirty
-  if (possibleTab) {
-    possibleTab.value.document.saveContext = null
-    possibleTab.value.document.isDirty = true
-  }
-
   toast.success(t("state.deleted"))
   displayConfirmModal(false)
 }
diff --git a/packages/hoppscotch-common/src/pages/index.vue b/packages/hoppscotch-common/src/pages/index.vue
index e8afb9d21..01dcfa86c 100644
--- a/packages/hoppscotch-common/src/pages/index.vue
+++ b/packages/hoppscotch-common/src/pages/index.vue
@@ -13,7 +13,7 @@
           <HoppSmartWindow
             v-for="tab in activeTabs"
             :id="tab.id"
-            :key="tab.id"
+            :key="`${tab.id}-${tab.document.isDirty}`"
             :label="tab.document.request.name"
             :is-removable="activeTabs.length > 1"
             :close-visibility="'hover'"
@@ -118,24 +118,26 @@
 </template>
 
 <script lang="ts" setup>
-import { ref, onMounted } from "vue"
-import { safelyExtractRESTRequest } from "@hoppscotch/data"
-import { translateExtURLParams } from "~/helpers/RESTExtURLParams"
-import { useRoute } from "vue-router"
 import { useI18n } from "@composables/i18n"
-import { getDefaultRESTRequest } from "~/helpers/rest/default"
-import { defineActionHandler, invokeAction } from "~/helpers/actions"
-import { platform } from "~/platform"
-import { useReadonlyStream } from "~/composables/stream"
+import { safelyExtractRESTRequest } from "@hoppscotch/data"
 import { useService } from "dioc/vue"
-import { InspectionService } from "~/services/inspection"
-import { HeaderInspectorService } from "~/services/inspection/inspectors/header.inspector"
-import { EnvironmentInspectorService } from "~/services/inspection/inspectors/environment.inspector"
-import { ResponseInspectorService } from "~/services/inspection/inspectors/response.inspector"
 import { cloneDeep } from "lodash-es"
-import { RESTTabService } from "~/services/tab/rest"
-import { HoppTab } from "~/services/tab"
+import { onMounted, ref } from "vue"
+import { useRoute } from "vue-router"
+import { useReadonlyStream } from "~/composables/stream"
+import { translateExtURLParams } from "~/helpers/RESTExtURLParams"
+import { defineActionHandler, invokeAction } from "~/helpers/actions"
+import { getDefaultRESTRequest } from "~/helpers/rest/default"
 import { HoppRESTDocument } from "~/helpers/rest/document"
+import { platform } from "~/platform"
+import { InspectionService } from "~/services/inspection"
+import { EnvironmentInspectorService } from "~/services/inspection/inspectors/environment.inspector"
+import { HeaderInspectorService } from "~/services/inspection/inspectors/header.inspector"
+import { ResponseInspectorService } from "~/services/inspection/inspectors/response.inspector"
+import { HandleRef } from "~/services/new-workspace/handle"
+import { WorkspaceRequest } from "~/services/new-workspace/workspace"
+import { HoppTab } from "~/services/tab"
+import { RESTTabService } from "~/services/tab/rest"
 
 const savingRequest = ref(false)
 const confirmingCloseForTabID = ref<string | null>(null)
@@ -291,15 +293,29 @@ const onCloseConfirmSaveTab = () => {
  * Called when the user confirms they want to save the tab
  */
 const onResolveConfirmSaveTab = () => {
-  if (tabs.currentActiveTab.value.document.saveContext) {
-    invokeAction("request.save")
+  const { saveContext } = tabs.currentActiveTab.value.document
 
-    if (confirmingCloseForTabID.value) {
-      tabs.closeTab(confirmingCloseForTabID.value)
-      confirmingCloseForTabID.value = null
-    }
-  } else {
-    savingRequest.value = true
+  // There're two cases where the save request under a collection modal should open
+  // 1. Attempting to save a request that is not under a collection (When the save context is not available)
+  // 2. Deleting a request from the collection tree and attempting to save it while closing the respective tab (When the request handle is invalid)
+  if (
+    !saveContext ||
+    (saveContext.originLocation === "workspace-user-collection" &&
+      // `requestHandle` gets unwrapped here
+      (
+        saveContext.requestHandle as
+          | HandleRef<WorkspaceRequest>["value"]
+          | undefined
+      )?.type === "invalid")
+  ) {
+    return (savingRequest.value = true)
+  }
+
+  invokeAction("request.save")
+
+  if (confirmingCloseForTabID.value) {
+    tabs.closeTab(confirmingCloseForTabID.value)
+    confirmingCloseForTabID.value = null
   }
 }
 
@@ -368,3 +384,5 @@ for (const inspectorDef of platform.additionalInspectors ?? []) {
   useService(inspectorDef.service)
 }
 </script>
+import { HandleRef } from "~/services/new-workspace/handle" import {
+WorkspaceRequest } from "~/services/new-workspace/workspace"
diff --git a/packages/hoppscotch-common/src/services/inspection/index.ts b/packages/hoppscotch-common/src/services/inspection/index.ts
index 1080e67dd..b58c6a8fe 100644
--- a/packages/hoppscotch-common/src/services/inspection/index.ts
+++ b/packages/hoppscotch-common/src/services/inspection/index.ts
@@ -121,6 +121,11 @@ export class InspectionService extends Service {
   }
 
   private initializeListeners() {
+    console.log(
+      `Current active tab from inspection service is `,
+      this.restTab.currentActiveTab.value
+    )
+
     watch(
       () => [this.inspectors.entries(), this.restTab.currentActiveTab.value.id],
       () => {
diff --git a/packages/hoppscotch-common/src/services/new-workspace/handle.ts b/packages/hoppscotch-common/src/services/new-workspace/handle.ts
index e7529ab3a..6404eebf4 100644
--- a/packages/hoppscotch-common/src/services/new-workspace/handle.ts
+++ b/packages/hoppscotch-common/src/services/new-workspace/handle.ts
@@ -1,5 +1,12 @@
-import { Ref } from "vue"
+import { Ref, WritableComputedRef } from "vue"
 
 export type HandleRef<T, InvalidateReason = unknown> = Ref<
   { type: "ok"; data: T } | { type: "invalid"; reason: InvalidateReason }
 >
+
+export type WritableHandleRef<
+  T,
+  InvalidateReason = unknown,
+> = WritableComputedRef<
+  { type: "ok"; data: T } | { type: "invalid"; reason: InvalidateReason }
+>
diff --git a/packages/hoppscotch-common/src/services/new-workspace/providers/personal.workspace.ts b/packages/hoppscotch-common/src/services/new-workspace/providers/personal.workspace.ts
index 27ce66dba..875a5073b 100644
--- a/packages/hoppscotch-common/src/services/new-workspace/providers/personal.workspace.ts
+++ b/packages/hoppscotch-common/src/services/new-workspace/providers/personal.workspace.ts
@@ -39,7 +39,7 @@ import {
 } from "~/newstore/collections"
 import { platform } from "~/platform"
 
-import { HandleRef } from "~/services/new-workspace/handle"
+import { HandleRef, WritableHandleRef } from "~/services/new-workspace/handle"
 import { WorkspaceProvider } from "~/services/new-workspace/provider"
 import {
   RESTCollectionChildrenView,
@@ -87,6 +87,10 @@ export class PersonalWorkspaceProviderService
 
   private restCollectionState: Ref<{ state: HoppCollection[] }>
 
+  private issuedHandles: WritableHandleRef<
+    WorkspaceCollection | WorkspaceRequest
+  >[] = []
+
   public constructor() {
     super()
 
@@ -298,6 +302,19 @@ export class PersonalWorkspaceProviderService
       )
     }
 
+    for (const handle of this.issuedHandles) {
+      if (handle.value.type === "invalid") continue
+
+      if ("requestID" in handle.value.data) {
+        if (handle.value.data.requestID.startsWith(collectionID)) {
+          handle.value = {
+            type: "invalid",
+            reason: "REQUEST_INVALIDATED",
+          }
+        }
+      }
+    }
+
     return Promise.resolve(E.right(undefined))
   }
 
@@ -329,35 +346,44 @@ export class PersonalWorkspaceProviderService
       platform: "rest",
     })
 
-    return Promise.resolve(
-      E.right(
-        computed(() => {
-          if (
-            !isValidCollectionHandle(
-              parentCollectionHandle,
-              this.providerID,
-              "personal"
-            )
-          ) {
-            return {
-              type: "invalid" as const,
-              reason: "COLLECTION_INVALIDATED" as const,
-            }
-          }
+    const handle: HandleRef<WorkspaceRequest> = computed(() => {
+      if (
+        !isValidCollectionHandle(
+          parentCollectionHandle,
+          this.providerID,
+          "personal"
+        )
+      ) {
+        return {
+          type: "invalid" as const,
+          reason: "COLLECTION_INVALIDATED" as const,
+        }
+      }
 
-          return {
-            type: "ok",
-            data: {
-              providerID,
-              workspaceID,
-              collectionID,
-              requestID,
-              request: newRequest,
-            },
-          }
-        })
-      )
-    )
+      return {
+        type: "ok",
+        data: {
+          providerID,
+          workspaceID,
+          collectionID,
+          requestID,
+          request: newRequest,
+        },
+      }
+    })
+
+    const writableHandle = computed({
+      get() {
+        return handle.value
+      },
+      set(newValue) {
+        handle.value = newValue
+      },
+    })
+
+    this.issuedHandles.push(writableHandle)
+
+    return Promise.resolve(E.right(handle))
   }
 
   public removeRESTRequest(
@@ -377,6 +403,19 @@ export class PersonalWorkspaceProviderService
 
     removeRESTRequest(collectionID, requestIndex, requestToRemove?.id)
 
+    for (const handle of this.issuedHandles) {
+      if (handle.value.type === "invalid") continue
+
+      if ("requestID" in handle.value.data) {
+        if (handle.value.data.requestID === requestID) {
+          handle.value = {
+            type: "invalid",
+            reason: "REQUEST_INVALIDATED",
+          }
+        }
+      }
+    }
+
     return Promise.resolve(E.right(undefined))
   }
 
@@ -647,35 +686,42 @@ export class PersonalWorkspaceProviderService
       return Promise.resolve(E.left("REQUEST_NOT_FOUND" as const))
     }
 
-    return Promise.resolve(
-      E.right(
-        computed(() => {
-          if (
-            !isValidWorkspaceHandle(
-              workspaceHandle,
-              this.providerID,
-              "personal"
-            )
-          ) {
-            return {
-              type: "invalid" as const,
-              reason: "WORKSPACE_INVALIDATED" as const,
-            }
-          }
+    const handleRefData = ref({
+      type: "ok" as const,
+      data: {
+        providerID,
+        workspaceID,
+        collectionID,
+        requestID,
+        request,
+      },
+    })
 
-          return {
-            type: "ok",
-            data: {
-              providerID,
-              workspaceID,
-              collectionID,
-              requestID,
-              request,
-            },
-          }
-        })
-      )
-    )
+    const handle: HandleRef<WorkspaceRequest> = computed(() => {
+      if (
+        !isValidWorkspaceHandle(workspaceHandle, this.providerID, "personal")
+      ) {
+        return {
+          type: "invalid" as const,
+          reason: "WORKSPACE_INVALIDATED" as const,
+        }
+      }
+
+      return handleRefData.value
+    })
+
+    const writableHandle = computed({
+      get() {
+        return handleRefData.value
+      },
+      set(newValue) {
+        handleRefData.value = newValue
+      },
+    })
+
+    this.issuedHandles.push(writableHandle)
+
+    return Promise.resolve(E.right(handle))
   }
 
   public getRESTCollectionChildrenView(
diff --git a/packages/hoppscotch-common/src/services/tab/rest.ts b/packages/hoppscotch-common/src/services/tab/rest.ts
index 198cdd754..6aad6c893 100644
--- a/packages/hoppscotch-common/src/services/tab/rest.ts
+++ b/packages/hoppscotch-common/src/services/tab/rest.ts
@@ -35,17 +35,19 @@ export class RESTTabService extends TabService<HoppRESTDocument> {
     lastActiveTabID: this.currentTabID.value,
     orderedDocs: this.tabOrdering.value.map((tabID) => {
       const tab = this.tabMap.get(tabID)! // tab ordering is guaranteed to have value for this key
+      const resolvedTabData = this.getResolvedTabData(tab)
+
       return {
         tabID: tab.id,
         doc: {
-          ...tab.document,
+          ...this.getPersistedDocument(resolvedTabData.document),
           response: null,
         },
       }
     }),
   }))
 
-  public getTabRefWithSaveContext(ctx: HoppRESTSaveContext) {
+  public getTabRefWithSaveContext(ctx: Partial<HoppRESTSaveContext>) {
     for (const tab of this.tabMap.values()) {
       // For `team-collection` request id can be considered unique
       if (ctx?.originLocation === "team-collection") {
diff --git a/packages/hoppscotch-common/src/services/tab/tab.ts b/packages/hoppscotch-common/src/services/tab/tab.ts
index 5683b989c..0caf6c52a 100644
--- a/packages/hoppscotch-common/src/services/tab/tab.ts
+++ b/packages/hoppscotch-common/src/services/tab/tab.ts
@@ -18,6 +18,7 @@ import {
   TabService as TabServiceInterface,
 } from "."
 
+import { HoppGQLDocument } from "~/helpers/graphql/document"
 import { NewWorkspaceService } from "../new-workspace"
 import { HandleRef } from "../new-workspace/handle"
 import { WorkspaceRequest } from "../new-workspace/workspace"
@@ -44,9 +45,12 @@ export abstract class TabService<Doc>
     },
   })
 
-  public currentActiveTab = computed(
-    () => this.tabMap.get(this.currentTabID.value)!
-  ) // Guaranteed to not be undefined
+  public currentActiveTab = computed(() => {
+    const tab = this.tabMap.get(this.currentTabID.value)!
+    return this.getResolvedTabData(
+      tab as HoppTab<HoppRESTDocument | HoppGQLDocument>
+    )
+  }) // Guaranteed to not be undefined
 
   protected watchCurrentTabID() {
     watch(
@@ -83,7 +87,15 @@ export abstract class TabService<Doc>
   }
 
   public getActiveTab(): HoppTab<Doc> | null {
-    return this.tabMap.get(this.currentTabID.value) ?? null
+    const tab = this.tabMap.get(this.currentTabID.value)
+
+    if (!tab) {
+      return null
+    }
+
+    return this.getResolvedTabData(
+      tab as HoppTab<HoppRESTDocument | HoppGQLDocument>
+    )
   }
 
   public setActiveTab(tabID: string): void {
@@ -159,18 +171,28 @@ export abstract class TabService<Doc>
   }
   public getActiveTabs(): Readonly<ComputedRef<HoppTab<Doc>[]>> {
     return shallowReadonly(
-      computed(() => this.tabOrdering.value.map((x) => this.tabMap.get(x)!))
+      computed(() =>
+        this.tabOrdering.value.map((x) => {
+          const tab = this.tabMap.get(x) as HoppTab<
+            HoppRESTDocument | HoppGQLDocument
+          >
+
+          return this.getResolvedTabData(tab)
+        })
+      )
     )
   }
 
   public getTabRef(tabID: string) {
     return computed({
       get: () => {
-        const result = this.tabMap.get(tabID)
+        const result = this.tabMap.get(tabID) as HoppTab<
+          HoppRESTDocument | HoppGQLDocument
+        >
 
         if (result === undefined) throw new Error(`Invalid tab id: ${tabID}`)
 
-        return result
+        return this.getResolvedTabData(result)
       },
       set: (value) => {
         return this.tabMap.set(tabID, value)
@@ -237,20 +259,23 @@ export abstract class TabService<Doc>
     this.currentTabID.value = tabID
   }
 
-  private getPersistedDocument(tabDoc: Doc): Doc {
+  public getPersistedDocument(tabDoc: Doc): Doc {
     const { saveContext } = tabDoc as HoppRESTDocument
 
     if (saveContext?.originLocation !== "workspace-user-collection") {
       return tabDoc
     }
 
-    const { requestHandle } = saveContext
+    // TODO: Investigate why requestHandle is available unwrapped here
+    const requestHandle = saveContext.requestHandle as
+      | HandleRef<WorkspaceRequest>["value"]
+      | undefined
 
     if (!requestHandle) {
       return tabDoc
     }
 
-    if (requestHandle.value.type === "invalid") {
+    if (requestHandle.type === "invalid") {
       // eslint-disable-next-line @typescript-eslint/no-unused-vars
       const { requestHandle, ...rest } = saveContext
 
@@ -261,7 +286,7 @@ export abstract class TabService<Doc>
       }
     }
 
-    const { providerID, workspaceID, requestID } = requestHandle.value.data
+    const { providerID, workspaceID, requestID } = requestHandle.data
 
     // Return the document without the handle
     return {
@@ -279,9 +304,13 @@ export abstract class TabService<Doc>
     lastActiveTabID: this.currentTabID.value,
     orderedDocs: this.tabOrdering.value.map((tabID) => {
       const tab = this.tabMap.get(tabID)! // tab ordering is guaranteed to have value for this key
+      const resolvedTabData = this.getResolvedTabData(
+        tab as HoppTab<HoppRESTDocument | HoppGQLDocument>
+      )
+
       return {
         tabID: tab.id,
-        doc: this.getPersistedDocument(tab.document),
+        doc: this.getPersistedDocument(resolvedTabData.document),
       }
     }),
   }))
@@ -299,4 +328,32 @@ export abstract class TabService<Doc>
       if (!this.tabMap.has(id)) return id
     }
   }
+
+  protected getResolvedTabData(
+    tab: HoppTab<HoppRESTDocument | HoppGQLDocument>
+  ): HoppTab<Doc> {
+    if (
+      tab.document.isDirty ||
+      !tab.document.saveContext ||
+      tab.document.saveContext.originLocation !== "workspace-user-collection"
+    ) {
+      return tab as HoppTab<Doc>
+    }
+
+    const requestHandle = tab.document.saveContext.requestHandle as
+      | HandleRef<WorkspaceRequest>["value"]
+      | undefined
+
+    if (!requestHandle) {
+      return tab as HoppTab<Doc>
+    }
+
+    return {
+      ...tab,
+      document: {
+        ...tab.document,
+        isDirty: requestHandle.type === "invalid",
+      },
+    } as HoppTab<Doc>
+  }
 }