Skip to content
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

Fix unnecessary notification API reloads for all users #596

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

pu-raihan
Copy link
Contributor

@pu-raihan pu-raihan commented Feb 19, 2025

This PR fixes the issue #594 where every user received a WebSocket event for all CRM notifications, causing unnecessary API reloads. Now, the event is emitted only for the intended recipient, reducing server load and improving efficiency.

Changes Made:

Updated frappe.publish_realtime("crm_notification") to send the event only to the relevant user.

Before (Inefficient, Broadcasts to All Users)

frappe.publish_realtime("crm_notification")

After (Optimized, Sends Only to Intended User)

frappe.publish_realtime("crm_notification", user=self.to_user)

- Emit WebSocket event only to the intended recipient instead of broadcasting
- Improves performance by reducing unnecessary API calls and WebSocket traffic
@pu-raihan
Copy link
Contributor Author

@shariquerik In a large-scale production environment with over 15 active users, assigning multiple users at once becomes highly challenging. Each assignment triggers a notification call for every user, leading to a significant number of API requests per user. As a result, the total server load multiplies based on the number of assignments and users, potentially causing performance issues.

This has actually happened with our client. So please fix ASAP

@shariquerik shariquerik merged commit 2a8cc03 into frappe:develop Feb 20, 2025
1 check passed
@shariquerik
Copy link
Member

🎉 This PR is included in version 1.35.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants