fix: adjust normalize managed pages function, add dependency#907
Conversation
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
There was a problem hiding this comment.
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-pages-actions.js (1)
285-294:⚠️ Potential issue | 🟠 MajorUse the
normalizeSelectAllFieldhelper consistently in the create flow to avoid crashes and improve maintainability.At lines 286–294, the function manually implements add-ons normalization without null checks or the shared helper. This differs from the update flow (
normalizeSponsorManagedPageToCustomizeat line 300), which correctly usesnormalizeSelectAllField. Ifentity.add_onsis missing or falsy,.includes()at line 289 and.map()at line 293 will throw.Refactor to use the helper function with a safe fallback:
const normalizeSponsorManagedPage = (entity) => { + const addOns = entity.add_ons ?? entity.allowed_add_ons ?? []; const normalizedEntity = { show_page_ids: entity.pages, - allowed_add_ons: entity.add_ons + ...normalizeSelectAllField( + addOns, + "apply_to_all_add_ons", + "allowed_add_ons" + ) }; - - if (normalizedEntity.allowed_add_ons.includes("all")) { - normalizedEntity.apply_to_all_add_ons = true; - normalizedEntity.allowed_add_ons = []; - } else { - normalizedEntity.allowed_add_ons = entity.add_ons.map((a) => a.id); - normalizedEntity.apply_to_all_add_ons = false; - } return normalizedEntity; };This aligns with the pattern used in the update flow and similar functions like
normalizeSponsorManagedForm, eliminating code duplication and defensive logic gaps.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/sponsor-pages-actions.js` around lines 285 - 294, The create-flow block that manually handles add-ons (using normalizedEntity and entity.add_ons) should be replaced with the shared normalizeSelectAllField helper to avoid null/undefined crashes and duplicate logic; instead of calling .includes()/.map() on entity.add_ons directly, pass (entity.add_ons || []) into normalizeSelectAllField (so normalizeSponsorManagedPageToCustomize and normalizeSponsorManagedForm patterns are matched) and use its returned values to set normalizedEntity.allowed_add_ons and normalizedEntity.apply_to_all_add_ons.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/actions/sponsor-pages-actions.js`:
- Around line 285-294: The create-flow block that manually handles add-ons
(using normalizedEntity and entity.add_ons) should be replaced with the shared
normalizeSelectAllField helper to avoid null/undefined crashes and duplicate
logic; instead of calling .includes()/.map() on entity.add_ons directly, pass
(entity.add_ons || []) into normalizeSelectAllField (so
normalizeSponsorManagedPageToCustomize and normalizeSponsorManagedForm patterns
are matched) and use its returned values to set normalizedEntity.allowed_add_ons
and normalizedEntity.apply_to_all_add_ons.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e8abb547-e8de-4cc3-939e-c75623a5a46e
📒 Files selected for processing (1)
src/actions/sponsor-pages-actions.js
ref:
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit
Release Notes