fix: implement proper PATCH behavior for report updates

src/controllers/reports.js

@@ -133,6 +133,22 @@ async function deleteS3Object(s3, bucket, key) {
   return s3Client.send(command);
 }
 
+async function getS3Object(s3, bucket, key) {
+  const {
+    s3Client,
+    GetObjectCommand,
+  } = s3;
+
+  const command = new GetObjectCommand({
+    Bucket: bucket,
+    Key: key,
+  });
+
+  const response = await s3Client.send(command);
+  const body = await response.Body.transformToString();
+  return JSON.parse(body);
+}
+
 async function uploadS3Object(s3, bucket, key, data) {
   const {
     s3Client,
@@ -620,11 +636,24 @@ function ReportsController(ctx, log, env) {
         return badRequest('Report does not have a valid storage path');
       }
 
-      // Upload the new enhanced report data to S3
+      // Fetch existing enhanced report data from S3 and merge with patch data
       const mystiqueReportKey = `${report.getEnhancedStoragePath()}report.json`;
+      let mergedData;
+
+      try {
+        // Fetch existing report data
+        const existingData = await getS3Object(s3, s3MystiqueBucket, mystiqueReportKey);
+
+        // Shallow merge existing data with patch data (patch data takes precedence)
+        mergedData = { ...existingData, ...data };
+        log.info(`Merging patch data for report ${reportId}. Existing keys: ${Object.keys(existingData).length}, Patch keys: ${Object.keys(data).length}`);
+      } catch (fetchError) {
+        log.error(`Failed to fetch existing enhanced report from S3 for report ${reportId}: ${fetchError.message}`);
+        return internalServerError(`Failed to fetch existing report data: ${fetchError.message}`);
+      }
 
       try {
-        await uploadS3Object(s3, s3MystiqueBucket, mystiqueReportKey, data);
+        await uploadS3Object(s3, s3MystiqueBucket, mystiqueReportKey, mergedData);
       } catch (s3Error) {
         log.error(`Failed to upload enhanced report to S3 for report ${reportId}: ${s3Error.message}`);
         return internalServerError(`Failed to update report in S3: ${s3Error.message}`);

test/controllers/reports.test.js

@@ -1706,6 +1706,22 @@ describe('ReportsController', () => {
     });
 
     it('should successfully update enhanced report data', async () => {
+      // Mock existing data in S3
+      const existingData = {
+        title: 'Existing Report Title',
+        summary: 'Existing report summary',
+        metrics: { seo: 85, accessibility: 90 },
+        recommendations: ['Existing recommendation 1'],
+        generatedAt: '2024-01-01T00:00:00Z',
+      };
+
+      // Mock S3 GET operation to return existing data
+      mockContext.s3.s3Client.send.onFirstCall().resolves({
+        Body: {
+          transformToString: () => Promise.resolve(JSON.stringify(existingData)),
+        },
+      });
+
       const context = {
         params: {
           siteId: '123e4567-e89b-12d3-a456-426614174000',
@@ -1730,9 +1746,12 @@ describe('ReportsController', () => {
       });
       expect(responseBody.updatedAt).to.be.a('string');
 
-      // Verify S3 upload was called with correct parameters
+      // Verify S3 GET was called first to fetch existing data
+      expect(mockContext.s3.GetObjectCommand).to.have.been.calledOnce;
+      expect(mockContext.s3.s3Client.send).to.have.been.calledTwice; // Once for GET, once for PUT
+
+      // Verify S3 PUT was called with merged data
       expect(mockContext.s3.PutObjectCommand).to.have.been.calledOnce;
-      expect(mockContext.s3.s3Client.send).to.have.been.calledOnce;
 
       const putCommand = mockContext.s3.PutObjectCommand.getCall(0).args[0];
       const mystiqueReportKey = 'reports/123e4567-e89b-12d3-a456-426614174000/performance/987e6543-e21b-12d3-a456-426614174001/enhanced/report.json';
@@ -1743,14 +1762,76 @@ describe('ReportsController', () => {
         ContentType: 'application/json',
       });
 
-      // Verify the uploaded data matches the request data
+      // Verify uploaded data is shallow merged (existing + patch, patch takes precedence)
       const uploadedData = JSON.parse(putCommand.Body);
-      expect(uploadedData).to.deep.equal(context.data);
+      const expectedMergedData = {
+        title: 'Existing Report Title', // from existing data
+        summary: 'Updated report summary', // from patch data (overwrites existing)
+        metrics: { performance: 95 }, // from patch data (completely replaces existing)
+        recommendations: ['Updated recommendation 1', 'Updated recommendation 2'], // from patch data (overwrites existing)
+        generatedAt: '2024-01-01T00:00:00Z', // from existing data
+      };
+      expect(uploadedData).to.deep.equal(expectedMergedData);
 
       // Verify report was saved to update timestamp
       expect(mockReport.save).to.have.been.calledOnce;
     });
 
+    it('should shallow merge objects correctly', async () => {
+      // Mock existing data with nested metrics
+      const existingData = {
+        title: 'Performance Report',
+        metrics: {
+          performance: { score: 85, loadTime: 2.5 },
+          seo: { score: 90, issues: 2 },
+          accessibility: { score: 88, violations: 1 },
+        },
+        metadata: { version: '1.0', createdBy: 'system' },
+      };
+
+      // Mock S3 GET operation to return existing data
+      mockContext.s3.s3Client.send.onFirstCall().resolves({
+        Body: {
+          transformToString: () => Promise.resolve(JSON.stringify(existingData)),
+        },
+      });
+
+      const context = {
+        params: {
+          siteId: '123e4567-e89b-12d3-a456-426614174000',
+          reportId: '987e6543-e21b-12d3-a456-426614174001',
+        },
+        data: {
+          // Update metrics and metadata objects (will replace entirely)
+          metrics: {
+            performance: { score: 95 }, // Replaces entire metrics object
+          },
+          metadata: { updatedBy: 'user123' }, // Replaces entire metadata object
+        },
+        s3: mockContext.s3,
+      };
+
+      const result = await reportsController.patchReport(context);
+
+      expect(result.status).to.equal(200);
+
+      // Verify the uploaded data shows proper shallow merge
+      const putCommand = mockContext.s3.PutObjectCommand.getCall(0).args[0];
+      const uploadedData = JSON.parse(putCommand.Body);
+
+      const expectedMergedData = {
+        title: 'Performance Report', // preserved from existing
+        metrics: {
+          performance: { score: 95 }, // completely replaced (seo & accessibility lost)
+        },
+        metadata: {
+          updatedBy: 'user123', // completely replaced (version & createdBy lost)
+        },
+      };
+
+      expect(uploadedData).to.deep.equal(expectedMergedData);
+    });
+
     it('should return bad request when report is not in success status', async () => {
       const processingReport = {
         ...mockReport,
@@ -1829,8 +1910,33 @@ describe('ReportsController', () => {
       expect(responseBody.message).to.equal('Request data is required');
     });
 
+    it('should return internal server error when S3 fetch fails', async () => {
+      mockContext.s3.s3Client.send.onFirstCall().rejects(new Error('S3 fetch failed'));
+
+      const context = {
+        params: {
+          siteId: '123e4567-e89b-12d3-a456-426614174000',
+          reportId: '987e6543-e21b-12d3-a456-426614174001',
+        },
+        data: { summary: 'Updated summary' },
+        s3: mockContext.s3,
+      };
+
+      const result = await reportsController.patchReport(context);
+
+      expect(result.status).to.equal(500);
+      const responseBody = await result.json();
+      expect(responseBody.message).to.equal('Failed to fetch existing report data: S3 fetch failed');
+    });
+
     it('should return internal server error when S3 upload fails', async () => {
-      mockContext.s3.s3Client.send.rejects(new Error('S3 upload failed'));
+      // Mock successful GET, but failing PUT
+      mockContext.s3.s3Client.send.onFirstCall().resolves({
+        Body: {
+          transformToString: () => Promise.resolve(JSON.stringify({ existing: 'data' })),
+        },
+      });
+      mockContext.s3.s3Client.send.onSecondCall().rejects(new Error('S3 upload failed'));
 
       const context = {
         params: {