fix: remove ref_id field before collection exports and address race conditions (hoppscotch#5626)

Co-authored-by: Copilot <[email protected]>
Co-authored-by: James George <[email protected]>

Author: 3 people

packages/hoppscotch-common/src/components/collections/ImportExport.vue

@@ -60,6 +60,7 @@ import { GistSource } from "~/helpers/import-export/import/import-sources/GistSo
 import { TeamWorkspace } from "~/services/workspace.service"
 import { invokeAction } from "~/helpers/actions"
 import { ReqType } from "~/helpers/backend/graphql"
+import { sanitizeCollection } from "~/helpers/import-export/import"
 
 const isInsomniaImporterInProgress = ref(false)
 const isOpenAPIImporterInProgress = ref(false)
@@ -113,21 +114,35 @@ const handleImportToStore = async (collections: HoppCollection[]) => {
   }
 }
 
+/**
+ * Import collections to personal workspace
+ * We sanitize the collections before importing to remove old id from the imported collection and folders and transform it to new collection format
+ * @param collections Collections to import
+ */
 const importToPersonalWorkspace = (collections: HoppCollection[]) => {
+  // Remove old id from the imported collection and folders and transform it to new collection format
+  const sanitizedCollections = collections.map(sanitizeCollection)
+
   if (
     platform.sync.collections.importToPersonalWorkspace &&
     currentUser.value
   ) {
+    // The SH adds the id to the collection and folders but for safety we remove it by sanitizeCollection
     return platform.sync.collections.importToPersonalWorkspace(
-      collections,
+      sanitizedCollections,
       ReqType.Rest
     )
   }
 
-  appendRESTCollections(collections)
+  appendRESTCollections(sanitizedCollections)
   return E.right({ success: true })
 }
 
+/**
+ * Import collections to teams workspace
+ * No need to sanitize the collections before importing to teams workspace because the BE handles this and add the new id to the collection and folders
+ * @param collections Collections to import
+ */
 const importToTeamsWorkspace = async (collections: HoppCollection[]) => {
   if (!hasTeamWriteAccess.value || !selectedTeamID.value) {
     return E.left({

packages/hoppscotch-common/src/components/collections/index.vue

@@ -313,6 +313,7 @@ import {
   resolveSaveContextOnRequestReorder,
 } from "~/helpers/collection/request"
 import { TeamCollection } from "~/helpers/teams/TeamCollection"
+import { stripRefIdReplacer } from "~/helpers/import-export/export"
 import TeamEnvironmentAdapter from "~/helpers/teams/TeamEnvironmentAdapter"
 import { TeamSearchService } from "~/helpers/teams/TeamsSearch.service"
 import { HoppInheritedProperty } from "~/helpers/types/HoppInheritedProperties"
@@ -2871,7 +2872,7 @@ const initializeDownloadCollection = async (
  */
 const exportData = async (collection: HoppCollection | TeamCollection) => {
   if (collectionsType.value.type === "my-collections") {
-    const collectionJSON = JSON.stringify(collection, null, 2)
+    const collectionJSON = JSON.stringify(collection, stripRefIdReplacer, 2)
 
     // Strip `export {};\n` from `testScript` and `preRequestScript` fields
     const cleanedCollectionJSON = collectionJSON.replace(
@@ -2896,7 +2897,11 @@ const exportData = async (collection: HoppCollection | TeamCollection) => {
         },
         async (coll) => {
           const hoppColl = teamCollToHoppRESTColl(coll)
-          const collectionJSONString = JSON.stringify(hoppColl, null, 2)
+          const collectionJSONString = JSON.stringify(
+            hoppColl,
+            stripRefIdReplacer,
+            2
+          )
 
           // Strip `export {};\n` from `testScript` and `preRequestScript` fields
           const cleanedCollectionJSON = collectionJSONString.replace(

packages/hoppscotch-common/src/helpers/backend/helpers.ts

@@ -25,6 +25,7 @@ import {
   GetCollectionRequestsDocument,
   GetCollectionTitleAndDataDocument,
 } from "./graphql"
+import { stripRefIdReplacer } from "../import-export/export"
 
 type TeamCollectionJSON = {
   id: string
@@ -286,7 +287,7 @@ export const getTeamCollectionJSON = async (teamID: string) => {
   }
 
   const hoppCollections = collections.map(teamCollectionJSONToHoppRESTColl)
-  return E.right(JSON.stringify(hoppCollections, null, 2))
+  return E.right(JSON.stringify(hoppCollections, stripRefIdReplacer, 2))
 }
 
 /**

packages/hoppscotch-common/src/helpers/backend/queries/PublishedDocs.ts

@@ -54,7 +54,7 @@ export type PublishedDocQuery = {
   publishedDoc: PublishedDoc
 }
 
-type CollectionFolder = {
+export type CollectionFolder = {
   id?: string
   folders: CollectionFolder[]
   // Backend stores this as any, we translate it to HoppRESTRequest via translateToNewRequest

packages/hoppscotch-common/src/helpers/collection/collection.ts

@@ -8,6 +8,8 @@ import { RESTTabService } from "~/services/tab/rest"
 import { GQLTabService } from "~/services/tab/graphql"
 import { TeamCollectionsService } from "~/services/team-collection.service"
 import { cascadeParentCollectionForProperties } from "~/newstore/collections"
+import { CollectionDataProps } from "../backend/helpers"
+import { CollectionFolder } from "../backend/queries/PublishedDocs"
 
 /**
  * Resolve save context on reorder
@@ -289,25 +291,27 @@ export function getFoldersByPath(
 
 /**
  * Transforms a collection to the format expected by team or personal collections.
- * Extracts auth, headers, and variables into a data object and recursively processes folders.
+ * BE expects CollectionFolder format with a data field containing auth, headers, variables, and description.
  * @param collection The collection to transform
  * @returns The transformed collection
  */
-export function transformCollectionForImport(collection: any): any {
-  const folders: any[] = (collection.folders ?? []).map(
-    transformCollectionForImport
-  )
+export function transformCollectionForImport(
+  collection: HoppCollection
+): CollectionFolder {
+  const folders = (collection.folders ?? []).map(transformCollectionForImport)
 
-  const data = {
+  const data: CollectionDataProps = {
     auth: collection.auth,
     headers: collection.headers,
     variables: collection.variables,
+    description: collection.description,
   }
 
-  const obj = {
-    ...collection,
-    folders,
-    data,
+  const obj: CollectionFolder = {
+    name: collection.name,
+    folders: folders,
+    requests: collection.requests,
+    data: JSON.stringify(data),
   }
 
   if (collection.id) obj.id = collection.id

packages/hoppscotch-common/src/helpers/import-export/export/gqlCollections.ts

@@ -1,5 +1,6 @@
 import { HoppCollection } from "@hoppscotch/data"
+import { stripRefIdReplacer } from "."
 
 export const gqlCollectionsExporter = (gqlCollections: HoppCollection[]) => {
-  return JSON.stringify(gqlCollections, null, 2)
+  return JSON.stringify(gqlCollections, stripRefIdReplacer, 2)
 }

packages/hoppscotch-common/src/helpers/import-export/export/index.ts

@@ -34,3 +34,10 @@ export const initializeDownloadFile = async (
 
   return E.left("state.download_failed")
 }
+
+/**
+ * JSON replacer to remove `_ref_id` from the exported JSON
+ */
+export const stripRefIdReplacer = (key: string, value: any) => {
+  return key === "_ref_id" ? undefined : value
+}

packages/hoppscotch-common/src/helpers/import-export/export/myCollections.ts

@@ -1,5 +1,6 @@
 import { HoppCollection } from "@hoppscotch/data"
+import { stripRefIdReplacer } from "."
 
 export const myCollectionsExporter = (myCollections: HoppCollection[]) => {
-  return JSON.stringify(myCollections, null, 2)
+  return JSON.stringify(myCollections, stripRefIdReplacer, 2)
 }

packages/hoppscotch-common/src/helpers/import-export/import/index.ts

@@ -1,6 +1,11 @@
 import * as TE from "fp-ts/TaskEither"
 import type { Component } from "vue"
 import { StepsOutputList } from "../steps"
+import {
+  HoppCollection,
+  makeCollection,
+  translateToNewRESTCollection,
+} from "@hoppscotch/data"
 
 /**
  * A common error state to be used when the file formats are not expected
@@ -67,3 +72,25 @@ export const defineImporter = <ReturnType, StepType, Errors>(input: {
     ...input,
   }
 }
+
+/**
+ * Sanitize collection for import, removes old id and ref_id from collection and folders, and transforms it to
+ * new collection format with a newly generated ref_id.
+ * @param collection The collection to sanitize
+ * @returns The sanitized collection with new ref_id
+ */
+export const sanitizeCollection = (
+  collection: HoppCollection
+): HoppCollection => {
+  const {
+    id: _id,
+    _ref_id: _refId,
+    v: _v,
+    ...rest
+  } = translateToNewRESTCollection(collection)
+
+  return makeCollection({
+    ...rest,
+    folders: rest.folders.map(sanitizeCollection),
+  })
+}

packages/hoppscotch-common/src/services/__tests__/workspace.service.spec.ts

@@ -54,6 +54,7 @@ describe("WorkspaceService", () => {
     auth: {
       getCurrentUserStream: vi.fn(),
       getCurrentUser: vi.fn(),
+      waitProbableLoginToConfirm: vi.fn().mockResolvedValue(undefined),
     },
   }
 
@@ -267,6 +268,13 @@ describe("WorkspaceService", () => {
   describe("Workspace Synchronization", () => {
     it("should call changeTeamID and fetchTeamPublishedDocs when workspace changes to a team workspace", async () => {
       const container = new TestContainer()
+
+      // Mock user for this test
+      platformMock.auth.getCurrentUser.mockReturnValue({ uid: "test-user" })
+      platformMock.auth.getCurrentUserStream.mockReturnValue(
+        new BehaviorSubject({ uid: "test-user" })
+      )
+
       const service = container.bind(WorkspaceService)
 
       // Access the mocks
@@ -293,6 +301,13 @@ describe("WorkspaceService", () => {
 
     it("should call clearCollections and fetchUserPublishedDocs when workspace changes to personal workspace", async () => {
       const container = new TestContainer()
+
+      // Mock user for this test
+      platformMock.auth.getCurrentUser.mockReturnValue({ uid: "test-user" })
+      platformMock.auth.getCurrentUserStream.mockReturnValue(
+        new BehaviorSubject({ uid: "test-user" })
+      )
+
       const service = container.bind(WorkspaceService)
 
       // Start with a team workspace
@@ -324,6 +339,13 @@ describe("WorkspaceService", () => {
 
     it("should call clearCollections and fetchUserPublishedDocs when workspace changes to team workspace without teamID", async () => {
       const container = new TestContainer()
+
+      // Mock user for this test
+      platformMock.auth.getCurrentUser.mockReturnValue({ uid: "test-user" })
+      platformMock.auth.getCurrentUserStream.mockReturnValue(
+        new BehaviorSubject({ uid: "test-user" })
+      )
+
       const service = container.bind(WorkspaceService)
 
       const teamCollectionServiceMock = (service as any).teamCollectionService
@@ -404,12 +426,41 @@ describe("WorkspaceService", () => {
       await nextTick()
 
       expect(consoleSpy).toHaveBeenCalledWith(
-        "Failed to sync team collections and published docs:",
+        "Failed to sync workspace data:",
         expect.any(Error)
       )
 
       consoleSpy.mockRestore()
     })
+
+    it("should fetch user published docs only when user is authenticated", async () => {
+      // Case 1: No user
+      platformMock.auth.getCurrentUser.mockReturnValue(null)
+      platformMock.auth.getCurrentUserStream.mockReturnValue(
+        new BehaviorSubject(null)
+      )
+      const container1 = new TestContainer()
+      const service1 = container1.bind(WorkspaceService)
+      const docMock1 = (service1 as any).documentationService
+      docMock1.fetchUserPublishedDocs.mockClear()
+
+      service1.changeWorkspace({ type: "personal" })
+      await nextTick()
+      expect(docMock1.fetchUserPublishedDocs).not.toHaveBeenCalled()
+
+      // Case 2: With user
+      platformMock.auth.getCurrentUser.mockReturnValue({ uid: "test-user" })
+      platformMock.auth.getCurrentUserStream.mockReturnValue(
+        new BehaviorSubject({ uid: "test-user" })
+      )
+      const container2 = new TestContainer()
+      const service2 = container2.bind(WorkspaceService)
+      const docMock2 = (service2 as any).documentationService
+
+      // We check if it was called on initialization
+      await nextTick()
+      expect(docMock2.fetchUserPublishedDocs).toHaveBeenCalled()
+    })
   })
 
   describe("areWorkspacesEqual", () => {