8000 perf: Avoid querying all users in team for common notification types by tommoor · Pull Request #8806 · outline/outline · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
8000

perf: Avoid querying all users in team for common notification types #8806

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 26, 2025
Merged

perf: Avoid querying all users in team for common notification types #8806

merged 1 commit into from
Mar 26, 2025

Conversation

tommoor
Copy link
Member
@tommoor tommoor commented Mar 26, 2025

Noticed while working on #8734 that this is querying all users in the team unnecessarily when onlySubscribers: true

@codegen-sh codegen-sh
Copy link
Contributor
codegen-sh bot commented Mar 26, 2025

I've reviewed this PR which optimizes the notification system by avoiding unnecessary database queries when onlySubscribers: true. Here's my analysis:

What the PR does

This PR optimizes the getDocumentNotificationRecipients method in NotificationHelper.ts by changing the query approach:

Before:

  1. Query all users in the team who aren't the actor
  2. Filter users by whether they're subscribed to the notification type
  3. If onlySubscribers is true, query subscriptions and further filter users

After:

  1. If onlySubscribers is true, directly query subscriptions with the user association
  2. Otherwise, query all users in the team who aren't the actor
  3. Filter users by whether they're subscribed to the notification type

Benefits

  • Avoids querying all users in a team when we only need subscribers
  • More efficient database access pattern, especially for teams with many users
  • Includes the user association in the subscription query, avoiding additional queries

Considerations

The change maintains the same functionality while being more efficient. The filtering by subscribedToEventType is still applied to ensure users have notifications enabled for the specific event type.

This is a solid performance improvement that should reduce database load, especially for larger teams. The code is also cleaner and more straightforward.

👍 LGTM

@tommoor tommoor merged commit ba7d102 into main Mar 26, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
performance
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@tommoor
0