Skip to content

fix: fix bug in event list with null option#825

Open
priscila-moneo wants to merge 1 commit intomasterfrom
fix/event-list-null-option-crash
Open

fix: fix bug in event list with null option#825
priscila-moneo wants to merge 1 commit intomasterfrom
fix/event-list-null-option-crash

Conversation

@priscila-moneo
Copy link

@priscila-moneo priscila-moneo commented Mar 16, 2026

ref: https://app.clickup.com/t/86b8rpumr

Root cause: The Events page assumed all API items in event_types/tracks were always valid objects, so it directly read name during DDL mapping and crashed when malformed/null entries were returned.

Solution: Centralized and reused a defensive DDL builder that validates items, safely normalizes sorting, and maps options only from valid records before rendering.

Summary by CodeRabbit

  • New Features

    • Enhanced event type management with improved data validation and null-safety checks.
    • Streamlined dropdown list generation for events, locations, tracks, and selection plans.
  • Bug Fixes

    • Improved handling of edge cases when filtering and sorting event data.
    • Strengthened nullish checks to prevent issues with missing or undefined values.
  • Tests

    • Added comprehensive test coverage for selection plan and event type handling.

@coderabbitai
Copy link

coderabbitai bot commented Mar 16, 2026

📝 Walkthrough

Walkthrough

This pull request introduces event type constant renaming, extracts utility functions for dropdown list building, refactors the summit event list page to use these utilities, and adds corresponding unit tests to verify the changes.

Changes

Cohort / File(s) Summary
Event Type Constants Renaming
src/actions/selection-plan-actions.js, src/reducers/selection_plans/selection-plan-reducer.js
Renamed EVENT_TYPE_ADDED/REMOVED constants to SELECTION_PLAN_EVENT_TYPE_ADDED/REMOVED with updated action creator and reducer case references.
Utility Functions
src/utils/events/summit-event-list-page.utils.js
Added sortByOrder() and buildNameIdDDL() utilities for consistent dropdown list generation with filtering, sorting, and normalization logic.
Utility Test Coverage
src/utils/events/__tests__/summit-event-list-page.utils.test.js
Comprehensive unit tests for new utility functions covering edge cases (undefined, null, empty strings, zero-id) and immutability verification.
Page Refactoring
src/pages/events/summit-event-list-page.js
Replaced ad-hoc dropdown generation logic with buildNameIdDDL() utility calls; strengthened nullish checks and validation guards for event type and selection plan resolution.
Reducer Test Coverage
src/reducers/selection_plans/__tests__/selection-plan-reducer.test.js
Added unit test verifying SELECTION_PLAN_EVENT_TYPE_ADDED action handling and proper state accumulation of event types.
Current Summit Reducer Tests
src/reducers/summits/__tests__/current-summit-reducer.test.js
Added tests distinguishing between SELECTION_PLAN_EVENT_TYPE_ADDED (state unchanged) and EVENT_TYPE_ADDED (event types appended) actions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • martinquiroga-exo
  • smarcet
  • romanetar

Poem

🐰 Hopping through the code with glee,
Constants renamed, clean as can be,
Utils extracted, dropdowns aligned,
Tests verify each thoughtful design,
Selection plans sorted and bright!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title references fixing a null option bug in the event list, which aligns with the main objective of adding defensive validation and handling malformed/null entries in dropdown builders.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/event-list-null-option-crash
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@priscila-moneo
Copy link
Author

#821 (comment)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/reducers/selection_plans/selection-plan-reducer.js (1)

122-127: ⚠️ Potential issue | 🟠 Major

Reset allowedMembers on logout as well.

This branch only clears entity and errors. allowedMembers survives the logout, so the next session can briefly inherit stale allowed-member data until a refetch replaces it.

💡 Suggested fix
     case LOGOUT_USER: {
       // we need this in case the token expired while editing the form
       if (payload.hasOwnProperty("persistStore")) {
         return state;
       }
-      return { ...state, entity: { ...DEFAULT_ENTITY }, errors: {} };
+      return {
+        ...DEFAULT_STATE,
+        entity: { ...DEFAULT_ENTITY },
+        allowedMembers: { ...DEFAULT_STATE.allowedMembers },
+        errors: {}
+      };
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/reducers/selection_plans/selection-plan-reducer.js` around lines 122 -
127, The LOGOUT_USER branch currently resets only entity and errors but leaves
allowedMembers populated; update the LOGOUT_USER handler in
selection-plan-reducer (the case handling LOGOUT_USER) to also reset
allowedMembers (e.g., set allowedMembers: [] or to an existing
DEFAULT_ALLOWED_MEMBERS constant) when returning the cleared state while keeping
the early return for payload.hasOwnProperty("persistStore") unchanged so
persisted-logout logic still returns state.
🤖 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/reducers/selection_plans/selection-plan-reducer.js`:
- Around line 122-127: The LOGOUT_USER branch currently resets only entity and
errors but leaves allowedMembers populated; update the LOGOUT_USER handler in
selection-plan-reducer (the case handling LOGOUT_USER) to also reset
allowedMembers (e.g., set allowedMembers: [] or to an existing
DEFAULT_ALLOWED_MEMBERS constant) when returning the cleared state while keeping
the early return for payload.hasOwnProperty("persistStore") unchanged so
persisted-logout logic still returns state.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 26c5e54c-86b4-49e1-89d6-f04c5b53f55b

📥 Commits

Reviewing files that changed from the base of the PR and between 6152c8e and 621ffc4.

📒 Files selected for processing (7)
  • src/actions/selection-plan-actions.js
  • src/pages/events/summit-event-list-page.js
  • src/reducers/selection_plans/__tests__/selection-plan-reducer.test.js
  • src/reducers/selection_plans/selection-plan-reducer.js
  • src/reducers/summits/__tests__/current-summit-reducer.test.js
  • src/utils/events/__tests__/summit-event-list-page.utils.test.js
  • src/utils/events/summit-event-list-page.utils.js

@priscila-moneo priscila-moneo requested a review from smarcet March 16, 2026 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant