fix: change managed form endpoint, rename actions, clean dead code#966
Conversation
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR renames getSponsorTiers to getSponsorSponsorships, removes sponsorship add-ons actions/thunks, updates managed-form requests to sponsor-scoped endpoints, and flattens sponsor state from sponsorships_collection.sponsorships to sponsorships, with corresponding wiring and test updates. ChangesSponsor actions and data structure refactor
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Suggested Reviewers
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/actions/sponsor-actions.js (1)
383-388:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftUpdate the reducer contract before switching consumers to
entity.sponsorships.Line 383 still emits
RECEIVE_SPONSOR_SPONSORSHIPS, andsrc/reducers/sponsors/sponsor-reducer.js:503-525currently stores that payload inentity.sponsorships_collection.sponsorships. The rest of this PR stack now readsentity.sponsorships, so the fetch will succeed but the new callers will still render an empty sponsorship list until the reducer migration lands with this rename.🤖 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/actions/sponsor-actions.js` around lines 383 - 388, The reducer contract must be updated so the existing RECEIVE_SPONSOR_SPONSORSHIPS action continues to populate the new location expected by consumers (entity.sponsorships) before you switch callers to that path; modify the sponsor reducer handling of RECEIVE_SPONSOR_SPONSORSHIPS in sponsor-reducer.js (the code that currently writes into entity.sponsorships_collection.sponsorships) to instead set entity.sponsorships (and for safety keep the old sponsorships_collection.sponsorships until all callers are migrated), leaving the getRequest/createAction call and action constant (RECEIVE_SPONSOR_SPONSORSHIPS) unchanged.
🤖 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/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/customized-form/customized-form.js`:
- Line 57: sponsor.sponsorships may be undefined so the map call can throw;
update the calculation of sponsorshipIds to safely handle that by using optional
chaining and a fallback (e.g., compute sponsorshipIds from
sponsor.sponsorships?.map(...) ?? [] or an equivalent null-safe expression) so
that sponsorshipIds becomes an empty array when sponsor.sponsorships is missing;
adjust the code where sponsorshipIds is referenced if it assumes non-empty
values.
In `@src/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.js`:
- Line 384: sponsor.sponsorships may be undefined and calling .map on it can
throw; update the computation of sponsorshipIds to guard against null/undefined
by using optional chaining or a fallback array (e.g., use
sponsor.sponsorships?.map(...) with a fallback to [] or (sponsor.sponsorships ||
[]).map(...)) so the code that computes sponsorshipIds (the line referencing
sponsor.sponsorships.map and the variable sponsorshipIds) never invokes .map on
undefined.
---
Outside diff comments:
In `@src/actions/sponsor-actions.js`:
- Around line 383-388: The reducer contract must be updated so the existing
RECEIVE_SPONSOR_SPONSORSHIPS action continues to populate the new location
expected by consumers (entity.sponsorships) before you switch callers to that
path; modify the sponsor reducer handling of RECEIVE_SPONSOR_SPONSORSHIPS in
sponsor-reducer.js (the code that currently writes into
entity.sponsorships_collection.sponsorships) to instead set entity.sponsorships
(and for safety keep the old sponsorships_collection.sponsorships until all
callers are migrated), leaving the getRequest/createAction call and action
constant (RECEIVE_SPONSOR_SPONSORSHIPS) unchanged.
🪄 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: 8f6658fc-87f5-4cce-ae1c-c5b83b546479
📒 Files selected for processing (9)
src/actions/sponsor-actions.jssrc/actions/sponsor-forms-actions.jssrc/pages/sponsors/__tests__/edit-sponsor-page.test.jssrc/pages/sponsors/sponsor-page/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/customized-form/customized-form.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-general-form/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-general-form/sponsorship.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/__tests__/sponsor-pages-tab.test.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.js
💤 Files with no reviewable changes (1)
- src/pages/sponsors/sponsor-page/tabs/sponsor-general-form/sponsorship.js
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
| const sponsorships = sponsor.sponsorships_collection.sponsorships.map( | ||
| (e) => e.id | ||
| ); | ||
| const sponsorshipIds = sponsor.sponsorships?.map((e) => e.id) || []; |
There was a problem hiding this comment.
@tomrndom define sponsorships: [] at DEFAULT_ENTITY in sponsor-reducer.js
| @@ -54,9 +54,7 @@ const CustomizedForm = ({ | |||
| isSaving = false, | |||
There was a problem hiding this comment.
The entity mock in customized-form-popup.test.js still uses the old shape (sponsorships_collection: { sponsorships: [] }), so sponsor.sponsorships is undefined in every test and sponsorshipIds is always []. The non-empty filtering path is never exercised.
Consider updating the mock to the new shape and adding an assertion that the sponsorship ID reaches the queryParams prop on the MuiFormikSelectGroup that loads add-ons:
// customized-form-popup.test.js
const createSponsor = (overrides = {}) => ({
id: 1,
sponsorships: [{ id: 42 }],
...overrides
});… check sponsorships Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
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-reducer.js`:
- Around line 132-133: The reducer normalizes API sponsor data in the
RECEIVE_SPONSOR handling but coerces null sponsorships to an empty string which
can break consumers expecting an array; update the normalization in the
RECEIVE_SPONSOR branch so that any null/undefined sponsorships field is
converted to an empty array (or DEFAULT_SPONSORHIPS_STATE) instead of "", and
ensure the state key sponsorships remains an array (refer to the sponsorships
state, RECEIVE_SPONSOR handling, and DEFAULT_SPONSORHIPS_STATE to locate and
change the coercion).
🪄 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: 5238a92a-fa74-4817-93c5-a61a7fba0b1b
📒 Files selected for processing (2)
src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/customized-form/__tests__/customized-form-popup.test.jssrc/reducers/sponsors/sponsor-reducer.js
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
) * fix: change managed form endpoint, rename actions, clean dead code Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com> * fix: add conditional chaining for array of sponsorships Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com> * fix: add sponsorship array to reducer, update customized form test to check sponsorships Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com> * fix: preserve sponsorship array type on RECEIVE_SPONSOR Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com> --------- Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
ref: https://app.clickup.com/t/86baayr5u
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit
Refactor
Tests