Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions src/renderer/utils/notifications/filters/filter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,72 @@ describe('renderer/utils/notifications/filters/filter.ts', () => {
expect(result).toEqual([mockNotifications[0]]);
});

it('should not exclude directly-addressed notifications by author handle (#2914)', async () => {
// A human-authored PR where a bot left the latest comment, so
// enrichment resolves `subject.user` to the bot, but the user was
// directly asked to review.
const reviewRequested = mockPartialGitifyNotification({
title: 'Please review this PR',
user: {
login: 'coderabbitai',
htmlUrl: 'https://github.com/coderabbitai' as Link,
avatarUrl: 'https://avatars.githubusercontent.com/u/1' as Link,
type: 'Bot',
},
});
reviewRequested.reason.code = 'review_requested';

// A notification that exists only because the bot commented should
// still be excluded.
const botComment = mockPartialGitifyNotification({
title: 'Bot comment noise',
user: {
login: 'coderabbitai',
htmlUrl: 'https://github.com/coderabbitai' as Link,
avatarUrl: 'https://avatars.githubusercontent.com/u/1' as Link,
type: 'Bot',
},
});
botComment.reason.code = 'subscribed';

useFiltersStore.setState({
excludeSearchTokens: ['author:coderabbitai' as SearchToken],
});

const result = filterDetailedNotifications([reviewRequested, botComment], {
...mockSettings,
detailedNotifications: true,
});

expect(result).toEqual([reviewRequested]);
});

it('should still apply author include filters for directly-addressed reasons', async () => {
const reviewRequested = mockPartialGitifyNotification({
title: 'Please review this PR',
user: {
login: 'coderabbitai',
htmlUrl: 'https://github.com/coderabbitai' as Link,
avatarUrl: 'https://avatars.githubusercontent.com/u/1' as Link,
type: 'Bot',
},
});
reviewRequested.reason.code = 'review_requested';

// The exemption applies to excludes only; an author include still
// narrows to matching notifications.
useFiltersStore.setState({
includeSearchTokens: ['author:someone-else' as SearchToken],
});

const result = filterDetailedNotifications([reviewRequested], {
...mockSettings,
detailedNotifications: true,
});

expect(result).toEqual([]);
});

it('should filter notifications by state when provided', async () => {
useFiltersStore.setState({ states: ['closed'] });

Expand Down
13 changes: 12 additions & 1 deletion src/renderer/utils/notifications/filters/filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ import type {
} from '../../../types';

import {
AUTHOR_SEARCH_PREFIX,
BASE_SEARCH_QUALIFIERS,
DETAILED_ONLY_SEARCH_QUALIFIERS,
filterNotificationBySearchTerm,
hasExcludeSearchFilters,
hasIncludeSearchFilters,
isDirectlyAddressedReason,
reasonFilter,
type SearchQualifier,
stateFilter,
Expand Down Expand Up @@ -116,7 +118,16 @@ function passesSearchTokenFiltersForQualifier(

if (hasExcludeSearchFilters()) {
const excludeTokens = filters.excludeSearchTokens.filter((t) => t.startsWith(prefix));
if (excludeTokens.length > 0) {

// An `author:` exclude must not hide notifications that directly address
// the user (e.g. review requested), since `subject.user` may be a bot that
// only left the latest comment rather than the actor who addressed them.
// `reason` is optional-chained because `isUserFilteredOut` builds synthetic
// notifications without one.
const skipExclude =
prefix === AUTHOR_SEARCH_PREFIX && isDirectlyAddressedReason(notification.reason?.code);

if (excludeTokens.length > 0 && !skipExclude) {
passes =
passes &&
!excludeTokens.some((token) => filterNotificationBySearchTerm(notification, token));
Expand Down
27 changes: 26 additions & 1 deletion src/renderer/utils/notifications/filters/search.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ import { mockPartialGitifyNotification } from '../../../__mocks__/notifications-

import type { GitifyOwner, Link } from '../../../types';

import { ALL_SEARCH_QUALIFIERS, filterNotificationBySearchTerm, parseSearchInput } from './search';
import {
ALL_SEARCH_QUALIFIERS,
filterNotificationBySearchTerm,
isDirectlyAddressedReason,
parseSearchInput,
} from './search';

// (helper removed – no longer used)

Expand Down Expand Up @@ -89,4 +94,24 @@ describe('renderer/utils/notifications/filters/search.ts', () => {
expect(filterNotificationBySearchTerm(mockNotification, '')).toBe(false);
});
});

describe('isDirectlyAddressedReason', () => {
it('returns true for reasons that ask the user to act', () => {
expect(isDirectlyAddressedReason('approval_requested')).toBe(true);
expect(isDirectlyAddressedReason('assign')).toBe(true);
expect(isDirectlyAddressedReason('mention')).toBe(true);
expect(isDirectlyAddressedReason('review_requested')).toBe(true);
expect(isDirectlyAddressedReason('team_mention')).toBe(true);
});

it('returns false for passive reasons', () => {
expect(isDirectlyAddressedReason('subscribed')).toBe(false);
expect(isDirectlyAddressedReason('comment')).toBe(false);
expect(isDirectlyAddressedReason('author')).toBe(false);
});

it('returns false for undefined reason', () => {
expect(isDirectlyAddressedReason(undefined)).toBe(false);
});
});
});
27 changes: 25 additions & 2 deletions src/renderer/utils/notifications/filters/search.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,35 @@
import { useFiltersStore } from '../../../stores';

import type { RawGitifyNotification } from '../../../types';
import type { RawGitifyNotification, Reason } from '../../../types';

export const SEARCH_DELIMITER = ':';

export const AUTHOR_SEARCH_PREFIX = 'author:';

/**
* Notification reasons where the user was directly asked to act (review,
* approval, assignment, mention). An `author:` exclude filter must not hide
* these: after enrichment a notification's `subject.user` can resolve to a bot
* that merely left the most recent comment rather than the actor who addressed
* the user, so excluding that bot would drop actionable notifications.
*
* See https://github.com/gitify-app/gitify/issues/2914.
*/
const DIRECTLY_ADDRESSED_REASONS: ReadonlySet<Reason> = new Set([
'approval_requested',
'assign',
'mention',
'review_requested',
'team_mention',
]);

export function isDirectlyAddressedReason(reason: Reason | undefined): boolean {
return reason !== undefined && DIRECTLY_ADDRESSED_REASONS.has(reason);
}

const SEARCH_QUALIFIERS = {
author: {
prefix: 'author:',
prefix: AUTHOR_SEARCH_PREFIX,
description: 'filter by notification author',
requiresDetailsNotifications: true,
extract: (n: RawGitifyNotification) => n.subject?.user?.login,
Expand Down
Loading