Fix | SummitDropdown | Type Error - null is not an object (evaluating 'this.state.summitValue.value')#203
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded a Jest test suite (React Testing Library) for SummitDropdown, tightened SummitDropdown input validation and click-guarding logic, and removed an empty top-level Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
Caution Docstrings generation - FAILED No docstrings were generated. |
![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
🧹 Nitpick comments (1)
src/components/summit-dropdown/__tests__/summit-dropdown.test.js (1)
46-60: Seed the invalid-input tests from a selected state.These assertions start with
summitValue === null, so they pass whether invalid input is ignored, clears the selection, or leaves a stale previous choice intact. Set a valid option first, then assert the intended post-condition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/summit-dropdown/__tests__/summit-dropdown.test.js` around lines 46 - 60, The tests currently start with summitValue null so they don't verify that invalid inputs leave an existing selection alone; first seed a valid selection (either by calling wrapper.instance().handleChange(validOption) or setting wrapper.instance().state.summitValue to a valid object), then call wrapper.instance().handleChange('not-an-object') (and similarly for null) and assert that wrapper.instance().state.summitValue still equals the original valid option; reference the render() helper, the component method handleChange, and the state.summitValue field when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/summit-dropdown/index.js`:
- Around line 34-35: The summit option validation should require a defined
.value; update the summit-dropdown logic so handleChange, isDisabled, and any
state updates normalize invalid options to null instead of accepting any object.
Specifically, in the handleChange handler and wherever summitValue is set,
replace the current typeof-object check with a single predicate that checks
option is an object and option.value is not undefined/null (e.g., option &&
typeof option === 'object' && option.value != null) and set summitValue to that
option or null; update isDisabled to use the same predicate (disabled when
summitValue is null or option.value missing) so the button only enables for
usable options.
---
Nitpick comments:
In `@src/components/summit-dropdown/__tests__/summit-dropdown.test.js`:
- Around line 46-60: The tests currently start with summitValue null so they
don't verify that invalid inputs leave an existing selection alone; first seed a
valid selection (either by calling wrapper.instance().handleChange(validOption)
or setting wrapper.instance().state.summitValue to a valid object), then call
wrapper.instance().handleChange('not-an-object') (and similarly for null) and
assert that wrapper.instance().state.summitValue still equals the original valid
option; reference the render() helper, the component method handleChange, and
the state.summitValue field when making the change.
🪄 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: 18a34570-77b6-4882-8d81-3b3e2045fe75
📒 Files selected for processing (3)
src/components/summit-dropdown/__tests__/summit-dropdown.test.jssrc/components/summit-dropdown/index.jssrc/utils/constants.js
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/summit-dropdown/index.js (1)
56-60: Avoid sortingprops.summitsin place.
summits.sort()mutates the prop array during render. Clone it first so this component does not reorder parent-owned data as a side effect.♻️ Suggested change
- let summitOptions = summits - .sort( + let summitOptions = [...summits] + .sort( (a, b) => (a.start_date < b.start_date ? 1 : (a.start_date > b.start_date ? -1 : 0)) ).map(s => ({label: s.name, value: s.id}));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/summit-dropdown/index.js` around lines 56 - 60, The code mutates the incoming prop array by calling sort() directly on summits when building summitOptions; change it to sort a cloned array instead (e.g., clone summits with slice() or spread before calling sort) so the component does not reorder parent-owned data, updating the summitOptions expression that currently references summits.sort(...) to operate on the cloned array.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/summit-dropdown/index.js`:
- Around line 56-60: The code mutates the incoming prop array by calling sort()
directly on summits when building summitOptions; change it to sort a cloned
array instead (e.g., clone summits with slice() or spread before calling sort)
so the component does not reorder parent-owned data, updating the summitOptions
expression that currently references summits.sort(...) to operate on the cloned
array.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0acdd9f1-feb5-4de7-81cb-8c0156101ad7
📒 Files selected for processing (1)
src/components/summit-dropdown/index.js
matiasperrone-exo
force-pushed
the
fix/type-error---null-is-not-an-object-evaluating-this.state.summitvalue.value
branch
from
988b5e2 to
7dac5c7
Compare
matiasperrone-exo
force-pushed
the
fix/type-error---null-is-not-an-object-evaluating-this.state.summitvalue.value
branch
3 times, most recently
from
4d4bb12 to
0a7d308
Compare
matiasperrone-exo
force-pushed
the
fix/type-error---null-is-not-an-object-evaluating-this.state.summitvalue.value
branch
from
0a7d308 to
bcb6dda
Compare
Functionality added |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 194-196: Move "enzyme" out of the top-level "dependencies" object
and into "devDependencies" in package.json, and add the appropriate React 17
adapter package "@wojtekmaj/enzyme-adapter-react-17" under devDependencies
(remove or replace any reference to "enzyme-adapter-react-16"); then update the
test import in src/components/summit-dropdown/__tests__/summit-dropdown.test.js
to import Adapter from '@wojtekmaj/enzyme-adapter-react-17' and ensure any setup
that calls Enzyme.configure uses that Adapter.
🪄 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: 75b3ba45-40d0-4d9c-b436-a2057bf85c53
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
package.jsonsrc/components/summit-dropdown/__tests__/summit-dropdown.test.jssrc/components/summit-dropdown/index.js
✅ Files skipped from review due to trivial changes (1)
- src/components/summit-dropdown/tests/summit-dropdown.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/summit-dropdown/index.js
matiasperrone-exo
force-pushed
the
fix/type-error---null-is-not-an-object-evaluating-this.state.summitvalue.value
branch
from
6991619 to
7b39c70
Compare
4 participants
Task:
Ref: https://app.clickup.com/t/86b93hcm9
Chages Proposed
Enhance the SummitDropdown component to prevent errors when summitValue is null. Implement checks to ensure onClick is only executed with valid summitValue objects.
Summary by CodeRabbit
Bug Fixes
Tests