Skip to content

fix: media type filter type error#849

Open
priscila-moneo wants to merge 2 commits intomasterfrom
fix/media-type-filter-type-error
Open

fix: media type filter type error#849
priscila-moneo wants to merge 2 commits intomasterfrom
fix/media-type-filter-type-error

Conversation

@priscila-moneo
Copy link
Copy Markdown

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

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

Summary by CodeRabbit

  • Bug Fixes

    • Media-type filter now tolerates cleared/null operator selections; changing the media value before or after operator changes no longer throws and emits consistent change events.
  • Style

    • Minor internal state-initialization formatting updated with no behavioral change.
  • Tests

    • Added tests covering operator and media-value interactions, including selecting, clearing, and change-order scenarios.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2fc6738a-ad71-4b85-b84c-14516cb51b7a

📥 Commits

Reviewing files that changed from the base of the PR and between d2403d0 and af94164.

📒 Files selected for processing (2)
  • src/components/filters/media-type-filter/__tests__/media-type-filter.test.js
  • src/components/filters/media-type-filter/index.js

📝 Walkthrough

Walkthrough

Refactors MediaTypeFilter internal handling to allow operator to be null when cleared and adds a Jest test suite that validates operator/value interaction and onChange payloads.

Changes

Cohort / File(s) Summary
Media Type Filter component
src/components/filters/media-type-filter/index.js
Normalized filterValue state initialization and updated onChangeOperator / onChangeFilterValue to use optional chaining (?.value ?? null) and early return when operator value is absent so dispatched operator can be null.
Tests for MediaTypeFilter
src/components/filters/media-type-filter/__tests__/media-type-filter.test.js
Added Jest tests that mock i18n-react, react-select, and the media upload input; verify onChange behavior for operator-first, value-first, and cleared-operator sequences.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • smarcet
  • martinquiroga-exo
  • caseylocker

Poem

🐰 I hopped through code both light and small,
When operators fall, I catch them all.
Values dance, then settle null—so neat,
Tests patter soft with steady feet,
A rabbit's tweak: precise and briskly sweet.

🚥 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 'fix: media type filter type error' directly describes the main change: fixing a type error in the media type filter component by improving null/undefined handling.
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
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/media-type-filter-type-error

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.

Copy link
Copy Markdown

@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.

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 `@src/components/filters/media-type-filter/index.js`:
- Line 53: The operator assignment currently uses newOperatorValue?.value and
operatorValue?.value which can yield undefined; change those assignments so that
when the value is missing you set operator: null (e.g., replace
newOperatorValue?.value / operatorValue?.value with an expression that returns
the value or null). Update the two occurrences where operator is assigned
(references to newOperatorValue and operatorValue in the media-type-filter
component) so downstream code receives null for “not set” instead of undefined.
🪄 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: 4421437b-0c19-4a44-a718-d94ebe9781db

📥 Commits

Reviewing files that changed from the base of the PR and between 6831f2d and e470b4d.

📒 Files selected for processing (1)
  • src/components/filters/media-type-filter/index.js

Copy link
Copy Markdown

@santipalenque santipalenque left a comment

Choose a reason for hiding this comment

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

LGTM

@fntechgit fntechgit deleted a comment from coderabbitai bot Apr 9, 2026
value: filterValue,
type: "mediatypeinput",
operator: newOperatorValue.value
operator: newOperatorValue?.value
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@priscila-moneo this should be
newOperatorValue?.value ?? null

operator: newOperatorValue?.value
}
};
onChange(ev);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@priscila-moneo i dont think that we should trigger the onChange event here if operator == null

value,
type: "mediatypeinput",
operator: operatorValue.value
operator: operatorValue?.value
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copy link
Copy Markdown

@smarcet smarcet left a comment

Choose a reason for hiding this comment

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

@priscila-moneo review comments

@smarcet
Copy link
Copy Markdown

smarcet commented Apr 9, 2026

@priscila-moneo also please provide regresion unit tests

@priscila-moneo priscila-moneo force-pushed the fix/media-type-filter-type-error branch from e470b4d to 6b86039 Compare April 10, 2026 13:49
@priscila-moneo priscila-moneo requested a review from smarcet April 10, 2026 14:16
@priscila-moneo priscila-moneo force-pushed the fix/media-type-filter-type-error branch from d2403d0 to af94164 Compare April 10, 2026 14:25
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.

3 participants