From 7dc6bf9a9da1cc231e59001a4569dff81f1f7b10 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 30 Mar 2026 14:15:27 -0400 Subject: [PATCH] fix: upload attachments stuck loading [dev] [Marfuen] mariano/fix-evidence-upload-dropzone --- .../actions/organization/accept-invitation.ts | 40 ++- .../app/src/app/(app)/[orgId]/layout.test.tsx | 40 ++- apps/app/src/app/(app)/[orgId]/layout.tsx | 11 +- .../[taskId]/components/TaskBody.test.tsx | 130 +++++++++ .../tasks/[taskId]/components/TaskBody.tsx | 248 +++++++++--------- apps/app/src/hooks/use-api-swr.test.tsx | 95 +++++++ apps/app/src/hooks/use-api-swr.ts | 5 +- 7 files changed, 402 insertions(+), 167 deletions(-) create mode 100644 apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/TaskBody.test.tsx create mode 100644 apps/app/src/hooks/use-api-swr.test.tsx diff --git a/apps/app/src/actions/organization/accept-invitation.ts b/apps/app/src/actions/organization/accept-invitation.ts index 30731b97f9..0570f9b8dd 100644 --- a/apps/app/src/actions/organization/accept-invitation.ts +++ b/apps/app/src/actions/organization/accept-invitation.ts @@ -1,8 +1,10 @@ 'use server'; import { createTrainingVideoEntries } from '@/lib/db/employee'; +import { auth } from '@/utils/auth'; import { db } from '@db'; import { revalidatePath, revalidateTag } from 'next/cache'; +import { headers } from 'next/headers'; import { z } from 'zod'; import { authActionClientWithoutOrg } from '../safe-action'; import type { ActionResponse } from '../types'; @@ -72,15 +74,25 @@ export const completeInvitation = authActionClientWithoutOrg }); if (existingMembership) { - if (ctx.session.activeOrganizationId !== invitation.organizationId) { - await db.session.update({ - where: { id: ctx.session.id }, + // Reactivate member before setting active org, since better-auth + // validates membership status when setting the active organization. + if (existingMembership.deactivated) { + await db.member.update({ + where: { id: existingMembership.id }, data: { - activeOrganizationId: invitation.organizationId, + deactivated: false, + role: invitation.role, }, }); } + if (ctx.session.activeOrganizationId !== invitation.organizationId) { + await auth.api.setActiveOrganization({ + headers: await headers(), + body: { organizationId: invitation.organizationId }, + }); + } + await db.invitation.update({ where: { id: invitation.id }, data: { @@ -88,16 +100,6 @@ export const completeInvitation = authActionClientWithoutOrg }, }); - if (existingMembership.deactivated) { - await db.member.update({ - where: { id: existingMembership.id }, - data: { - deactivated: false, - role: invitation.role, - }, - }); - } - revalidatePath(`/${invitation.organization.id}`); revalidateTag(`user_${user.id}`, 'max'); @@ -135,13 +137,9 @@ export const completeInvitation = authActionClientWithoutOrg }, }); - await db.session.update({ - where: { - id: ctx.session.id, - }, - data: { - activeOrganizationId: invitation.organizationId, - }, + await auth.api.setActiveOrganization({ + headers: await headers(), + body: { organizationId: invitation.organizationId }, }); revalidatePath(`/${invitation.organization.id}`); diff --git a/apps/app/src/app/(app)/[orgId]/layout.test.tsx b/apps/app/src/app/(app)/[orgId]/layout.test.tsx index 90e28192be..264012e3e9 100644 --- a/apps/app/src/app/(app)/[orgId]/layout.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/layout.test.tsx @@ -31,6 +31,7 @@ vi.mock('@/lib/api-server', () => ({ })); vi.mock('@/lib/permissions', () => ({ canAccessApp: vi.fn().mockReturnValue(true), + parseRolesString: vi.fn().mockReturnValue(['owner']), })); vi.mock('@/lib/permissions.server', () => ({ resolveUserPermissions: vi.fn().mockResolvedValue([]), @@ -44,7 +45,7 @@ vi.mock('next/dynamic', () => ({ vi.mock('@aws-sdk/client-s3', () => ({ GetObjectCommand: vi.fn() })); vi.mock('@aws-sdk/s3-request-presigner', () => ({ getSignedUrl: vi.fn() })); -import { createMockSession, setupAuthMocks } from '@/test-utils/mocks/auth'; +import { createMockSession, mockAuthApi, setupAuthMocks } from '@/test-utils/mocks/auth'; import { mockDb } from '@/test-utils/mocks/db'; const { default: Layout } = await import('./layout'); @@ -70,10 +71,10 @@ describe('Layout activeOrganizationId sync', () => { deactivated: false, }); mockDb.onboarding.findFirst.mockResolvedValue(null); - mockDb.session.update.mockResolvedValue({}); + mockAuthApi.setActiveOrganization.mockResolvedValue({}); }); - it('should update session directly in DB when session org differs from URL org', async () => { + it('should call setActiveOrganization via auth API when session org differs from URL org', async () => { setupAuthMocks({ session: createMockSession({ id: sessionId, activeOrganizationId: 'org_other' }), }); @@ -83,13 +84,13 @@ describe('Layout activeOrganizationId sync', () => { params: Promise.resolve({ orgId: requestedOrgId }), }); - expect(mockDb.session.update).toHaveBeenCalledWith({ - where: { id: sessionId }, - data: { activeOrganizationId: requestedOrgId }, + expect(mockAuthApi.setActiveOrganization).toHaveBeenCalledWith({ + headers: expect.anything(), + body: { organizationId: requestedOrgId }, }); }); - it('should update session when activeOrganizationId is null', async () => { + it('should call setActiveOrganization when activeOrganizationId is null', async () => { setupAuthMocks({ session: createMockSession({ id: sessionId, activeOrganizationId: null }), }); @@ -99,13 +100,13 @@ describe('Layout activeOrganizationId sync', () => { params: Promise.resolve({ orgId: requestedOrgId }), }); - expect(mockDb.session.update).toHaveBeenCalledWith({ - where: { id: sessionId }, - data: { activeOrganizationId: requestedOrgId }, + expect(mockAuthApi.setActiveOrganization).toHaveBeenCalledWith({ + headers: expect.anything(), + body: { organizationId: requestedOrgId }, }); }); - it('should NOT update session when session org matches URL org', async () => { + it('should NOT call setActiveOrganization when session org matches URL org', async () => { setupAuthMocks({ session: createMockSession({ id: sessionId, activeOrganizationId: requestedOrgId }), }); @@ -115,14 +116,27 @@ describe('Layout activeOrganizationId sync', () => { params: Promise.resolve({ orgId: requestedOrgId }), }); + expect(mockAuthApi.setActiveOrganization).not.toHaveBeenCalled(); + }); + + it('should not do a direct DB session update', async () => { + setupAuthMocks({ + session: createMockSession({ id: sessionId, activeOrganizationId: 'org_other' }), + }); + + await Layout({ + children: null, + params: Promise.resolve({ orgId: requestedOrgId }), + }); + expect(mockDb.session.update).not.toHaveBeenCalled(); }); - it('should continue rendering even if session update fails', async () => { + it('should continue rendering even if setActiveOrganization fails', async () => { setupAuthMocks({ session: createMockSession({ id: sessionId, activeOrganizationId: 'org_other' }), }); - mockDb.session.update.mockRejectedValue(new Error('db update failed')); + mockAuthApi.setActiveOrganization.mockRejectedValue(new Error('API call failed')); const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); diff --git a/apps/app/src/app/(app)/[orgId]/layout.tsx b/apps/app/src/app/(app)/[orgId]/layout.tsx index 997d93d64e..3dcff20d3f 100644 --- a/apps/app/src/app/(app)/[orgId]/layout.tsx +++ b/apps/app/src/app/(app)/[orgId]/layout.tsx @@ -68,16 +68,13 @@ export default async function Layout({ } // Sync activeOrganizationId if it doesn't match the URL's orgId. - // Direct DB update instead of HTTP call to avoid race conditions: - // Next.js renders layouts and pages in parallel, so child pages may call - // serverApi before an HTTP-based sync completes. A direct DB write is faster - // and membership has already been validated above. + // Uses better-auth's API so both server and client-side session state stay in sync. const currentActiveOrgId = session.session.activeOrganizationId; if (!currentActiveOrgId || currentActiveOrgId !== requestedOrgId) { try { - await db.session.update({ - where: { id: session.session.id }, - data: { activeOrganizationId: requestedOrgId }, + await auth.api.setActiveOrganization({ + headers: requestHeaders, + body: { organizationId: requestedOrgId }, }); } catch (error) { console.error('[Layout] Failed to sync activeOrganizationId:', error); diff --git a/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/TaskBody.test.tsx b/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/TaskBody.test.tsx new file mode 100644 index 0000000000..5d440f4b55 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/TaskBody.test.tsx @@ -0,0 +1,130 @@ +import { render, screen } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +// Mock the task API hooks +const mockRefreshAttachments = vi.fn(); +const mockUploadAttachment = vi.fn(); +const mockGetDownloadUrl = vi.fn(); +const mockDeleteAttachment = vi.fn(); + +vi.mock('@/hooks/use-tasks-api', () => ({ + useTaskAttachments: vi.fn(), + useTaskAttachmentActions: vi.fn(() => ({ + uploadAttachment: mockUploadAttachment, + getDownloadUrl: mockGetDownloadUrl, + deleteAttachment: mockDeleteAttachment, + })), +})); + +// Mock UI components to simplify rendering +vi.mock('@trycompai/ui/button', () => ({ + Button: ({ children, ...props }: React.PropsWithChildren>) => ( + + ), +})); +vi.mock('@trycompai/ui/dialog', () => ({ + Dialog: ({ children }: React.PropsWithChildren) =>
{children}
, + DialogContent: ({ children }: React.PropsWithChildren) =>
{children}
, + DialogDescription: ({ children }: React.PropsWithChildren) =>
{children}
, + DialogFooter: ({ children }: React.PropsWithChildren) =>
{children}
, + DialogHeader: ({ children }: React.PropsWithChildren) =>
{children}
, + DialogTitle: ({ children }: React.PropsWithChildren) =>
{children}
, +})); + +import { useTaskAttachments } from '@/hooks/use-tasks-api'; +import { TaskBody } from './TaskBody'; + +const mockUseTaskAttachments = vi.mocked(useTaskAttachments); + +describe('TaskBody', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('should show upload dropzone even when attachments are loading', () => { + mockUseTaskAttachments.mockReturnValue({ + data: undefined, + error: undefined, + isLoading: true, + mutate: mockRefreshAttachments, + isValidating: false, + }); + + render(); + + expect(screen.getByText('Drag and drop files here')).toBeInTheDocument(); + }); + + it('should show upload dropzone when attachments data is undefined (SWR key is null)', () => { + mockUseTaskAttachments.mockReturnValue({ + data: undefined, + error: undefined, + isLoading: false, + mutate: mockRefreshAttachments, + isValidating: false, + }); + + render(); + + expect(screen.getByText('Drag and drop files here')).toBeInTheDocument(); + }); + + it('should show upload dropzone when attachments have loaded successfully', () => { + mockUseTaskAttachments.mockReturnValue({ + data: { data: [], status: 200 } as never, + error: undefined, + isLoading: false, + mutate: mockRefreshAttachments, + isValidating: false, + }); + + render(); + + expect(screen.getByText('Drag and drop files here')).toBeInTheDocument(); + }); + + it('should show upload dropzone when attachments fail to load', () => { + mockUseTaskAttachments.mockReturnValue({ + data: undefined, + error: new Error('Failed to fetch'), + isLoading: false, + mutate: mockRefreshAttachments, + isValidating: false, + }); + + render(); + + expect(screen.getByText('Drag and drop files here')).toBeInTheDocument(); + expect(screen.getByText('Failed to load attachments. Please try again.')).toBeInTheDocument(); + }); + + it('should show loading skeletons while attachments are loading', () => { + mockUseTaskAttachments.mockReturnValue({ + data: undefined, + error: undefined, + isLoading: true, + mutate: mockRefreshAttachments, + isValidating: false, + }); + + const { container } = render(); + + const skeletons = container.querySelectorAll('.animate-pulse'); + expect(skeletons.length).toBe(3); + }); + + it('should not show loading skeletons when attachments have loaded', () => { + mockUseTaskAttachments.mockReturnValue({ + data: { data: [], status: 200 } as never, + error: undefined, + isLoading: false, + mutate: mockRefreshAttachments, + isValidating: false, + }); + + const { container } = render(); + + const skeletons = container.querySelectorAll('.animate-pulse'); + expect(skeletons.length).toBe(0); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/TaskBody.tsx b/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/TaskBody.tsx index 08e3cb3473..b7c89d2867 100644 --- a/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/TaskBody.tsx +++ b/apps/app/src/app/(app)/[orgId]/tasks/[taskId]/components/TaskBody.tsx @@ -302,137 +302,135 @@ export function TaskBody({

Failed to load attachments. Please try again.

)} - {(attachmentsLoading || attachmentsData === undefined) && ( -
- {/* Enhanced loading skeleton for attachments */} - {[1, 2, 3].map((i) => ( -
-
-
-
- ))} -
- )} - - {!attachmentsLoading && attachmentsData !== undefined && ( -
- {/* Existing attachments list */} - {attachments.length > 0 && ( -
- {attachments.map((attachment) => { - const isBusy = busyAttachmentId === attachment.id; - const fileExt = attachment.name.split('.').pop()?.toLowerCase() || ''; - const isPDF = fileExt === 'pdf'; - const isImage = ['jpg', 'jpeg', 'png', 'gif', 'webp'].includes(fileExt); - const isDoc = ['doc', 'docx'].includes(fileExt); - const uploadMonthYear = formatUploadMonthYear(attachment.createdAt); - - const getFileTypeStyles = () => { - if (isPDF) - return 'bg-primary/10 border-primary/20 hover:bg-primary/20 hover:border-primary/30'; - if (isImage) - return 'bg-primary/10 border-primary/20 hover:bg-primary/20 hover:border-primary/30'; - if (isDoc) - return 'bg-primary/10 border-primary/20 hover:bg-primary/20 hover:border-primary/30'; - return 'bg-muted/50 border-border hover:bg-muted/70'; - }; - - const getFileIconColor = () => { - if (isPDF || isImage || isDoc) return 'text-primary'; - return 'text-muted-foreground'; - }; - - return ( -
+ {/* Loading skeleton for attachments */} + {(attachmentsLoading || attachmentsData === undefined) && ( +
+ {[1, 2, 3].map((i) => ( +
+
+
+
+ ))} +
+ )} + + {/* Existing attachments list */} + {!attachmentsLoading && attachments.length > 0 && ( +
+ {attachments.map((attachment) => { + const isBusy = busyAttachmentId === attachment.id; + const fileExt = attachment.name.split('.').pop()?.toLowerCase() || ''; + const isPDF = fileExt === 'pdf'; + const isImage = ['jpg', 'jpeg', 'png', 'gif', 'webp'].includes(fileExt); + const isDoc = ['doc', 'docx'].includes(fileExt); + const uploadMonthYear = formatUploadMonthYear(attachment.createdAt); + + const getFileTypeStyles = () => { + if (isPDF) + return 'bg-primary/10 border-primary/20 hover:bg-primary/20 hover:border-primary/30'; + if (isImage) + return 'bg-primary/10 border-primary/20 hover:bg-primary/20 hover:border-primary/30'; + if (isDoc) + return 'bg-primary/10 border-primary/20 hover:bg-primary/20 hover:border-primary/30'; + return 'bg-muted/50 border-border hover:bg-muted/70'; + }; + + const getFileIconColor = () => { + if (isPDF || isImage || isDoc) return 'text-primary'; + return 'text-muted-foreground'; + }; + + return ( +
+ {isPDF ? ( + + ) : isImage ? ( + + ) : isDoc ? ( + + ) : ( + + )} + + {uploadMonthYear && ( + ({uploadMonthYear}) + )} + - {uploadMonthYear && ( - ({uploadMonthYear}) + )} - -
- ); - })} -
- )} - - {/* Drag and drop zone - always visible */} -
- )} -
- - {isUploading - ? 'Uploading...' - : isDragging - ? 'Drop files here' - : 'Drag and drop files here'} - - {!isUploading && !isDragging && ( - - or click to browse • max 100MB • most file types accepted - - )} + ); + })} +
+ )} + + {/* Drag and drop zone - always visible */} + -
- )} +
+ +
!open && handleReminderClose()}> diff --git a/apps/app/src/hooks/use-api-swr.test.tsx b/apps/app/src/hooks/use-api-swr.test.tsx new file mode 100644 index 0000000000..07cbb1066e --- /dev/null +++ b/apps/app/src/hooks/use-api-swr.test.tsx @@ -0,0 +1,95 @@ +import { renderHook, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +// Mock useActiveOrganization +const mockUseActiveOrganization = vi.fn(); +vi.mock('@/utils/auth-client', () => ({ + useActiveOrganization: () => mockUseActiveOrganization(), +})); + +// Mock useParams +const mockUseParams = vi.fn(); +vi.mock('next/navigation', async () => { + const actual = await vi.importActual('next/navigation'); + return { + ...actual, + useParams: () => mockUseParams(), + useRouter: vi.fn(() => ({ push: vi.fn(), replace: vi.fn(), refresh: vi.fn(), back: vi.fn() })), + usePathname: vi.fn(() => '/'), + useSearchParams: vi.fn(() => new URLSearchParams()), + }; +}); + +// Mock apiClient +const mockGet = vi.fn(); +vi.mock('@/lib/api-client', () => ({ + apiClient: { + get: (...args: unknown[]) => mockGet(...args), + }, + ApiClient: vi.fn(), +})); + +import { useApiSWR } from './use-api-swr'; + +describe('useApiSWR', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockGet.mockResolvedValue({ data: { items: [] }, status: 200 }); + }); + + it('should use activeOrganization ID when available', async () => { + mockUseActiveOrganization.mockReturnValue({ data: { id: 'org_from_auth' } }); + mockUseParams.mockReturnValue({ orgId: 'org_from_url' }); + + const { result } = renderHook(() => useApiSWR('/v1/tasks')); + + await waitFor(() => { + expect(result.current.data).toBeDefined(); + }); + + expect(mockGet).toHaveBeenCalledWith('/v1/tasks'); + }); + + it('should fall back to URL params orgId when activeOrganization has no data', async () => { + mockUseActiveOrganization.mockReturnValue({ data: null }); + mockUseParams.mockReturnValue({ orgId: 'org_from_url' }); + + const { result } = renderHook(() => useApiSWR('/v1/tasks')); + + await waitFor(() => { + expect(result.current.data).toBeDefined(); + }); + + expect(mockGet).toHaveBeenCalledWith('/v1/tasks'); + }); + + it('should not fetch when neither activeOrganization nor URL params have org ID', () => { + mockUseActiveOrganization.mockReturnValue({ data: null }); + mockUseParams.mockReturnValue({}); + + const { result } = renderHook(() => useApiSWR('/v1/tasks')); + + expect(result.current.data).toBeUndefined(); + expect(mockGet).not.toHaveBeenCalled(); + }); + + it('should not fetch when endpoint is null', () => { + mockUseActiveOrganization.mockReturnValue({ data: { id: 'org_123' } }); + mockUseParams.mockReturnValue({ orgId: 'org_123' }); + + const { result } = renderHook(() => useApiSWR(null)); + + expect(result.current.data).toBeUndefined(); + expect(mockGet).not.toHaveBeenCalled(); + }); + + it('should not fetch when enabled is false', () => { + mockUseActiveOrganization.mockReturnValue({ data: { id: 'org_123' } }); + mockUseParams.mockReturnValue({ orgId: 'org_123' }); + + const { result } = renderHook(() => useApiSWR('/v1/tasks', { enabled: false })); + + expect(result.current.data).toBeUndefined(); + expect(mockGet).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/app/src/hooks/use-api-swr.ts b/apps/app/src/hooks/use-api-swr.ts index c104bdd4af..41a81b97bb 100644 --- a/apps/app/src/hooks/use-api-swr.ts +++ b/apps/app/src/hooks/use-api-swr.ts @@ -2,6 +2,7 @@ import { apiClient, ApiResponse } from '@/lib/api-client'; import { useActiveOrganization } from '@/utils/auth-client'; +import { useParams } from 'next/navigation'; import { useMemo } from 'react'; import useSWR, { SWRConfiguration, SWRResponse } from 'swr'; @@ -19,9 +20,11 @@ export function useApiSWR( options: UseApiSWROptions = {}, ): SWRResponse, Error> { const activeOrg = useActiveOrganization(); + const params = useParams<{ orgId?: string }>(); const { enabled = true, ...swrOptions } = options; - const organizationId = activeOrg.data?.id; + // Fall back to URL params org ID when better-auth client hasn't resolved yet + const organizationId = activeOrg.data?.id ?? params?.orgId; // Create stable key for SWR — include org ID for cache scoping const swrKey = useMemo(() => {