Skip to content

Commit 3f5c242

Browse files
thewatermethodCopilot
authored andcommitted
[TTAHUB-5374] Fix collab report export (#3660)
* Fix collab report export * test: make getCSVReports tests self-contained with own fixtures All tests in the getCSVReports describe block now create their own approved CollabReport records in beforeAll and clean them up in afterAll. Queries are scoped via 'id.in' to prevent interference from ambient database state, replacing fragile 'length >= 2' assertions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: expand getCSVReports fixtures to 11 records to prove unlimited default Creating only 2 records meant the old REPORTS_PER_PAGE=10 default would still return all records, so the 'uses default parameters' test passed regardless of whether the limit was 'all' or 10. With 11 records (one more than REPORTS_PER_PAGE), arrayContaining(csvReportIds) can only succeed when the query has no limit — proving getCSVReports truly omits pagination for CSV exports. Also updates the sort test's toHaveLength(2) to toHaveLength(csvReportIds.length) to stay accurate with the larger fixture set. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update collab report model with a transformed virtual field * Fix unit tests for transformers * Add encoding, update test --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent fc9c1c2 commit 3f5c242

6 files changed

Lines changed: 88 additions & 32 deletions

File tree

src/lib/transform.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,7 @@ const collabReportTransformers = [
609609
'displayId',
610610
'regionId',
611611
'name',
612-
'description',
612+
'descriptionFormatted',
613613
'conductMethod',
614614
'isStateActivity',
615615
'duration',

src/models/collabReport.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ const { REPORT_STATUSES, COLLAB_REPORT_PARTICIPANTS } = require('@ttahub/common'
33
const { sortBy } = require('lodash');
44
const { formatDate } = require('../lib/modelHelpers');
55
const { beforeUpdate } = require('./hooks/collabReport');
6+
const { convert } = require('html-to-text');
67

78
export default (sequelize, DataTypes) => {
89
class CollabReport extends Model {
@@ -219,14 +220,21 @@ export default (sequelize, DataTypes) => {
219220
return this.conductMethod;
220221
},
221222
},
223+
descriptionFormatted: {
224+
type: DataTypes.VIRTUAL,
225+
get() {
226+
if (!this.description) return null;
227+
return convert(this.description);
228+
},
229+
},
222230
approvedAt: {
223231
type: DataTypes.VIRTUAL,
224232
get() {
225233
if (this.calculatedStatus !== REPORT_STATUSES.APPROVED) {
226234
return '';
227235
}
228236

229-
if (!this.approvers || !this.approvers.length) {
237+
if (!this.approvers?.length) {
230238
return '';
231239
}
232240

src/routes/collaborationReports/handlers.test.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,20 @@ describe('Collaboration Reports Handlers', () => {
4747
let mockJson: jest.Mock;
4848
let mockSendStatus: jest.Mock;
4949
let mockSend: jest.Mock;
50+
let mockAttachment: jest.Mock;
5051

5152
beforeEach(() => {
5253
mockJson = jest.fn();
5354
mockSendStatus = jest.fn();
5455
mockSend = jest.fn();
56+
mockAttachment = jest.fn();
5557

5658
mockRequest = {};
5759
mockResponse = {
5860
json: mockJson,
5961
sendStatus: mockSendStatus,
6062
send: mockSend,
63+
attachment: mockAttachment,
6164
};
6265

6366
jest.clearAllMocks();
@@ -1344,7 +1347,7 @@ describe('Collaboration Reports Handlers', () => {
13441347

13451348
expect(CRServices.getCSVReports).toHaveBeenCalledWith({});
13461349
expect(collabReportToCsvRecord).not.toHaveBeenCalled();
1347-
expect(mockSend).toHaveBeenCalledWith('\n');
1350+
expect(mockSend).toHaveBeenCalledWith('\ufeff\n');
13481351
});
13491352

13501353
it('should pass query parameters through setReadRegions', async () => {
@@ -1465,7 +1468,9 @@ describe('Collaboration Reports Handlers', () => {
14651468
await sendCollabReportCSV(mockReports, mockResponse);
14661469

14671470
expect(collabReportToCsvRecord).not.toHaveBeenCalled();
1468-
expect(mockSend).toHaveBeenCalledWith('\n');
1471+
const csvCall = mockSend.mock.calls[0][0];
1472+
const lines = csvCall.replace(/^\ufeff/, '').split('\n');
1473+
expect(lines).toEqual(['', '']);
14691474
});
14701475

14711476
it('should handle single report', async () => {
@@ -1494,13 +1499,13 @@ describe('Collaboration Reports Handlers', () => {
14941499

14951500
it('should handle reports with special characters in CSV', async () => {
14961501
const mockReports = [
1497-
{ id: '1', name: 'Report "with quotes"', description: 'Description,with,commas' },
1502+
{ id: '1', name: 'Report "with quotes"', descriptionFormatted: 'Description,with,commas' },
14981503
];
14991504

15001505
const mockCsvRecord = {
15011506
displayId: 'R01-CR-001',
15021507
name: 'Report "with quotes"',
1503-
description: 'Description,with,commas',
1508+
descriptionFormatted: 'Description,with,commas',
15041509
};
15051510

15061511
(collabReportToCsvRecord as jest.Mock).mockResolvedValue(mockCsvRecord);
@@ -1554,20 +1559,23 @@ describe('Collaboration Reports Handlers', () => {
15541559
});
15551560

15561561
it('should generate proper CSV format with correct column structure', async () => {
1557-
const mockReports = [{ id: '1', name: 'Test Report', description: 'Test Description' }];
1562+
const mockReports = [
1563+
{ id: '1', name: 'Test Report', descriptionFormatted: 'Test Description' },
1564+
];
15581565

15591566
const mockCsvRecord = {
15601567
displayId: 'R01-CR-001',
15611568
name: 'Test Report',
1562-
description: 'Test Description',
1569+
descriptionFormatted: 'Test Description',
15631570
};
15641571

15651572
(collabReportToCsvRecord as jest.Mock).mockResolvedValue(mockCsvRecord);
15661573

15671574
await sendCollabReportCSV(mockReports, mockResponse);
15681575

15691576
const csvCall = mockSend.mock.calls[0][0];
1570-
const lines = csvCall.split('\n');
1577+
// Strip the UTF-8 BOM before parsing
1578+
const lines = csvCall.replace(/^\ufeff/, '').split('\n');
15711579

15721580
// Should have header and data lines
15731581
expect(lines.length).toBeGreaterThanOrEqual(2);

src/routes/collaborationReports/handlers.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ export async function sendCollabReportCSV(reports, res) {
148148
header: 'Method',
149149
},
150150
{
151-
key: 'description',
151+
key: 'descriptionFormatted',
152152
header: 'Description',
153153
},
154154
{
@@ -169,7 +169,8 @@ export async function sendCollabReportCSV(reports, res) {
169169

170170
const csvData = stringify(csvRows, options);
171171

172-
res.send(csvData);
172+
res.attachment('collaboration-reports.csv');
173+
res.send(`\ufeff${csvData}`);
173174
}
174175

175176
/**
@@ -188,6 +189,7 @@ export async function downloadReports(req: Request, res: Response) {
188189
// the query here may contain additional filter information
189190
// so we expect the collab reports to have a full filter suite
190191
const reportPayload = await getCSVReports(query);
192+
191193
await sendCollabReportCSV(reportPayload, res);
192194
} catch (error) {
193195
await handleErrors(req, res, error, logContext);

src/services/collabReports.test.js

Lines changed: 58 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -447,14 +447,55 @@ describe('Collab Reports Service', () => {
447447
});
448448

449449
describe('getCSVReports', () => {
450+
let csvReportIds = [];
451+
452+
beforeAll(async () => {
453+
// Create 11 records — one more than REPORTS_PER_PAGE (10) — so that the
454+
// "uses default parameters" test can only pass when the query has no limit.
455+
const reports = await Promise.all(
456+
Array.from({ length: 11 }, (_, i) =>
457+
CollabReport.create({
458+
...reportObject,
459+
name: `CSV export test report ${i + 1}`,
460+
calculatedStatus: REPORT_STATUSES.APPROVED,
461+
})
462+
)
463+
);
464+
465+
const [report1] = reports;
466+
467+
await CollabReportSpecialist.create({
468+
collabReportId: report1.id,
469+
specialistId: mockUserTwo.id,
470+
});
471+
472+
await CollabReportApprover.create({
473+
collabReportId: report1.id,
474+
userId: mockUserTwo.id,
475+
status: 'approved',
476+
note: 'Test approval',
477+
});
478+
479+
csvReportIds = reports.map(({ id }) => id);
480+
});
481+
482+
afterAll(async () => {
483+
await CollabReportSpecialist.destroy({
484+
where: { collabReportId: csvReportIds },
485+
force: true,
486+
});
487+
await CollabReportApprover.destroy({ where: { collabReportId: csvReportIds }, force: true });
488+
await CollabReport.destroy({ where: { id: csvReportIds }, force: true });
489+
});
490+
450491
it('returns all reports when no filters are provided', async () => {
451-
const result = await getCSVReports();
492+
const result = await getCSVReports({ 'id.in': csvReportIds });
452493
expect(Array.isArray(result)).toBe(true);
453-
expect(result.length).toBeGreaterThanOrEqual(2);
494+
expect(result.map(({ id }) => id)).toEqual(expect.arrayContaining(csvReportIds));
454495
});
455496

456497
it('includes all required attributes for CSV export', async () => {
457-
const result = await getCSVReports({ limit: '1' });
498+
const result = await getCSVReports({ 'id.in': csvReportIds, limit: '1' });
458499
expect(result.length).toBe(1);
459500

460501
const report = result[0];
@@ -469,7 +510,7 @@ describe('Collab Reports Service', () => {
469510
});
470511

471512
it('includes author information with roles', async () => {
472-
const result = await getCSVReports({ limit: '1' });
513+
const result = await getCSVReports({ 'id.in': csvReportIds, limit: '1' });
473514
const report = result[0];
474515

475516
expect(report.author).toHaveProperty('fullName');
@@ -479,9 +520,8 @@ describe('Collab Reports Service', () => {
479520
});
480521

481522
it('includes collaborating specialists with roles', async () => {
482-
const result = await getCSVReports({ limit: '10' });
523+
const result = await getCSVReports({ 'id.in': csvReportIds });
483524

484-
// Find a report with specialists
485525
const reportWithSpecialists = result.find(
486526
(r) => r.collaboratingSpecialists && r.collaboratingSpecialists.length > 0
487527
);
@@ -497,9 +537,8 @@ describe('Collab Reports Service', () => {
497537
});
498538

499539
it('includes approvers with user information', async () => {
500-
const result = await getCSVReports({ limit: '10' });
540+
const result = await getCSVReports({ 'id.in': csvReportIds });
501541

502-
// Find a report with approvers
503542
const reportWithApprovers = result.find((r) => r.approvers && r.approvers.length > 0);
504543

505544
expect(reportWithApprovers).toBeTruthy();
@@ -515,50 +554,49 @@ describe('Collab Reports Service', () => {
515554
});
516555

517556
it('respects limit parameter', async () => {
518-
const result = await getCSVReports({ limit: '1' });
557+
const result = await getCSVReports({ 'id.in': csvReportIds, limit: '1' });
519558
expect(result).toHaveLength(1);
520559
});
521560

522561
it('handles "all" limit parameter', async () => {
523-
const result = await getCSVReports({ limit: 'all' });
562+
const result = await getCSVReports({ 'id.in': csvReportIds, limit: 'all' });
524563
expect(Array.isArray(result)).toBe(true);
525-
expect(result.length).toBeGreaterThanOrEqual(2);
564+
expect(result.map(({ id }) => id)).toEqual(expect.arrayContaining(csvReportIds));
526565
});
527566

528567
it('respects sortBy and sortDir parameters', async () => {
529568
const resultAsc = await getCSVReports({
569+
'id.in': csvReportIds,
530570
sortBy: 'Activity_name',
531571
sortDir: 'asc',
532-
limit: '2',
533572
});
534573
const resultDesc = await getCSVReports({
574+
'id.in': csvReportIds,
535575
sortBy: 'Activity_name',
536576
sortDir: 'desc',
537-
limit: '2',
538577
});
539578

540-
expect(resultAsc).toHaveLength(2);
541-
expect(resultDesc).toHaveLength(2);
579+
expect(resultAsc).toHaveLength(csvReportIds.length);
580+
expect(resultDesc).toHaveLength(csvReportIds.length);
542581

543582
// Names should be in opposite order
544583
expect(resultAsc[0].name).not.toBe(resultDesc[0].name);
545584
});
546585

547586
it('includes steps data when available', async () => {
548-
const result = await getCSVReports({ limit: '10' });
587+
const result = await getCSVReports({ 'id.in': csvReportIds });
549588

550589
result.forEach((report) => {
551590
expect(report).toHaveProperty('steps');
552591
expect(Array.isArray(report.steps)).toBe(true);
553592
});
554593
});
555594

556-
it('uses default parameters when none provided', async () => {
557-
const result = await getCSVReports();
595+
it('uses default parameters when none provided, returning all matching records', async () => {
596+
const result = await getCSVReports({ 'id.in': csvReportIds });
558597

559598
expect(Array.isArray(result)).toBe(true);
560-
// Should respect default limit of REPORTS_PER_PAGE (10)
561-
expect(result.length).toBeLessThanOrEqual(10);
599+
expect(result.map(({ id }) => id)).toEqual(expect.arrayContaining(csvReportIds));
562600
});
563601
});
564602

src/services/collabReports.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ export async function getCSVReports({
469469
sortBy = 'updatedAt',
470470
sortDir = 'desc',
471471
offset = '0',
472-
limit = String(REPORTS_PER_PAGE),
472+
limit = 'all',
473473
status = REPORT_STATUSES.APPROVED,
474474
userId,
475475
...filters

0 commit comments

Comments
 (0)