Merge pull request #230 from refactor-group/201-broken-collaboration-cursors

Fix collaboration cursor bubbles not appearing in coaching notes

Author: jhodapp

__tests__/components/ui/coaching-sessions/editor-cache-context.test.tsx

@@ -169,7 +169,7 @@ describe('EditorCacheProvider', () => {
   // THE CRITICAL TEST: Logout cleanup
   it('should destroy TipTap provider when user logs out', async () => {
     let cacheRef: any = null
-    
+
     // Start with logged in user
     render(
       <EditorCacheProvider sessionId="test-session">
@@ -198,4 +198,350 @@ describe('EditorCacheProvider', () => {
     // After reset, the cache should be in loading state (not ready)
     expect(screen.getByTestId('is-ready')).toHaveTextContent('no')
   })
+
+  describe('Provider Lifecycle - Preventing Unnecessary Disconnections', () => {
+    it('should NOT disconnect provider when re-rendering with same sessionId', async () => {
+      const { TiptapCollabProvider } = await import('@hocuspocus/provider')
+
+      const { rerender } = render(
+        <EditorCacheProvider sessionId="test-session">
+          <TestConsumer />
+        </EditorCacheProvider>
+      )
+
+      await waitFor(() => {
+        expect(screen.getByTestId('is-ready')).toHaveTextContent('yes')
+      })
+
+      // Get the mock instance and clear previous calls
+      const mockProvider = vi.mocked(TiptapCollabProvider).mock.results[0]?.value
+      vi.clearAllMocks()
+
+      // Re-render the same component (simulates user clicking in editor)
+      rerender(
+        <EditorCacheProvider sessionId="test-session">
+          <TestConsumer />
+        </EditorCacheProvider>
+      )
+
+      // Wait a bit to ensure no cleanup happens
+      await new Promise(resolve => setTimeout(resolve, 50))
+
+      // CRITICAL: Provider should NOT be disconnected
+      expect(mockProvider?.disconnect).not.toHaveBeenCalled()
+
+      // CRITICAL: Provider should NOT be recreated
+      expect(TiptapCollabProvider).not.toHaveBeenCalled()
+    })
+
+    it('should disconnect old provider when sessionId changes', async () => {
+      const { TiptapCollabProvider } = await import('@hocuspocus/provider')
+
+      const { rerender } = render(
+        <EditorCacheProvider sessionId="session-1">
+          <TestConsumer />
+        </EditorCacheProvider>
+      )
+
+      await waitFor(() => {
+        expect(screen.getByTestId('is-ready')).toHaveTextContent('yes')
+      })
+
+      const oldProvider = vi.mocked(TiptapCollabProvider).mock.results[0]?.value
+
+      // Change session ID
+      rerender(
+        <EditorCacheProvider sessionId="session-2">
+          <TestConsumer />
+        </EditorCacheProvider>
+      )
+
+      await waitFor(() => {
+        // Old provider should be disconnected
+        expect(oldProvider?.disconnect).toHaveBeenCalled()
+      })
+
+      // New provider should be created
+      expect(TiptapCollabProvider).toHaveBeenCalledTimes(2)
+    })
+  })
+
+  describe('Extension Creation', () => {
+    it('should create extensions only once even if synced event fires multiple times', async () => {
+      const { Extensions } = await import('@/components/ui/coaching-sessions/coaching-notes/extensions')
+
+      // Create a mock provider that we can control
+      const { TiptapCollabProvider } = await import('@hocuspocus/provider')
+      let syncedCallback: (() => void) | undefined
+
+      vi.mocked(TiptapCollabProvider).mockImplementationOnce(function() {
+        const provider = {
+          on: vi.fn((event, callback) => {
+            if (event === 'synced') {
+              syncedCallback = callback
+            }
+            return provider
+          }),
+          off: vi.fn(),
+          setAwarenessField: vi.fn(),
+          destroy: vi.fn(),
+          disconnect: vi.fn(),
+          connect: vi.fn()
+        }
+        return provider as any
+      })
+
+      render(
+        <EditorCacheProvider sessionId="test-session">
+          <TestConsumer />
+        </EditorCacheProvider>
+      )
+
+      // Wait for provider initialization
+      await waitFor(() => {
+        expect(syncedCallback).toBeDefined()
+      })
+
+      // Trigger synced event multiple times
+      act(() => {
+        syncedCallback!()
+        syncedCallback!()
+        syncedCallback!()
+      })
+
+      // Extensions should only be created once
+      expect(Extensions).toHaveBeenCalledTimes(1)
+    })
+  })
+
+  describe('Awareness State Management', () => {
+    it('should use setAwarenessField for setting user presence', async () => {
+      const { TiptapCollabProvider } = await import('@hocuspocus/provider')
+
+      render(
+        <EditorCacheProvider sessionId="test-session">
+          <TestConsumer />
+        </EditorCacheProvider>
+      )
+
+      await waitFor(() => {
+        expect(screen.getByTestId('is-ready')).toHaveTextContent('yes')
+      })
+
+      const mockProvider = vi.mocked(TiptapCollabProvider).mock.results[0]?.value
+
+      // Should have called setAwarenessField with presence data
+      expect(mockProvider?.setAwarenessField).toHaveBeenCalledWith(
+        'presence',
+        expect.objectContaining({
+          userId: 'user-1',
+          name: 'Test User',
+          status: 'connected'
+        })
+      )
+    })
+
+    it('should update presence state when awarenessChange event fires', async () => {
+      const { TiptapCollabProvider } = await import('@hocuspocus/provider')
+      let awarenessCallback: ((data: any) => void) | undefined
+
+      vi.mocked(TiptapCollabProvider).mockImplementationOnce(function() {
+        const provider = {
+          on: vi.fn((event, callback) => {
+            if (event === 'awarenessChange') {
+              awarenessCallback = callback
+            }
+            return provider
+          }),
+          off: vi.fn(),
+          setAwarenessField: vi.fn(),
+          destroy: vi.fn(),
+          disconnect: vi.fn(),
+          connect: vi.fn()
+        }
+        return provider as any
+      })
+
+      let cacheRef: any = null
+
+      render(
+        <EditorCacheProvider sessionId="test-session">
+          <TestConsumer onCacheReady={(cache) => { cacheRef = cache }} />
+        </EditorCacheProvider>
+      )
+
+      await waitFor(() => {
+        expect(awarenessCallback).toBeDefined()
+      })
+
+      // Simulate awareness change with user data
+      act(() => {
+        awarenessCallback!({
+          states: [
+            {
+              clientId: 1,
+              presence: {
+                userId: 'user-1',
+                name: 'Test User',
+                relationshipRole: 'Coach',
+                color: '#ff0000',
+                status: 'connected'
+              }
+            },
+            {
+              clientId: 2,
+              presence: {
+                userId: 'user-2',
+                name: 'Other User',
+                relationshipRole: 'Coachee',
+                color: '#00ff00',
+                status: 'connected'
+              }
+            }
+          ]
+        })
+      })
+
+      // Presence state should be updated
+      expect(cacheRef?.presenceState.users.size).toBe(2)
+      expect(cacheRef?.presenceState.users.get('user-1')).toMatchObject({
+        userId: 'user-1',
+        name: 'Test User'
+      })
+    })
+
+    it('should NOT call setAwarenessField on disconnect event (already offline)', async () => {
+      const { TiptapCollabProvider } = await import('@hocuspocus/provider')
+      let disconnectCallback: (() => void) | undefined
+
+      vi.mocked(TiptapCollabProvider).mockImplementationOnce(function() {
+        const provider = {
+          on: vi.fn((event, callback) => {
+            if (event === 'disconnect') {
+              disconnectCallback = callback
+            }
+            return provider
+          }),
+          off: vi.fn(),
+          setAwarenessField: vi.fn(),
+          destroy: vi.fn(),
+          disconnect: vi.fn(),
+          connect: vi.fn()
+        }
+        return provider as any
+      })
+
+      render(
+        <EditorCacheProvider sessionId="test-session">
+          <TestConsumer />
+        </EditorCacheProvider>
+      )
+
+      await waitFor(() => {
+        expect(disconnectCallback).toBeDefined()
+      })
+
+      const mockProvider = vi.mocked(TiptapCollabProvider).mock.results[0]?.value
+      vi.clearAllMocks()
+
+      // Trigger disconnect event
+      act(() => {
+        disconnectCallback!()
+      })
+
+      // Should NOT call setAwarenessField because we're already disconnected
+      // The awareness protocol will handle removing stale clients via timeout
+      expect(mockProvider?.setAwarenessField).not.toHaveBeenCalled()
+    })
+
+    it('should mark users as disconnected when they disappear from awareness states', async () => {
+      const { TiptapCollabProvider } = await import('@hocuspocus/provider')
+      let awarenessCallback: ((data: any) => void) | undefined
+
+      vi.mocked(TiptapCollabProvider).mockImplementationOnce(function() {
+        const provider = {
+          on: vi.fn((event, callback) => {
+            if (event === 'awarenessChange') {
+              awarenessCallback = callback
+            }
+            return provider
+          }),
+          off: vi.fn(),
+          setAwarenessField: vi.fn(),
+          destroy: vi.fn(),
+          disconnect: vi.fn(),
+          connect: vi.fn()
+        }
+        return provider as any
+      })
+
+      let cacheRef: any = null
+
+      render(
+        <EditorCacheProvider sessionId="test-session">
+          <TestConsumer onCacheReady={(cache) => { cacheRef = cache }} />
+        </EditorCacheProvider>
+      )
+
+      await waitFor(() => {
+        expect(awarenessCallback).toBeDefined()
+      })
+
+      // First, simulate both users being connected
+      act(() => {
+        awarenessCallback!({
+          states: [
+            {
+              clientId: 1,
+              presence: {
+                userId: 'user-1',
+                name: 'Test User',
+                relationshipRole: 'Coach',
+                color: '#ff0000',
+                isConnected: true
+              }
+            },
+            {
+              clientId: 2,
+              presence: {
+                userId: 'user-2',
+                name: 'Other User',
+                relationshipRole: 'Coachee',
+                color: '#00ff00',
+                isConnected: true
+              }
+            }
+          ]
+        })
+      })
+
+      // Verify both users are connected
+      expect(cacheRef?.presenceState.users.size).toBe(2)
+      expect(cacheRef?.presenceState.users.get('user-2')?.status).toBe('connected')
+
+      // Now simulate user-2 disappearing (network disconnect)
+      act(() => {
+        awarenessCallback!({
+          states: [
+            {
+              clientId: 1,
+              presence: {
+                userId: 'user-1',
+                name: 'Test User',
+                relationshipRole: 'Coach',
+                color: '#ff0000',
+                isConnected: true
+              }
+            }
+            // user-2 is no longer in the states array
+          ]
+        })
+      })
+
+      // User-2 should still be in the map but marked as disconnected
+      expect(cacheRef?.presenceState.users.size).toBe(2)
+      expect(cacheRef?.presenceState.users.get('user-2')?.status).toBe('disconnected')
+      expect(cacheRef?.presenceState.users.get('user-1')?.status).toBe('connected')
+    })
+  })
 })