LF-5425: supportTicket POST test sends 11 form parts and trips the multer field limit

[litefarm-pr-bot]

Description

Fixes the flakiness in the Owner post support ticket test, which can fail on roughly one CI run in eleven since the upload limits were added (5987f69)

What the mock is still describing

The test builds its request from mocks.fakeSupportTicket(), which has built an array of image URLs since the endpoint first shipped:

// tests/mock.factories.js — fakeSupportTicket
const numberOfImage = faker.datatype.number(10); // 0 to 10 inclusive
const attachments = [];
for (let i = 0; i < numberOfImage; i++) {
  attachments.push(faker.image.imageUrl());
}

Those URLs matched the original design. The endpoint shipped in commit 30643878a (December 2020) inserting the POST body whole, and attachments was — still is — typed as a list of URL strings:

// models/supportTicketModel.js
attachments: {
  type: 'array',
  items: {
    type: 'string',
  },
},

Six days later, commit 3ef07eb95 replaced that design with the upload-and-email one we have now. The same commit added multerDiskUpload, .single('_file_'), the support email, and this line:

// controllers/supportTicketController.js — addSupportTicket
const data = JSON.parse(req.body.data);
data.attachments = []; // <-- added in 3ef07eb95, never removed

So attachments are not uploaded to be discarded: the user's file goes out as an email attachment, and the column is written empty on every insert. Storing attachment URLs in the database is the part that was abandoned, and three traces survive: the schema above, data.attachments = {} in components/Help/index.jsx, and the mock's array.

Why those URLs now fail the request

The helper passes the mock's array to superagent's .field(), which recurses over arrays: a mock holding ten URLs sends ten parts named _file_, plus the data part:

POST /support_ticket
  _file_   https://…/photo-1.jpg                    part 1
  …
  _file_   https://…/photo-10.jpg                   part 10
  data     {"support_type":"Other","message":"…"}    part 11

multerDiskUpload, the only middleware on the route, caps a request at ten fields, and busboy counts parts as they arrive. Part 11 raises LIMIT_FIELD_COUNT and the request gets a 400 before addSupportTicket runs.

Parts, not keys, is the easy thing to miss. Had the test's request parsed, those eleven parts would have collapsed into two keys, so from inside addSupportTicket it would have looked like an ordinary two-field request:

req.body = {
  _file_: ['https://…/photo-1.jpg', /* … ten URLs in one array */],
  data: '{"support_type":"Other","message":"…"}',
}
req.file = undefined

faker.datatype.number(10) is uniform over 0 to 10 inclusive, so ten URLs is one of eleven equally likely mocks: 9.1% of runs.

Fix

Delete _file_: data.attachments from postRequest, leaving a single data part. req.file stays undefined, exactly as it was when the ten URL parts were there — meaning this test has never exercised the upload path. The real client sends the user's file as a second part, so fields: 10 is nowhere near its limit and stays as it is.

For reviewers

One more piece of the abandoned design, untouched: supportTicketFactory (tests/mock.factories.js) is exported but never called anywhere, and it is the only code that would have inserted those URLs into the database.

Jira link: https://lite-farm.atlassian.net/browse/LF-5425

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

• Passes test case
• UI components visually reviewed on desktop view
• UI components visually reviewed on mobile view
• Other (please explain)

Test-only change, no UI. Owner post support ticket passes locally; the suite also reports an unrelated afterAll timeout in tableCleanup(knex) that every API test file shows on this machine.

Checklist:

• I have commented my code, particularly in hard-to-understand areas
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works — n/a, this is a fix to an existing test
• New and existing unit tests pass locally with my changes — the test passes; the local afterAll timeout is described above
• The precommit and linting ran successfully
• I have added or updated language tags for text that's part of the UI — n/a
• I have ordered translation keys alphabetically — n/a
• I have added the GNU General Public License to all new files — n/a, no new files

[kathyavini]

This is not a very high-stakes ticket so I was using it to explore if I could get Claude to write a more explanatory/story-style PR description. I think I pushed it too far 😅

Here was the original wall of text; having puzzled over this description for a while, now the original reads totally fine to me, but I found it quite opaque at first.

Not a priority, but curious to know which of these two you find more readable -- or if you have a different style preference (e.g. only bullet points). Even better if you could generate that style and share how you prompted it 😁

---

Description

multerDiskUpload recently gained a fields: 10 limit. multer 2.0.2 forwards its limits object verbatim to busboy, and busboy counts multipart parts, not distinct field names — so ten parts all named _file_ count as ten, even though req.body ends up with a single _file_ key. The postRequest helper in the supportTicket test passed the fixture's attachment array to superagent's .field(), which expands an array into one part per element. mocks.fakeSupportTicket generates 0 to 10 attachment URLs inclusive, so the request carried between 1 and 11 parts. At 11, busboy raised LIMIT_FIELD_COUNT and the middleware returned 400 {"message":"attachment could not be uploaded"} before the controller ran. The test then failed twice over: the expect(res.status).toBe(201) assertion threw inside the superagent callback, so done() was never called and Jest also reported a 5 s timeout. Measured rate: 9.1 percent of runs.

This removes _file_: data.attachments from the helper. Those parts carried no coverage — they are text parts rather than file parts, so req.file was undefined either way, and addSupportTicket overwrites data.attachments with an empty array before the insert. The request now sends one part (data), the field count is constant, and every existing assertion is unchanged.

The production limit is correct and is deliberately left alone: the webapp sends exactly two parts, one _file_ and one data, and the farm notes tests send one field plus at most one attachment. The test was the only caller that exceeded the cap. The rule to carry forward is that limits.fields bounds parts on the wire, so any test that hands an array to .field() multiplies its part count invisibly at the call site. Separately, and not addressed here, the fieldNestingDepth: 2 option set alongside the field limit has no effect — neither multer 2.0.2 nor busboy reads that key, so the nesting-depth protection it was meant to provide is not actually in place; that is filed as its own ticket.