Skip to content

Not filtering by excluded handle (GHE) #1848

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
1 task done
grahamj opened this issue Feb 19, 2025 · 10 comments · Fixed by #1852
Closed
1 task done

Not filtering by excluded handle (GHE) #1848

grahamj opened this issue Feb 19, 2025 · 10 comments · Fixed by #1852
Labels
bug Something isn't working

Comments

@grahamj
Copy link

grahamj commented Feb 19, 2025

🔍 Is there already an issue for your problem?

  • I have checked older issues, open and closed

📝 Description

I have a handle exclusion specified in filters yet am still receiving notifications for PRs from that user.

PS thanks for Gitify, love it!

🪜 Steps To Reproduce

  1. Using GitHub Enterprise account
  2. Ensure detailed notifications is on
  3. Add a handle to exclude
  4. Create a PR or other notification with that user
  5. Notification appears in list

🪵 Log Excerpts

No response

Gitify Version

6.1.0

Operating System

macOS

GitHub Account

GitHub Enterprise Server

📸 Screenshots

Image Image Image
@grahamj grahamj added the bug Something isn't working label Feb 19, 2025
@setchy
Copy link
Member

setchy commented Feb 20, 2025

@grahamj - in your settings is detailed notifications enabled or disabled?

@grahamj
Copy link
Author

grahamj commented Feb 20, 2025

@grahamj - in your settings is detailed notifications enabled or disabled?

I do have that enabled.

@setchy
Copy link
Member

setchy commented Feb 20, 2025

Thanks @grahamj - I missed you had mentioned this in your original issue - apologies.

I haven't been able to reproduce the above on my side this morning with GitHub Cloud notifications.

If you are able to run gitify from source and debug the following that would be helpful. From the screenshots you provided, notification.subject.user.login should be bender

if (hasExcludeHandleFilters(settings)) {
return !settings.filterExcludeHandles.some((handle) =>
filterNotificationByHandle(notification, handle),
);
}

@grahamj
Copy link
Author

grahamj commented Feb 20, 2025

ok maybe it's a GHE thing. I'll give it a whirl and get back.

@grahamj
Copy link
Author

grahamj commented Feb 20, 2025

Actually is there a way to pop open the developer tools from the app? I could probably figure out what’s going on just by snooping the request.

nvm, I found option-cmd-i

@grahamj
Copy link
Author

grahamj commented Feb 20, 2025

Alright I see the issue: you're looking for subject?.user?.login in the notification response:

return notification.subject?.user?.login === handleName;

but in my response the subject doesn't have a user property at all:

[
  {
    "id": "REDACTED",
    "unread": true,
    "reason": "review_requested",
    "updated_at": "2025-02-20T11:06:14Z",
    "last_read_at": "2025-02-19T21:29:37Z",
    "subject": {
      "title": "Lock file maintenance",
      "url": "REDACTED",
      "latest_comment_url": null,
      "type": "PullRequest"
    },
   ...

Unfortunately the handle doesn't appear anywhere else in that payload either. However it looks like you're requesting information about each PR too and the handle does appear there. If you're able to filter on user.login in the PR response that would do the trick.

@setchy
Copy link
Member

setchy commented Feb 20, 2025

When the detailed notifications flag is enabled, we do a bunch of enrichment to the raw response, including adding in the user details along with other data points back into the subject object.

export async function getGitifySubjectDetails(
notification: Notification,
): Promise<GitifySubject> {
try {
switch (notification.subject.type) {
case 'CheckSuite':
return getGitifySubjectForCheckSuite(notification);
case 'Commit':
return getGitifySubjectForCommit(notification);
case 'Discussion':
return await getGitifySubjectForDiscussion(notification);
case 'Issue':
return await getGitifySubjectForIssue(notification);
case 'PullRequest':
return await getGitifySubjectForPullRequest(notification);
case 'Release':
return await getGitifySubjectForRelease(notification);
case 'WorkflowRun':
return getGitifySubjectForWorkflowRun(notification);
default:
return null;
}
} catch (err) {
logError(
'getGitifySubjectDetails',
'failed to fetch details for notification for',
err,
notification,
);
}
}

@grahamj
Copy link
Author

grahamj commented Feb 20, 2025

ah ok, if you're merging user from the PR request into subject prior to filtering then I would expect this to work.

oh hold on, doesn't this being first mean if I have user type filters the excludes will not be evaluated?

if (hasUserTypeFilters(settings)) {

@grahamj
Copy link
Author

grahamj commented Feb 20, 2025

yeah that's it, if I turn off the user type filter it works

I think the logic there needs to be reworked - you can return immediately if the handle is excluded but for the other things you only want to return if there's not a match so they're effectively ANDed. I'd probably just have a "pass" flag and return it at the end.

@grahamj
Copy link
Author

grahamj commented Feb 25, 2025

Thanks very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants