openwisp / openwisp-notifications

Notifications module of OpenWISP
https://openwisp.io/docs/stable/notifications/
GNU General Public License v3.0
41 stars 42 forks source link

[fix] Prevent sending multiple identical notifications #277 #278

Closed kkreitmair closed 5 months ago

kkreitmair commented 5 months ago

If a user is in several organizations, they will receive multiple identical notifications. The reason for this is that the database query, which determines which user should receive the notification, does a LEFT OUTER JOIN with the "openwisp_users_organizationuser" table. In this table, a user is multiple times listed, when he is in several organizations. Therefore the query returns such a user multiple times, which will then receive multiple identical notifications. This commit fixes this bug by adding a distinct to all user queries.

Fixes #277

kkreitmair commented 5 months ago

@nemesifier I updated the issue comment by a description how to replicate the issue. The test will follow soon.

kkreitmair commented 5 months ago

@nemesifier I added the test.

In my patch, I naively added also a .distinct() to the QuerySet, which is injected through a parameter of the function. This can be a convenient functionality, but could be also seen as a bad practice, because normally the caller of the function should care about this. So the question is if this change should stay or should be removed. What is your opinion on this?