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

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: James George <25279263+jamesgeorge007@users.noreply.github.com>
This commit is contained in:
Nivedin 2025-12-03 23:00:25 +05:30 committed by GitHub
parent 4efe86f2e0
commit cd82eb212d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 167 additions and 35 deletions

View file

@ -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({

View file

@ -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(

View file

@ -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))
}
/**

View file

@ -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

View file

@ -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

View file

@ -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)
}

View file

@ -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
}

View file

@ -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)
}

View file

@ -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),
})
}

View file

@ -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", () => {

View file

@ -112,31 +112,49 @@ export class WorkspaceService extends Service<WorkspaceServiceEvent> {
}
/**
* Sets up synchronization with team collection service and documentation service
* This ensures team collections and published docs are updated when workspace changes
* Sets up synchronization between team collection service and documentation service.
* Ensures that team collections and published docs stay updated whenever
* the workspace or user changes.
*
* Fixes a bug where the initial fetch failed on cloud instances because
* authorization was null during user login. Now we wait for authentication
* to be ready before fetching team collections and published docs.
*/
private setupWorkspaceSync() {
watch(
this._currentWorkspace,
(newWorkspace, oldWorkspace) => {
// Skip update if workspaces are effectively the same
if (this.areWorkspacesEqual(newWorkspace, oldWorkspace)) return
[this._currentWorkspace, this.currentUser],
async ([newWorkspace, user], [oldWorkspace, oldUser]) => {
// Skip if workspace and user haven't changed
if (
this.areWorkspacesEqual(newWorkspace, oldWorkspace) &&
user?.uid === oldUser?.uid
) {
return
}
try {
if (newWorkspace.type === "team" && newWorkspace.teamID) {
// Ensure authentication is ready before fetching docs
if (user) {
await platform.auth.waitProbableLoginToConfirm()
}
if (newWorkspace?.type === "team" && newWorkspace.teamID) {
this.teamCollectionService.changeTeamID(newWorkspace.teamID)
this.documentationService.fetchTeamPublishedDocs(
newWorkspace.teamID
)
if (user) {
await this.documentationService.fetchTeamPublishedDocs(
newWorkspace.teamID
)
}
} else {
this.teamCollectionService.clearCollections()
this.documentationService.fetchUserPublishedDocs()
if (user) {
await this.documentationService.fetchUserPublishedDocs()
}
}
} catch (error) {
console.error(
"Failed to sync team collections and published docs:",
error
)
console.error("Failed to sync workspace data:", error)
}
},
{ immediate: true }

View file

@ -3,6 +3,7 @@ import {
appendGraphqlCollections,
appendRESTCollections,
} from "@hoppscotch/common/newstore/collections"
import { CollectionDataProps } from "@hoppscotch/common/helpers/backend/helpers"
import { HoppCollection } from "@hoppscotch/data"
import * as E from "fp-ts/Either"
import {
@ -64,10 +65,11 @@ export function translateToPersonalCollectionFormat(x: HoppCollection) {
translateToPersonalCollectionFormat
)
const data = {
const data: CollectionDataProps = {
auth: x.auth,
headers: x.headers,
variables: x.variables,
description: x.description,
}
const obj = {