NotificationsYou must be signed in to change notification settings
Fork501
Star5.5k

fix: filter count badge displaying incorrect count for empty filters#3882

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Open

dhruv-suthar wants to merge3 commits intorequestly:master

base:master

Choose a base branch

fromdhruv-suthar:fix/incorrect-filter-count

Open

fix: filter count badge displaying incorrect count for empty filters#3882

dhruv-suthar wants to merge3 commits intorequestly:masterfromdhruv-suthar:fix/incorrect-filter-count

+20 −8

Conversation

Copy link

dhruv-suthar commentedNov 19, 2025•
edited by coderabbitaibot
Loading

Closes issue:#3826

📜 Summary of changes:

Fixed the bug where rules incorrectly displayed "1 applied filter" even when no filters were configured. The issue occurred when:

✅ Checklist:

Make sure linting and unit tests pass.
No install/build warnings introduced.
Verified UI in browser.
For UI changes, added/updated analytics events (if applicable). - N/A (bug fix, no new events)
For changes in extension's code, manually tested in Chrome and Firefox. - N/A (web app change)
Added/updated unit tests for this change. - Recommended but not included in this PR
Raised pull request to update corresponding documentation (if already exists). - N/A
Added demo video showing the changes in action (if applicable). - See above

🎥 Demo Video:

Video/Demo:
https://github.com/user-attachments/assets/375d55df-f994-4be0-ba42-db5abd519d85

Summary by CodeRabbit

Bug Fixes

Fixed filter count calculation logic to accurately display results across all filter configurations and edge cases.

Fix filter count badge displaying incorrect count for empty filters (r…

381c00a

…equestly#3826)

Copy link

CLAassistant commentedNov 19, 2025

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign ourContributor License Agreement before we can accept your contribution.

dhruv-suthar seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, pleaseadd the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let usrecheck it.}

Merge branch 'master' into fix/incorrect-filter-count

1fc64c5

Copy link

Contributor

coderabbitaibot commentedNov 19, 2025•
edited
Loading

Walkthrough

ThegetFilterCount function has been refactored to compute filter counts from a normalized filters object. The new implementation uses a definedCOUNTABLE_FILTERS set containing REQUEST_METHOD, RESOURCE_TYPE, and PAGE_DOMAINS, counting only those filters that are arrays with length greater than 0. Defensive checks now handle missing or invalid filter objects, and optional chaining is used for the upgraded format path. This replaces the previous key-counting approach that had special handling for page URL exclusion.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Verify theCOUNTABLE_FILTERS set captures all relevant filter types previously counted
Confirm optional chaining correctly handles missing nested fields in the upgraded format path
Validate that defensive checks adequately cover edge cases for null, undefined, or malformed filter objects
Ensure the new logic produces equivalent results to the previous key-counting approach for existing use cases

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The description includes the issue link, a summary of changes, a demo video, and a completed checklist, but the summary section is incomplete with ellipsis where details should be.	Complete the 'Summary of changes' section by explaining the specific conditions that caused the bug and detailing how the fix addresses them.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly summarizes the main bug fix: correcting an incorrect filter count display for empty filters, which matches the changeset's core objective.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for usingCodeRabbit! 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.}

coderabbitaibot reviewed

Nov 19, 2025

View reviewed changes

Copy link

Contributor

coderabbitaibot 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: 0

🧹 Nitpick comments (1)

app/src/components/features/rules/RulePairs/Pairs/Rows/RowsMarkup/RequestSourceRow/index.js (1)
83-87:Consider moving COUNTABLE_FILTERS outside the callback.
The constant is recreated on every render insideuseCallback. Since it's static, moving it outside the component (or at least outside the callback) would improve performance slightly.
Move the constant to module scope:
+const COUNTABLE_FILTERS = [+  GLOBAL_CONSTANTS.RULE_SOURCE_FILTER_TYPES.REQUEST_METHOD,+  GLOBAL_CONSTANTS.RULE_SOURCE_FILTER_TYPES.RESOURCE_TYPE,+  GLOBAL_CONSTANTS.RULE_SOURCE_FILTER_TYPES.PAGE_DOMAINS,+];+ const RequestSourceRow = ({ rowIndex, pair, pairIndex, ruleDetails, isInputDisabled }) => {
And remove lines 83-87 from inside the callback.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between215d810 and1fc64c5.

📒 Files selected for processing (1)

app/src/components/features/rules/RulePairs/Pairs/Rows/RowsMarkup/RequestSourceRow/index.js (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

app/src/components/features/rules/RulePairs/Pairs/Rows/RowsMarkup/RequestSourceRow/index.js (2)

app/src/components/features/rules/RulePairs/Filters/index.jsx (1)
currentlySelectedRuleData (65-65)
app/src/components/features/rules/RulePairs/index.js (1)
currentlySelectedRuleData (18-18)

🔇 Additional comments (2)

app/src/components/features/rules/RulePairs/Pairs/Rows/RowsMarkup/RequestSourceRow/index.js (2)
73-79:LGTM! Solid defensive checks.
The filter retrieval logic correctly handles both upgraded and legacy formats with optional chaining, and the type guard prevents errors when filters are missing or invalid. This directly addresses the bug where empty filters would display an incorrect count.
89-92:Excellent fix for the filter count bug!
The logic correctly counts only non-empty array filters by explicitly checkingArray.isArray(value) && value.length > 0. This fixes the bug where empty filters would incorrectly display "1 applied filter" by ensuring only populated filters contribute to the count.