Skip to content

Commit c1e684e

Browse files
fix: team collection not loading on route change (hoppscotch#5533)
Co-authored-by: jamesgeorge007 <[email protected]>
1 parent 98f07f8 commit c1e684e

File tree

3 files changed

+312
-25
lines changed

3 files changed

+312
-25
lines changed

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

Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,18 @@ vi.mock("~/helpers/teams/TeamListAdapter", () => ({
2525
},
2626
}))
2727

28+
// Mock TeamCollectionsService to prevent i18n dependency issues
29+
vi.mock("../team-collection.service", () => ({
30+
TeamCollectionsService: class MockTeamCollectionsService {
31+
static readonly ID = "TEAM_COLLECTIONS_SERVICE"
32+
33+
changeTeamID = vi.fn()
34+
clearCollections = vi.fn()
35+
36+
onServiceInit = vi.fn()
37+
},
38+
}))
39+
2840
describe("WorkspaceService", () => {
2941
const platformMock = {
3042
auth: {
@@ -239,4 +251,226 @@ describe("WorkspaceService", () => {
239251
expect(listAdapterMock.fetchList).not.toHaveBeenCalled()
240252
})
241253
})
254+
255+
describe("Team Collection Service Synchronization", () => {
256+
it("should call changeTeamID when workspace changes to a team workspace", async () => {
257+
const container = new TestContainer()
258+
const service = container.bind(WorkspaceService)
259+
260+
// Access the team collection service mock
261+
const teamCollectionServiceMock = (service as any).teamCollectionService
262+
263+
// Change to team workspace
264+
service.changeWorkspace({
265+
type: "team",
266+
teamID: "team-123",
267+
teamName: "Test Team",
268+
role: null,
269+
})
270+
271+
await nextTick()
272+
273+
expect(teamCollectionServiceMock.changeTeamID).toHaveBeenCalledWith(
274+
"team-123"
275+
)
276+
})
277+
278+
it("should call clearCollections when workspace changes to personal workspace", async () => {
279+
const container = new TestContainer()
280+
const service = container.bind(WorkspaceService)
281+
282+
// Start with a team workspace
283+
service.changeWorkspace({
284+
type: "team",
285+
teamID: "team-123",
286+
teamName: "Test Team",
287+
role: null,
288+
})
289+
290+
await nextTick()
291+
292+
const teamCollectionServiceMock = (service as any).teamCollectionService
293+
teamCollectionServiceMock.clearCollections.mockClear()
294+
295+
// Change to personal workspace
296+
service.changeWorkspace({
297+
type: "personal",
298+
})
299+
300+
await nextTick()
301+
302+
expect(teamCollectionServiceMock.clearCollections).toHaveBeenCalled()
303+
})
304+
305+
it("should call clearCollections when workspace changes to team workspace without teamID", async () => {
306+
const container = new TestContainer()
307+
const service = container.bind(WorkspaceService)
308+
309+
const teamCollectionServiceMock = (service as any).teamCollectionService
310+
311+
// Change to team workspace without teamID
312+
service.changeWorkspace({
313+
type: "team",
314+
teamID: "",
315+
teamName: "Test Team",
316+
role: null,
317+
})
318+
319+
await nextTick()
320+
321+
expect(teamCollectionServiceMock.clearCollections).toHaveBeenCalled()
322+
})
323+
324+
it("should not sync when workspaces are effectively the same", async () => {
325+
const container = new TestContainer()
326+
const service = container.bind(WorkspaceService)
327+
328+
// Start with a team workspace
329+
service.changeWorkspace({
330+
type: "team",
331+
teamID: "team-123",
332+
teamName: "Test Team",
333+
role: null,
334+
})
335+
336+
await nextTick()
337+
338+
const teamCollectionServiceMock = (service as any).teamCollectionService
339+
teamCollectionServiceMock.changeTeamID.mockClear()
340+
341+
// Change to same team workspace (different name, same ID)
342+
service.changeWorkspace({
343+
type: "team",
344+
teamID: "team-123",
345+
teamName: "Updated Team Name",
346+
role: null,
347+
})
348+
349+
await nextTick()
350+
351+
// Should not call changeTeamID again since it's the same team
352+
expect(teamCollectionServiceMock.changeTeamID).not.toHaveBeenCalled()
353+
})
354+
355+
it("should handle errors during team collection service sync gracefully", async () => {
356+
const container = new TestContainer()
357+
const service = container.bind(WorkspaceService)
358+
359+
const teamCollectionServiceMock = (service as any).teamCollectionService
360+
teamCollectionServiceMock.changeTeamID.mockImplementation(() => {
361+
throw new Error("Sync failed")
362+
})
363+
364+
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {})
365+
366+
// Change to team workspace (should not throw)
367+
expect(() => {
368+
service.changeWorkspace({
369+
type: "team",
370+
teamID: "team-123",
371+
teamName: "Test Team",
372+
role: null,
373+
})
374+
}).not.toThrow()
375+
376+
await nextTick()
377+
378+
expect(consoleSpy).toHaveBeenCalledWith(
379+
"Failed to sync team collections:",
380+
expect.any(Error)
381+
)
382+
383+
consoleSpy.mockRestore()
384+
})
385+
})
386+
387+
describe("areWorkspacesEqual", () => {
388+
let service: WorkspaceService
389+
390+
beforeEach(() => {
391+
const container = new TestContainer()
392+
service = container.bind(WorkspaceService)
393+
})
394+
395+
it("should return false when newWorkspace is undefined", () => {
396+
const result = (service as any).areWorkspacesEqual(undefined, {
397+
type: "personal",
398+
})
399+
expect(result).toBe(false)
400+
})
401+
402+
it("should return false when oldWorkspace is undefined", () => {
403+
const result = (service as any).areWorkspacesEqual(
404+
{ type: "personal" },
405+
undefined
406+
)
407+
expect(result).toBe(false)
408+
})
409+
410+
it("should return true when both workspaces are personal", () => {
411+
const result = (service as any).areWorkspacesEqual(
412+
{ type: "personal" },
413+
{ type: "personal" }
414+
)
415+
expect(result).toBe(true)
416+
})
417+
418+
it("should return true when both workspaces are team workspaces with same teamID", () => {
419+
const workspace1 = {
420+
type: "team",
421+
teamID: "team-123",
422+
teamName: "Team A",
423+
role: null,
424+
}
425+
const workspace2 = {
426+
type: "team",
427+
teamID: "team-123",
428+
teamName: "Team A Updated",
429+
role: null,
430+
}
431+
432+
const result = (service as any).areWorkspacesEqual(workspace1, workspace2)
433+
expect(result).toBe(true)
434+
})
435+
436+
it("should return false when team workspaces have different teamIDs", () => {
437+
const workspace1 = {
438+
type: "team",
439+
teamID: "team-123",
440+
teamName: "Team A",
441+
role: null,
442+
}
443+
const workspace2 = {
444+
type: "team",
445+
teamID: "team-456",
446+
teamName: "Team B",
447+
role: null,
448+
}
449+
450+
const result = (service as any).areWorkspacesEqual(workspace1, workspace2)
451+
expect(result).toBe(false)
452+
})
453+
454+
it("should return false when one is personal and other is team workspace", () => {
455+
const personalWorkspace = { type: "personal" }
456+
const teamWorkspace = {
457+
type: "team",
458+
teamID: "team-123",
459+
teamName: "Team A",
460+
role: null,
461+
}
462+
463+
const result1 = (service as any).areWorkspacesEqual(
464+
personalWorkspace,
465+
teamWorkspace
466+
)
467+
const result2 = (service as any).areWorkspacesEqual(
468+
teamWorkspace,
469+
personalWorkspace
470+
)
471+
472+
expect(result1).toBe(false)
473+
expect(result2).toBe(false)
474+
})
475+
})
242476
})

packages/hoppscotch-common/src/services/team-collection.service.ts

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import { TeamCollection } from "~/helpers/teams/TeamCollection"
3131
import { TeamRequest } from "~/helpers/teams/TeamRequest"
3232
import { runGQLQuery, runGQLSubscription } from "~/helpers/backend/GQLClient"
3333
import { HoppInheritedProperty } from "~/helpers/types/HoppInheritedProperties"
34-
import { WorkspaceService } from "./workspace.service"
3534
import { ref, watch } from "vue"
3635
import { Service } from "dioc"
3736
import { updateInheritedPropertiesForAffectedRequests } from "~/helpers/collection/collection"
@@ -139,8 +138,6 @@ export class TeamCollectionsService extends Service<void> {
139138
private secretEnvironmentService = this.bind(SecretEnvironmentService)
140139
private currentEnvironmentValueService = this.bind(CurrentValueService)
141140

142-
private workspaceService = this.bind(WorkspaceService)
143-
144141
private teamID: string | null = null
145142

146143
public collections = ref<TeamCollection[]>([])
@@ -176,20 +173,13 @@ export class TeamCollectionsService extends Service<void> {
176173
private teamChildCollectionSortedSub: WSubscription | null = null
177174

178175
override onServiceInit() {
179-
// Watch for team change and update the collections accordingly
180-
watch(
181-
() => this.workspaceService.currentWorkspace,
182-
(workspace) => {
183-
if (workspace.value.type === "team" && workspace.value.teamID) {
184-
this.changeTeamID(workspace.value.teamID)
185-
} else {
186-
this.clearCollections()
187-
}
188-
},
189-
{ immediate: true, deep: true }
190-
)
176+
this.collectionLoadingWatcher()
177+
}
191178

192-
// Watch for completion of loading (when all loading flags are cleared) to update inherited properties once
179+
/**
180+
* Watches for loading collections and updates inherited properties once loading is done
181+
*/
182+
private collectionLoadingWatcher() {
193183
watch(
194184
() => this.loadingCollections.value.length,
195185
(loadingCount) => {
@@ -208,7 +198,11 @@ export class TeamCollectionsService extends Service<void> {
208198
)
209199
}
210200

211-
changeTeamID(newTeamID: string | null) {
201+
/**
202+
* Change the current team ID and resets the collections
203+
* @param newTeamID The new team ID to switch to
204+
*/
205+
public changeTeamID(newTeamID: string | null) {
212206
this.teamID = newTeamID
213207
this.collections.value = []
214208
this.entityIDs.clear()
@@ -220,6 +214,17 @@ export class TeamCollectionsService extends Service<void> {
220214
if (this.teamID) this.initialize()
221215
}
222216

217+
/**
218+
* Clears all collections and resets the service state
219+
*/
220+
public clearCollections() {
221+
this.collections.value = []
222+
this.entityIDs.clear()
223+
this.loadingCollections.value = []
224+
this.unsubscribeSubscriptions()
225+
this.teamID = null
226+
}
227+
223228
/**
224229
* Unsubscribes from the subscriptions
225230
* NOTE: Once this is called, no new updates to the tree will be detected
@@ -292,14 +297,6 @@ export class TeamCollectionsService extends Service<void> {
292297
this.collections.value = tree
293298
}
294299

295-
private clearCollections() {
296-
this.collections.value = []
297-
this.entityIDs.clear()
298-
this.loadingCollections.value = []
299-
this.unsubscribeSubscriptions()
300-
this.teamID = null
301-
}
302-
303300
/**
304301
* Loads the root collections of the current team
305302
* @param replace Whether to replace the existing collections or append to them

0 commit comments

Comments
 (0)