Skip to content

Maintenance: Extract parseFilterParam shared helper from tags and statuses modules#34436

Merged
valentinpalkovic merged 1 commit into
storybookjs:nextfrom
mixelburg:refactor/dedup-url-param-parsing-v2
Apr 2, 2026
Merged

Maintenance: Extract parseFilterParam shared helper from tags and statuses modules#34436
valentinpalkovic merged 1 commit into
storybookjs:nextfrom
mixelburg:refactor/dedup-url-param-parsing-v2

Conversation

@mixelburg
Copy link
Copy Markdown

@mixelburg mixelburg commented Apr 1, 2026

Closes #34426

What I did

parseTagsParam (tags.ts) and parseStatusesParam (statuses.ts) had identical structure for parsing semicolon-separated URL filter parameters with !-negation support — ~25-30 lines each, ~55 lines total of near-duplicate logic.

Extracted a generic parseFilterParam<T>(param, transform) helper to code/core/src/manager-api/lib/filter-param.ts:

  • Handles the ; splitting, empty-string guard, and ! prefix detection
  • Calls transform(rawValue) to convert the string to the target type
  • Returns null/undefined from transform to silently skip unknown values (used by statuses)

Both parseTagsParam and parseStatusesParam are reduced to one-liners that pass their respective transforms. Public API is unchanged.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests (existing tests for parseTagsParam and parseStatusesParam cover the shared logic)
  • integration tests
  • end-to-end tests

Manual testing

No manual testing required — this is a pure refactor with no behavior changes. Existing unit tests verify correctness.

Summary by CodeRabbit

  • Refactor
    • Centralized filter parameter parsing logic into a shared utility function. Status and tag filtering operations now leverage this unified helper to improve code maintainability and reduce duplication. No changes to filtering behavior or user-facing functionality.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 1, 2026

📝 Walkthrough

Walkthrough

A new generic utility function parseFilterParam<T> is extracted to centralize filter parameter parsing logic. Two existing functions, parseStatusesParam and parseTagsParam, are refactored to delegate their parsing logic to this shared utility, eliminating code duplication while preserving their public signatures and behavior.

Changes

Cohort / File(s) Summary
New Filter Parsing Utility
code/core/src/manager-api/lib/filter-param.ts
Added generic parseFilterParam<T> function that parses semicolon-separated filter strings, handles exclusion prefixes (!), applies a transform callback, and returns objects with included and excluded arrays.
Refactored Filter Consumers
code/core/src/manager-api/modules/statuses.ts, code/core/src/manager-api/modules/tags.ts
Updated parseStatusesParam and parseTagsParam to delegate parsing logic to the new parseFilterParam utility, removing inline splitting and filtering logic while preserving function signatures and behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes


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
Contributor

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

🧹 Nitpick comments (1)
code/core/src/manager-api/lib/filter-param.ts (1)

24-26: Skip empty token after removing ! to avoid empty filter values.

At Line 25, a token of just ! becomes '' and is passed to transform; for tags this can currently add an empty tag. Consider guarding the stripped token before transform.

♻️ Suggested hardening
   param.split(';').forEach((raw) => {
     if (!raw) {
       return;
     }

     const isExcluded = raw.startsWith('!');
-    const value = transform(isExcluded ? raw.slice(1) : raw);
+    const token = isExcluded ? raw.slice(1) : raw;
+    if (!token) {
+      return;
+    }
+    const value = transform(token);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@code/core/src/manager-api/lib/filter-param.ts` around lines 24 - 26, The code
currently marks tokens starting with '!' via isExcluded and then calls transform
on raw.slice(1), but if raw === '!' this yields an empty string which can create
empty filter values; update the logic in the filter-token handling (use
isExcluded, raw and transform) to detect and skip empty stripped tokens before
calling transform (e.g., only compute value = transform(raw.slice(1)) when
raw.slice(1).length > 0, otherwise treat as an empty/ignored token or return
early) so no empty tag/value is produced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@code/core/src/manager-api/lib/filter-param.ts`:
- Around line 24-26: The code currently marks tokens starting with '!' via
isExcluded and then calls transform on raw.slice(1), but if raw === '!' this
yields an empty string which can create empty filter values; update the logic in
the filter-token handling (use isExcluded, raw and transform) to detect and skip
empty stripped tokens before calling transform (e.g., only compute value =
transform(raw.slice(1)) when raw.slice(1).length > 0, otherwise treat as an
empty/ignored token or return early) so no empty tag/value is produced.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3bebfc5b-a9c6-4aaa-b24d-d68b401c5006

📥 Commits

Reviewing files that changed from the base of the PR and between 95aa114 and 99707c2.

📒 Files selected for processing (3)
  • code/core/src/manager-api/lib/filter-param.ts
  • code/core/src/manager-api/modules/statuses.ts
  • code/core/src/manager-api/modules/tags.ts

@valentinpalkovic valentinpalkovic self-assigned this Apr 2, 2026
@valentinpalkovic valentinpalkovic added maintenance User-facing maintenance tasks ci:normal labels Apr 2, 2026
@valentinpalkovic valentinpalkovic moved this to In Progress in Core Team Projects Apr 2, 2026
@valentinpalkovic valentinpalkovic changed the title Maintenance: extract parseFilterParam shared helper from tags and statuses modules Maintenance: Extract parseFilterParam shared helper from tags and statuses modules Apr 2, 2026
@valentinpalkovic valentinpalkovic merged commit a43cc33 into storybookjs:next Apr 2, 2026
123 of 130 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Core Team Projects Apr 2, 2026
@github-actions github-actions Bot mentioned this pull request Apr 2, 2026
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:normal maintenance User-facing maintenance tasks

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Duplicate Code: URL Param Parsing Pattern Duplicated Across Tags and Statuses Modules

2 participants