Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion jest/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ jest.mock('@libs/prepareRequestPayload/index.native.ts', () => ({
for (const key of Object.keys(data)) {
const value = data[key];

if (value === undefined) {
if (value === undefined || value === null) {
continue;
}

Expand Down
2 changes: 1 addition & 1 deletion src/libs/prepareRequestPayload/index.native.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const prepareRequestPayload: PrepareRequestPayload = (command, data, initiatedOf
promiseChain = promiseChain.then(() => {
const value = data[key];

if (value === undefined) {
if (value === undefined || value === null) {
return Promise.resolve();
}

Expand Down
2 changes: 1 addition & 1 deletion src/libs/prepareRequestPayload/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const prepareRequestPayload: PrepareRequestPayload = (command, data) => {
for (const key of Object.keys(data)) {
const value = data[key];

if (value === undefined) {
if (value === undefined || value === null) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep nullable params in FormData payload

This new value === null filter drops explicit nulls from all requests, but some commands rely on sending a nullable field to clear server-side state. For example, SET_POLICY_TAG_APPROVER intentionally sends approver: null when unselecting an approver (src/libs/actions/Policy/Tag.ts:1283-1289), and the param is defined as required-but-nullable (src/libs/API/parameters/SetPolicyTagApproverParams.ts:1-5). After this change, that key is omitted entirely, so the backend cannot distinguish “clear this value” from “no update,” which can leave stale data.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 I'm pretty sure this behavior is broken for that flow (and others) then, since we're not passing null but rather the stringified version of it

Copy link
Copy Markdown
Contributor Author

@NikkiWines NikkiWines Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the web code, we'd just end up throwing an error here the value doesn't match the expected format 😄

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And, actually, with this fix the backend will check Request::getString('approver') which will return '' and then createOrUpdateApprovalRule receives '', which will return false in strlen and give us the behavior we actually want - so this fixes that bug too :D

continue;
}

Expand Down
40 changes: 40 additions & 0 deletions tests/unit/prepareRequestPayloadTest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import prepareRequestPayload from '@libs/prepareRequestPayload';

describe('prepareRequestPayload', () => {
it('should append string values to FormData', async () => {
const formData = await prepareRequestPayload('TestCommand', {authToken: 'abc123', email: 'test@example.com'}, false);

expect(formData.get('authToken')).toBe('abc123');
expect(formData.get('email')).toBe('test@example.com');
});

it('should omit null values from FormData instead of coercing them to the string "null"', async () => {
const formData = await prepareRequestPayload('TestCommand', {authToken: null, email: null, referer: 'ecash'}, false);

expect(formData.has('authToken')).toBe(false);
expect(formData.has('email')).toBe(false);
expect(formData.get('referer')).toBe('ecash');
});

it('should omit undefined values from FormData', async () => {
const formData = await prepareRequestPayload('TestCommand', {authToken: undefined, platform: 'web'}, false);

expect(formData.has('authToken')).toBe(false);
expect(formData.get('platform')).toBe('web');
});

it('should include falsy non-null/undefined values (0, false, empty string)', async () => {
const formData = await prepareRequestPayload('TestCommand', {count: 0, flag: false, label: ''}, false);

expect(formData.get('count')).toBe('0');
expect(formData.get('flag')).toBe('false');
expect(formData.get('label')).toBe('');
});

it('should return an empty FormData for an empty data object', async () => {
const formData = await prepareRequestPayload('TestCommand', {}, false);
const entries = Array.from(formData.entries());

expect(entries).toHaveLength(0);
});
});
Loading