Repository navigation
fix: stop star notifications from people you mute - #797
sulimanbenhalim wants to merge 2 commits into
Conversation
…fication-respects-mutes # Conflicts: # app/Actions/CreateStarNotification.php
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Filter muted recipients app/Actions/CreateStarNotification.php, tests/Unit/Actions/CreateStarNotificationTest.php |
The recipient query excludes users who muted the stargazer. Tests check that a muted commit author receives no notification and that only the commit member receives a notification in the organization scenario. |
Estimated code review effort: 2 (Simple) | ~8 minutes
Change: Bug fix
Possibly related issues
dev-null-sh/dev-null.sh#740— The issue concerns mute filtering for starred notifications, though it specifies muted-thread filtering rather than muted stargazers.
Possibly related PRs
- dev-null-sh/dev-null.sh#1361 — Adds organization-authored commits and changes organization handling in the same star-notification flow.
- dev-null-sh/dev-null.sh#1368 — Introduces profile-based recipients for stars on organization commits, which this change filters by mute status.
- dev-null-sh/dev-null.sh#1348 — Changes commit authorship to profile records used by the star-notification recipient query.
Suggested reviewers: mnapoli, xheaven
Merge Risk: 
bfc2a
-
The notification filtering change is ready to merge.
Security architecture risk: bfc2a
-
The change prevents unwanted notifications without expanding access or weakening existing controls. No material security risk was found in the affected behavior.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- inferred — The changed outcome is limited to notification rows for the starred commit's personal author or organization members. The predicate narrows that recipient set; it does not introduce another recipient source or greater authority.
Trust Boundaries and Controls
- observed — The inspected web and API controllers pass the current user as the star actor. Their request authorization denies stars on owned commits and requires commit visibility. These callers and controls are unchanged by the PR; the public PHP method does not establish a newly exposed endpoint.
Resilience and Maintainability Implications
- inferred — Recipient selection and insertion remain separate statements, so a mute can interleave between them. Existing query-time filtering still hides notifications from muted actors. A stored notification reappearing after unmute is pre-existing behavior, not a security regression introduced by this predicate.
-
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: preventing star notifications from users who muted the recipient. Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files.
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
muting someone is supposed to stop their notifications but a star from them still created one
CreateStarNotificationwas the only notification from another user that skipped the mute checkthe row stayed hidden while muted and even marking everything read left it unread so it popped up as soon as you unmuted them
now it skips authors who mute the stargazer the same way
CreateFollowNotificationdoesadded a test that fails without the change and the full suite passes at 100% coverage
Summary by CodeRabbit