Simplify and remove the notification "cache" #4508
Conversation
claucambra
force-pushed
the
bugfix/notifications
branch
from
b03abf0 to
ab4b37c
Compare
claucambra
force-pushed
the
bugfix/notifications
branch
2 times, most recently
from
53348ff to
1f26247
Compare
claucambra
changed the title
src/gui/tray/usermodel.cpp
Outdated
| @@ -505,7 +501,8 @@ void User::slotAddErrorToGui(const QString &folderAlias, SyncFileItem::Status st | |||
| // add 'other errors' to activity list | |||
| _activityModel->addErrorToActivityList(activity); | |||
|
|
|||
| showDesktopNotification(activity._subject, activity._message); | |||
| // Error notifications don't have ids by themselves so we will create one for it | |||
| showDesktopNotification(activity._subject, activity._message, qHash(activity._subject + activity._message)); | |||
There was a problem hiding this comment.
I am wondering if collisions could be possible
there is also the option of asking the server teams to add the missing ids
There was a problem hiding this comment.
Since errors in this case are created locally I don't think this would be possible
What I have done is instead assign errors with negative ids since activity ids are unsigned ints and therefore always positive. No possibility of collision this way. We also no longer randomly generate error activity ids, but instead create them as increasingly negative numbers
claucambra
force-pushed
the
bugfix/notifications
branch
from
1f26247 to
84e28bf
Compare
claucambra
force-pushed
the
bugfix/notifications
branch
from
84e28bf to
c4e32f1
Compare
claucambra
force-pushed
the
bugfix/notifications
branch
2 times, most recently
from
de8e2de to
0988100
Compare
Signed-off-by: Claudio Cambra <claudio.cambra@gmail.com>
claucambra
force-pushed
the
bugfix/notifications
branch
from
0988100 to
b6def51
Compare
|
SonarCloud Quality Gate failed. |
|
AppImage file: Nextcloud-PR-4508-b6def519457239bff6d5dcfcf7ccf7d99d2771d0-x86_64.AppImage |
|
/backport to stable-3.5 |
This PR removes removes the notification cache entirely and replaces it with a QSet in UserModel.
The notification "cache"'s only purpose was to keep track of notifications which had already been notified, and it kept track of these by storing randomly-generated hashes composed of the notification text in an internal QSet. The new QSet serves the same functionality as the notification "cache" but instead stores notifications' actual ids, letting us actually filter out notifications which are the same without unintended side-effects. It also obviates the need for custom setters/getters/clearers and therefore the entire notification cache can be removed altogether. Since we didn't actually cache any notifications, we can simplify the code this way.