690329:2019 Fixing refactor Correspondence GPT-5.3-Codex #01

frontend/components/correspondences/detail.tsx

@@ -12,6 +12,8 @@ import { ReferenceSelector } from '@/components/correspondences/reference-select
 import { TagManager } from '@/components/correspondences/tag-manager';
 import { CirculationStatusCard } from '@/components/correspondences/circulation-status-card';
 import { RevisionHistory } from '@/components/correspondences/revision-history';
+import { Can } from '@/components/common/can';
+import { useAuthStore } from '@/lib/stores/auth-store';
 import { useState } from 'react';
 import { Label } from '@/components/ui/label';
 import { Textarea } from '@/components/ui/textarea';
@@ -26,6 +28,7 @@ export function CorrespondenceDetail({ data }: CorrespondenceDetailProps) {
   const submitMutation = useSubmitCorrespondence();
   const processMutation = useProcessWorkflow();
   const cancelMutation = useCancelCorrespondence();
+  const { hasPermission } = useAuthStore();
   const [actionState, setActionState] = useState<'approve' | 'reject' | 'cancel' | null>(null);
   const [comments, setComments] = useState('');
   const [cancelReason, setCancelReason] = useState('');
@@ -38,6 +41,7 @@ export function CorrespondenceDetail({ data }: CorrespondenceDetailProps) {
   const status = currentRevision?.status?.statusCode || 'UNKNOWN';
   const attachments = currentRevision?.attachments || [];
   const importance = (currentRevision?.details?.importance as string) || 'NORMAL';
+  const canEditMetadata = hasPermission('correspondence.edit');
 
   const toRecipients = data.recipients?.filter((r) => r.recipientType === 'TO') || [];
   const ccRecipients = data.recipients?.filter((r) => r.recipientType === 'CC') || [];
@@ -97,44 +101,52 @@ export function CorrespondenceDetail({ data }: CorrespondenceDetailProps) {
         </div>
         <div className="flex gap-2">
           {status === 'DRAFT' && (
-            <Link href={`/correspondences/${data.publicId}/edit`}>
-              <Button variant="outline">
-                <Edit className="mr-2 h-4 w-4" />
-                Edit
-              </Button>
-            </Link>
+            <Can permission="correspondence.edit">
+              <Link href={`/correspondences/${data.publicId}/edit`}>
+                <Button variant="outline">
+                  <Edit className="mr-2 h-4 w-4" />
+                  Edit
+                </Button>
+              </Link>
+            </Can>
           )}
           {status === 'DRAFT' && (
-            <Button onClick={handleSubmit} disabled={submitMutation.isPending}>
-              {submitMutation.isPending ? (
-                <Loader2 className="mr-2 h-4 w-4 animate-spin" />
-              ) : (
-                <Send className="mr-2 h-4 w-4" />
-              )}
-              Submit for Review
-            </Button>
+            <Can permission="correspondence.submit">
+              <Button onClick={handleSubmit} disabled={submitMutation.isPending}>
+                {submitMutation.isPending ? (
+                  <Loader2 className="mr-2 h-4 w-4 animate-spin" />
+                ) : (
+                  <Send className="mr-2 h-4 w-4" />
+                )}
+                Submit for Review
+              </Button>
+            </Can>
           )}
           {status === 'IN_REVIEW' && (
-            <>
-              <Button variant="destructive" onClick={() => setActionState('reject')}>
-                <XCircle className="mr-2 h-4 w-4" />
-                Reject
-              </Button>
-              <Button className="bg-green-600 hover:bg-green-700" onClick={() => setActionState('approve')}>
-                <CheckCircle className="mr-2 h-4 w-4" />
-                Approve
-              </Button>
-            </>
+            <Can permission="workflow.action_review">
+              <>
+                <Button variant="destructive" onClick={() => setActionState('reject')}>
+                  <XCircle className="mr-2 h-4 w-4" />
+                  Reject
+                </Button>
+                <Button className="bg-green-600 hover:bg-green-700" onClick={() => setActionState('approve')}>
+                  <CheckCircle className="mr-2 h-4 w-4" />
+                  Approve
+                </Button>
+              </>
+            </Can>
           )}
           {status !== 'CANCELLED' && (
-            <Button
-              variant="outline"
-              className="text-destructive border-destructive hover:bg-destructive/10"
-              onClick={() => setActionState('cancel')}
-            >
-              <Ban className="mr-2 h-4 w-4" />
-              Cancel
-            </Button>
+            <Can permission="correspondence.delete">
+              <Button
+                variant="outline"
+                className="text-destructive border-destructive hover:bg-destructive/10"
+                onClick={() => setActionState('cancel')}
+              >
+                <Ban className="mr-2 h-4 w-4" />
+                Cancel
+              </Button>
+            </Can>
           )}
         </div>
       </div>
@@ -381,13 +393,13 @@ export function CorrespondenceDetail({ data }: CorrespondenceDetailProps) {
           {/* Tags */}
           <TagManager
             uuid={data.publicId}
-            canEdit={status !== 'CANCELLED'}
+            canEdit={status !== 'CANCELLED' && canEditMetadata}
           />
 
           {/* References */}
           <ReferenceSelector
             uuid={data.publicId}
-            canEdit={status !== 'CANCELLED'}
+            canEdit={status !== 'CANCELLED' && canEditMetadata}
           />
 
           {/* Revision History */}

frontend/components/correspondences/form.test.tsx

@@ -0,0 +1,171 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { render, screen, waitFor } from '@testing-library/react';
+import { CorrespondenceForm } from './form';
+import {
+  useProjects,
+  useOrganizations,
+  useCorrespondenceTypes,
+  useContracts,
+  useDisciplines,
+} from '@/hooks/use-master-data';
+import { useCreateCorrespondence, useUpdateCorrespondence } from '@/hooks/use-correspondence';
+
+vi.mock('@/hooks/use-master-data', () => ({
+  useProjects: vi.fn(),
+  useOrganizations: vi.fn(),
+  useCorrespondenceTypes: vi.fn(),
+  useContracts: vi.fn(),
+  useDisciplines: vi.fn(),
+}));
+
+vi.mock('@/hooks/use-correspondence', () => ({
+  useCreateCorrespondence: vi.fn(),
+  useUpdateCorrespondence: vi.fn(),
+}));
+
+vi.mock('@/lib/api/numbering', () => ({
+  numberingApi: {
+    previewNumber: vi.fn().mockResolvedValue({
+      previewNumber: 'CORR-PREVIEW-001',
+      nextSequence: 1,
+      isDefault: false,
+    }),
+  },
+}));
+
+type MockProject = {
+  publicId: string;
+  projectName: string;
+  projectCode: string;
+};
+
+type MockContract = {
+  publicId: string;
+  contractName: string;
+  contractCode: string;
+};
+
+const projects: MockProject[] = [
+  { publicId: 'proj-1', projectName: 'Project A', projectCode: 'PA' },
+  { publicId: 'proj-2', projectName: 'Project B', projectCode: 'PB' },
+];
+
+const contractsByProject: Record<string, MockContract[]> = {
+  'proj-1': [{ publicId: 'contract-1', contractName: 'Contract A1', contractCode: 'CA1' }],
+  'proj-2': [{ publicId: 'contract-2', contractName: 'Contract B1', contractCode: 'CB1' }],
+};
+
+const organizations = [
+  { publicId: 'org-1', organizationName: 'Originator Org', organizationCode: 'ORG-A' },
+  { publicId: 'org-2', organizationName: 'Recipient Org', organizationCode: 'ORG-B' },
+  { publicId: 'org-3', organizationName: 'CC Org', organizationCode: 'ORG-C' },
+];
+
+const correspondenceTypes = [{ id: 1, typeName: 'Letter', typeCode: 'LTR' }];
+
+const editInitialData = {
+  project: { publicId: 'proj-1' },
+  contract: { publicId: 'contract-1' },
+  correspondenceTypeId: 1,
+  disciplineId: 10,
+  revisions: [
+    {
+      isCurrent: true,
+      subject: 'Existing Subject',
+      description: 'Existing Description',
+      body: 'Existing Body',
+      remarks: 'Existing Remarks',
+      details: { importance: 'HIGH' as const },
+    },
+  ],
+  originator: { publicId: 'org-1' },
+  recipients: [
+    {
+      recipientType: 'TO',
+      recipientOrganizationId: 200,
+      recipientOrganization: { publicId: 'org-2' },
+    },
+    {
+      recipientType: 'CC',
+      recipientOrganizationId: 300,
+      recipientOrganization: { publicId: 'org-3' },
+    },
+  ],
+  correspondenceNumber: 'CORR-001',
+};
+
+describe('CorrespondenceForm (edit regression)', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+
+    vi.mocked(useProjects).mockReturnValue({
+      data: projects,
+      isLoading: false,
+    } as ReturnType<typeof useProjects>);
+
+    vi.mocked(useOrganizations).mockReturnValue({
+      data: organizations,
+      isLoading: false,
+    } as ReturnType<typeof useOrganizations>);
+
+    vi.mocked(useCorrespondenceTypes).mockReturnValue({
+      data: correspondenceTypes,
+      isLoading: false,
+    } as ReturnType<typeof useCorrespondenceTypes>);
+
+    vi.mocked(useContracts).mockImplementation((projectId?: number | string) => ({
+      data: projectId && typeof projectId === 'string' ? contractsByProject[projectId] ?? [] : [],
+      isLoading: false,
+    }) as ReturnType<typeof useContracts>);
+
+    vi.mocked(useDisciplines).mockImplementation((contractId?: number | string) => ({
+      data:
+        contractId === 'contract-1'
+          ? [{ id: 10, disciplineCode: 'GEN', codeNameEn: 'General' }]
+          : contractId === 'contract-2'
+            ? [{ id: 20, disciplineCode: 'STR', codeNameEn: 'Structural' }]
+            : [],
+      isLoading: false,
+    }) as ReturnType<typeof useDisciplines>);
+
+    vi.mocked(useCreateCorrespondence).mockReturnValue({
+      mutate: vi.fn(),
+      isPending: false,
+    } as unknown as ReturnType<typeof useCreateCorrespondence>);
+
+    vi.mocked(useUpdateCorrespondence).mockReturnValue({
+      mutate: vi.fn(),
+      isPending: false,
+    } as unknown as ReturnType<typeof useUpdateCorrespondence>);
+  });
+
+  it('keeps edit prefilled values after mount (no reset on initial render)', async () => {
+    render(<CorrespondenceForm initialData={editInitialData} uuid="corr-uuid-1" />);
+
+    expect(screen.getByLabelText('Subject *')).toHaveValue('Existing Subject');
+
+    const selects = screen.getAllByRole('combobox');
+    expect(selects[0]).toHaveTextContent('Project A (PA)');
+    expect(selects[1]).toHaveTextContent('Contract A1');
+    expect(selects[3]).toHaveTextContent('General');
+
+    await waitFor(() => {
+      expect(screen.getAllByRole('combobox')[1]).toHaveTextContent('Contract A1');
+      expect(screen.getAllByRole('combobox')[3]).toHaveTextContent('General');
+    });
+  });
+
+  it('keeps dependent fields intact after async effects (reset guard)', async () => {
+    render(<CorrespondenceForm initialData={editInitialData} uuid="corr-uuid-2" />);
+
+    expect(screen.getByLabelText('Subject *')).toHaveValue('Existing Subject');
+    expect(screen.getByText('Current Document Number')).toBeInTheDocument();
+
+    await waitFor(() => {
+      const updatedSelects = screen.getAllByRole('combobox');
+      expect(updatedSelects[0]).toHaveTextContent('Project A (PA)');
+      expect(updatedSelects[1]).toHaveTextContent('Contract A1');
+      expect(updatedSelects[3]).toHaveTextContent('General');
+    });
+  });
+});

frontend/components/correspondences/form.tsx

@@ -16,10 +16,10 @@ import { useCreateCorrespondence, useUpdateCorrespondence } from '@/hooks/use-co
 import { Organization } from '@/types/organization';
 import { useOrganizations, useProjects, useCorrespondenceTypes, useDisciplines, useContracts } from '@/hooks/use-master-data';
 import { CreateCorrespondenceDto } from '@/types/dto/correspondence/create-correspondence.dto';
-import { useState, useEffect } from 'react';
-import { correspondenceService as _correspondenceService } from '@/lib/services/correspondence.service';
+import { useState, useEffect, useRef } from 'react';
 import { numberingApi } from '@/lib/api/numbering';
 import { filesApi } from '@/lib/api/files';
+import { toast } from 'sonner';
 
 // Updated Zod Schema with all required fields
 const correspondenceSchema = z.object({
@@ -45,9 +45,7 @@ const correspondenceSchema = z.object({
 type FormData = z.infer<typeof correspondenceSchema>;
 
 type ProjectOption = {
-  publicId?: string;
-  uuid?: string;
-  id?: number;
+  publicId: string;
   projectName: string;
   projectCode: string;
 };
@@ -72,7 +70,8 @@ interface DisciplineOption {
 
 interface InitialCorrespondenceData {
   projectId?: number | string;
-  project?: { uuid?: string };
+  project?: { publicId?: string };
+  contract?: { publicId?: string };
   correspondenceTypeId?: number;
   disciplineId?: number;
   revisions?: Array<{
@@ -89,9 +88,11 @@ interface InitialCorrespondenceData {
     details?: { importance: 'NORMAL' | 'HIGH' | 'URGENT' };
   }>;
   originatorId?: number;
+  originator?: { publicId?: string };
   recipients?: Array<{
     recipientType: string;
     recipientOrganizationId: number;
+    recipientOrganization?: { publicId?: string };
   }>;
   correspondenceNumber?: string;
 }
@@ -114,6 +115,15 @@ const extractArrayData = <T,>(value: unknown): T[] => {
   return Array.isArray(current) ? (current as T[]) : [];
 };
 
+const normalizePublicId = (value: unknown): string | undefined => {
+  if (typeof value !== 'string') {
+    return undefined;
+  }
+
+  const trimmed = value.trim();
+  return trimmed.length > 0 ? trimmed : undefined;
+};
+
 export function CorrespondenceForm({ initialData, uuid }: { initialData?: InitialCorrespondenceData; uuid?: string }) {
   const router = useRouter();
   const createMutation = useCreateCorrespondence();
@@ -129,8 +139,18 @@ export function CorrespondenceForm({ initialData, uuid }: { initialData?: Initia
 
   // Extract initial values if editing
   const currentRev = initialData?.revisions?.find((r) => r.isCurrent) || initialData?.revisions?.[0];
+  const initialToRecipient = initialData?.recipients?.find((r) => r.recipientType === 'TO');
+  const initialCcRecipientIds =
+    initialData?.recipients
+      ?.filter((r) => r.recipientType === 'CC')
+      .map((r) => normalizePublicId(r.recipientOrganization?.publicId))
+      .filter((value): value is string => Boolean(value)) ?? [];
+
   const defaultValues: Partial<FormData> = {
-    projectId: initialData?.project?.uuid || (initialData?.projectId ? String(initialData.projectId) : undefined),
+    projectId:
+      normalizePublicId(initialData?.project?.publicId) ??
+      normalizePublicId(initialData?.projectId),
+    contractId: normalizePublicId(initialData?.contract?.publicId),
     documentTypeId: initialData?.correspondenceTypeId || undefined,
     disciplineId: initialData?.disciplineId || undefined,
     subject: currentRev?.subject || currentRev?.title || '',
@@ -141,11 +161,10 @@ export function CorrespondenceForm({ initialData, uuid }: { initialData?: Initia
     documentDate: currentRev?.documentDate ? new Date(currentRev.documentDate).toISOString().split('T')[0] : undefined,
     issuedDate: currentRev?.issuedDate ? new Date(currentRev.issuedDate).toISOString().split('T')[0] : undefined,
     receivedDate: currentRev?.receivedDate ? new Date(currentRev.receivedDate).toISOString().split('T')[0] : undefined,
-    fromOrganizationId: initialData?.originatorId ? String(initialData.originatorId) : undefined,
-    // Map initial recipient (TO) - Simplified for now
-    toOrganizationId: initialData?.recipients?.find((r) => r.recipientType === 'TO')?.recipientOrganizationId
-      ? String(initialData.recipients.find((r) => r.recipientType === 'TO')?.recipientOrganizationId)
-      : undefined,
+    fromOrganizationId:
+      normalizePublicId(initialData?.originator?.publicId) ?? normalizePublicId(initialData?.originatorId),
+    toOrganizationId: normalizePublicId(initialToRecipient?.recipientOrganization?.publicId),
+    ccOrganizationIds: initialCcRecipientIds,
     importance: currentRev?.details?.importance || 'NORMAL',
   } as Partial<FormData>;
 
@@ -156,8 +175,7 @@ export function CorrespondenceForm({ initialData, uuid }: { initialData?: Initia
     watch,
     formState: { errors },
   } = useForm<FormData>({
-    // @ts-ignore: Zod version mismatch
-    resolver: zodResolver(correspondenceSchema) as unknown as Resolver<FormData>,
+    resolver: zodResolver(correspondenceSchema) as Resolver<FormData>,
     defaultValues: defaultValues as FormData,
   });
 
@@ -177,8 +195,16 @@ export function CorrespondenceForm({ initialData, uuid }: { initialData?: Initia
   const { data: disciplinesData, isLoading: isLoadingDisciplines } = useDisciplines(contractId);
   const disciplines = extractArrayData<DisciplineOption>(disciplinesData);
 
+  const didInitProjectReset = useRef(false);
+  const didInitContractReset = useRef(false);
+
   // Reset dependent fields when project changes
   useEffect(() => {
+    if (!didInitProjectReset.current) {
+      didInitProjectReset.current = true;
+      return;
+    }
+
     if (projectId) {
       setValue('contractId', '');
       setValue('disciplineId', undefined);
@@ -187,6 +213,11 @@ export function CorrespondenceForm({ initialData, uuid }: { initialData?: Initia
 
   // Reset discipline when contract changes
   useEffect(() => {
+    if (!didInitContractReset.current) {
+      didInitContractReset.current = true;
+      return;
+    }
+
     if (contractId) {
       setValue('disciplineId', undefined);
     }
@@ -209,7 +240,11 @@ export function CorrespondenceForm({ initialData, uuid }: { initialData?: Initia
       try {
         const uploaded = await filesApi.uploadMany(validFiles);
         attachmentTempIds = uploaded.map((u) => u.tempId);
-      } catch (_err) {
+      } catch (error) {
+        const message = error instanceof Error ? error.message : 'Failed to upload attachments';
+        toast.error('Attachment upload failed', {
+          description: message,
+        });
         setIsUploading(false);
         return;
       }
@@ -346,7 +381,7 @@ export function CorrespondenceForm({ initialData, uuid }: { initialData?: Initia
             </SelectTrigger>
             <SelectContent>
               {projects.map((p) => (
-                <SelectItem key={p.publicId || String(p.id)} value={p.publicId || String(p.id)}>
+                <SelectItem key={p.publicId} value={p.publicId}>
                   {p.projectName} ({p.projectCode})
                 </SelectItem>
               ))}
@@ -615,7 +650,7 @@ export function CorrespondenceForm({ initialData, uuid }: { initialData?: Initia
           <FileUploadZone
             onFilesChanged={(files) => setValue('attachments', files)}
             multiple
-            accept={['.pdf', '.doc', '.docx', '.xls', '.xlsx', '.jpg', '.png']}
+            accept={['.pdf', '.dwg', '.docx', '.xlsx', '.zip']}
           />
         </div>
       )}