fix: adjust payload to customized managed sponsor forms#958
Conversation
Signed-off-by: Tomás Castillo <[email protected]>
📝 WalkthroughWalkthroughThe sponsor customized form reducer now initializes ChangesSponsor Customized Form Normalization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/reducers/sponsors/sponsor-customized-form-reducer.js`:
- Around line 46-52: The current construction of entity spreads payload.response
and maps payload.response.items and it.images directly, which will throw if
items or images are null/undefined; update the logic in the entity creation (the
block that builds entity from payload.response and uses
payload.response.items.map and it.images.map) to use defensive checks/optional
chaining and defaults (e.g., treat payload.response.items as an empty array and
each it.images as an empty array before mapping) so mapping never runs on
null/undefined and preserves existing behavior for present arrays.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e2f79e4-8787-4990-841c-cafb425e32d2
📒 Files selected for processing (1)
src/reducers/sponsors/sponsor-customized-form-reducer.js
Signed-off-by: Tomás Castillo <[email protected]>
| return { | ||
| ...state, | ||
| entity: payload.response | ||
| const entity = { |
There was a problem hiding this comment.
@tomrndom
The fix addresses the bug for one data path (entity-level items from getSponsorManagedForm) but leaves the same bug active on the individual-item edit path (currentItem from getSponsorCustomizedFormItem), meaning images will still be silently dropped in the most common editing scenario. The missing regression test also means the fix is unverified.
| entity: payload.response | ||
| const entity = { | ||
| ...payload.response, | ||
| items: (payload.response.items || []).map((it) => ({ |
There was a problem hiding this comment.
@tomrndom
Add a unit test for RECEIVE_SPONSOR_CUSTOMIZED_FORM in sponsor-customized-form-reducer.js covering: items with images (file_path mapped), items with no images, and empty items array
| @@ -43,10 +43,17 @@ const sponsorCustomizedFormReducer = (state = DEFAULT_STATE, action) => { | |||
| return DEFAULT_STATE; | |||
There was a problem hiding this comment.
@tomrndom
DEFAULT_ENTITY missing items field
fix: Add items: [] to DEFAULT_ENTITY
smarcet
left a comment
smarcet
left a comment
There was a problem hiding this comment.
@tomrndom
Incomplete fix ( same image-drop bug on the individual-item edit path)
The primary edit flow for items goes through getSponsorCustomizedFormItem → RECEIVE_SPONSOR_CUSTOMIZED_FORM_ITEM → currentItem; that reducer case stores images raw (no file_url → file_path mapping), so saveSponsorFormManagedItem → normalizeManagedItem (line 1446: filter((img) => img.file_path)) still silently drops all existing images on save
src/reducers/sponsors/sponsor-customized-form-items-list-reducer.js, RECEIVE_SPONSOR_CUSTOMIZED_FORM_ITEM case (line 108) — currentItem stores raw API images without the mapping
Suggested fix
Apply the same images: (item.images || []).map(img => ({ ...img, file_path: img.file_url })) transform to currentItem in that case
… form item reducer Signed-off-by: Tomás Castillo <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/reducers/sponsors/__tests__/sponsor-customized-form-reducer.test.js`:
- Around line 58-93: Add a new unit test that dispatches
RECEIVE_SPONSOR_CUSTOMIZED_FORM to sponsorCustomizedFormReducer with a payload
whose response.items contains an item object that omits the images property
(e.g., { id: 10, name: "Item A" }), and assert that
result.entity.items[0].images is an empty array; use the same DEFAULT_STATE
setup and naming pattern as the existing tests to ensure the reducer's fallback
for missing images is validated.
In `@src/reducers/sponsors/sponsor-customized-form-items-list-reducer.js`:
- Around line 113-117: The reducer reads item.meta_fields.length without
guarding for a missing meta_fields which can throw; change the meta_fields
normalization to mirror images by first defaulting meta_fields to an empty array
when falsy and then using the length check — e.g., treat (item.meta_fields ||
[]) as the source for the length test and return either the original
item.meta_fields when non-empty or an empty array otherwise so the reducer never
reads .length of undefined (refer to the item.meta_fields expression in the
sponsor-customized-form-items-list-reducer).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7b9d3745-3eac-4fc2-8dbf-c618dbc923ab
📒 Files selected for processing (3)
src/reducers/sponsors/__tests__/sponsor-customized-form-reducer.test.jssrc/reducers/sponsors/sponsor-customized-form-items-list-reducer.jssrc/reducers/sponsors/sponsor-customized-form-reducer.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/reducers/sponsors/sponsor-customized-form-reducer.js
| it("handles items with no images", () => { | ||
| const result = sponsorCustomizedFormReducer(DEFAULT_STATE, { | ||
| type: RECEIVE_SPONSOR_CUSTOMIZED_FORM, | ||
| payload: { | ||
| response: { | ||
| id: 1, | ||
| code: "FORM1", | ||
| items: [{ id: 10, name: "Item A", images: [] }] | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| expect(result.entity.items[0].images).toEqual([]); | ||
| }); | ||
|
|
||
| it("handles empty items array", () => { | ||
| const result = sponsorCustomizedFormReducer(DEFAULT_STATE, { | ||
| type: RECEIVE_SPONSOR_CUSTOMIZED_FORM, | ||
| payload: { | ||
| response: { id: 1, code: "FORM1", items: [] } | ||
| } | ||
| }); | ||
|
|
||
| expect(result.entity.items).toEqual([]); | ||
| }); | ||
|
|
||
| it("handles missing items field", () => { | ||
| const result = sponsorCustomizedFormReducer(DEFAULT_STATE, { | ||
| type: RECEIVE_SPONSOR_CUSTOMIZED_FORM, | ||
| payload: { | ||
| response: { id: 1, code: "FORM1" } | ||
| } | ||
| }); | ||
|
|
||
| expect(result.entity.items).toEqual([]); | ||
| }); |
There was a problem hiding this comment.
Add a test for items where images is missing (not just empty).
You already validate images: [], but the reducer also handles images being absent. A direct test for that case will lock the intended fallback contract.
Suggested test case
it("handles items with no images", () => {
@@
expect(result.entity.items[0].images).toEqual([]);
});
+ it("handles items with missing images field", () => {
+ const result = sponsorCustomizedFormReducer(DEFAULT_STATE, {
+ type: RECEIVE_SPONSOR_CUSTOMIZED_FORM,
+ payload: {
+ response: {
+ id: 1,
+ code: "FORM1",
+ items: [{ id: 10, name: "Item A" }]
+ }
+ }
+ });
+
+ expect(result.entity.items[0].images).toEqual([]);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("handles items with no images", () => { | |
| const result = sponsorCustomizedFormReducer(DEFAULT_STATE, { | |
| type: RECEIVE_SPONSOR_CUSTOMIZED_FORM, | |
| payload: { | |
| response: { | |
| id: 1, | |
| code: "FORM1", | |
| items: [{ id: 10, name: "Item A", images: [] }] | |
| } | |
| } | |
| }); | |
| expect(result.entity.items[0].images).toEqual([]); | |
| }); | |
| it("handles empty items array", () => { | |
| const result = sponsorCustomizedFormReducer(DEFAULT_STATE, { | |
| type: RECEIVE_SPONSOR_CUSTOMIZED_FORM, | |
| payload: { | |
| response: { id: 1, code: "FORM1", items: [] } | |
| } | |
| }); | |
| expect(result.entity.items).toEqual([]); | |
| }); | |
| it("handles missing items field", () => { | |
| const result = sponsorCustomizedFormReducer(DEFAULT_STATE, { | |
| type: RECEIVE_SPONSOR_CUSTOMIZED_FORM, | |
| payload: { | |
| response: { id: 1, code: "FORM1" } | |
| } | |
| }); | |
| expect(result.entity.items).toEqual([]); | |
| }); | |
| it("handles items with no images", () => { | |
| const result = sponsorCustomizedFormReducer(DEFAULT_STATE, { | |
| type: RECEIVE_SPONSOR_CUSTOMIZED_FORM, | |
| payload: { | |
| response: { | |
| id: 1, | |
| code: "FORM1", | |
| items: [{ id: 10, name: "Item A", images: [] }] | |
| } | |
| } | |
| }); | |
| expect(result.entity.items[0].images).toEqual([]); | |
| }); | |
| it("handles items with missing images field", () => { | |
| const result = sponsorCustomizedFormReducer(DEFAULT_STATE, { | |
| type: RECEIVE_SPONSOR_CUSTOMIZED_FORM, | |
| payload: { | |
| response: { | |
| id: 1, | |
| code: "FORM1", | |
| items: [{ id: 10, name: "Item A" }] | |
| } | |
| } | |
| }); | |
| expect(result.entity.items[0].images).toEqual([]); | |
| }); | |
| it("handles empty items array", () => { | |
| const result = sponsorCustomizedFormReducer(DEFAULT_STATE, { | |
| type: RECEIVE_SPONSOR_CUSTOMIZED_FORM, | |
| payload: { | |
| response: { id: 1, code: "FORM1", items: [] } | |
| } | |
| }); | |
| expect(result.entity.items).toEqual([]); | |
| }); | |
| it("handles missing items field", () => { | |
| const result = sponsorCustomizedFormReducer(DEFAULT_STATE, { | |
| type: RECEIVE_SPONSOR_CUSTOMIZED_FORM, | |
| payload: { | |
| response: { id: 1, code: "FORM1" } | |
| } | |
| }); | |
| expect(result.entity.items).toEqual([]); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/reducers/sponsors/__tests__/sponsor-customized-form-reducer.test.js`
around lines 58 - 93, Add a new unit test that dispatches
RECEIVE_SPONSOR_CUSTOMIZED_FORM to sponsorCustomizedFormReducer with a payload
whose response.items contains an item object that omits the images property
(e.g., { id: 10, name: "Item A" }), and assert that
result.entity.items[0].images is an empty array; use the same DEFAULT_STATE
setup and naming pattern as the existing tests to ensure the reducer's fallback
for missing images is validated.
| images: (item.images || []).map((img) => ({ | ||
| ...img, | ||
| file_path: img.file_url | ||
| })), | ||
| meta_fields: item.meta_fields.length > 0 ? item.meta_fields : [] |
There was a problem hiding this comment.
Guard meta_fields before reading .length to avoid reducer crashes.
item.meta_fields.length can throw when the backend omits meta_fields. This path should be normalized the same way as images to prevent runtime TypeError.
Suggested fix
const currentItem = {
...item,
images: (item.images || []).map((img) => ({
...img,
file_path: img.file_url
})),
- meta_fields: item.meta_fields.length > 0 ? item.meta_fields : []
+ meta_fields: Array.isArray(item.meta_fields) ? item.meta_fields : []
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| images: (item.images || []).map((img) => ({ | |
| ...img, | |
| file_path: img.file_url | |
| })), | |
| meta_fields: item.meta_fields.length > 0 ? item.meta_fields : [] | |
| images: (item.images || []).map((img) => ({ | |
| ...img, | |
| file_path: img.file_url | |
| })), | |
| meta_fields: Array.isArray(item.meta_fields) ? item.meta_fields : [] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/reducers/sponsors/sponsor-customized-form-items-list-reducer.js` around
lines 113 - 117, The reducer reads item.meta_fields.length without guarding for
a missing meta_fields which can throw; change the meta_fields normalization to
mirror images by first defaulting meta_fields to an empty array when falsy and
then using the length check — e.g., treat (item.meta_fields || []) as the source
for the length test and return either the original item.meta_fields when
non-empty or an empty array otherwise so the reducer never reads .length of
undefined (refer to the item.meta_fields expression in the
sponsor-customized-form-items-list-reducer).
2 participants
ref: https://app.clickup.com/t/86b66n5jn
Signed-off-by: Tomás Castillo [email protected]
Summary by CodeRabbit