Skip to content

[Refactor] Group security issues declaratively with groupBy - #8981

Draft
github-actions[bot] wants to merge 1 commit into
mainfrom
refactor-maintenance-39699369679
Draft

github-actions[bot] wants to merge 1 commit into
mainfrom
refactor-maintenance-39699369679

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

groupIssues in the app security engine built its result by hand: a Map<string, IssueGroup>, a branch for "group exists" vs "first time seeing this key", and mutation of the accumulator's issues and files arrays. The grouping intent was obscured by the bookkeeping, and the de-duplication of file names was an includes check buried inside the loop.

WHAT is this pull request doing?

Replace the manual Map accumulation with the existing groupBy helper from @shopify/cli-kit/common/collection, already used elsewhere in this package (services/generate.ts, services/dev/app-events/app-event-watcher.ts). Each group's severity, issues, and distinct files are now derived from the grouped entries rather than accumulated through mutation, and the file de-duplication becomes a Set.

Behavior is unchanged. The sort that establishes ordering is untouched, and group ordering is still first-appearance order: groupKey joins its parts with |, so keys are never integer-like and Object.values preserves insertion order exactly as the Map did.

How to manually test your changes?

shopify app security check

The grouped issue list, its ordering, and the per-group file lists should be identical to before.

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • The change is user-facing — I've identified the correct bump type (patch for bug fixes · minor for new features · major for breaking changes) and added a changeset with pnpm changeset add

Replace the manual Map accumulation in groupIssues with the cli-kit
groupBy helper, deriving each group's issues and distinct files from
the grouped entries instead of mutating accumulator arrays.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

0 participants