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

Don't re-show repeated notifications #1860

Merged
merged 3 commits into from
Oct 2, 2024

Conversation

ferdnyc
Copy link
Member

@ferdnyc ferdnyc commented Sep 11, 2024

GSConnect receives a lot of duplicate notifications, whether due to reconnects, proactively requesting updates, etc.

Since GNOME 46, each of those repeats has been popped up as a new banner notification every time it's received, even if nothing has changed in the notification content. This is extremely disruptive.

To avoid that, don't blindly flag all repeated notifications as "unacknowledged", which will cause them to be popped up again. Only do so if the content has changed. (This covers re-displaying banners for SMS conversations that receive an additional message, since those are sent as updates to the existing notification.)

Fixes #1855

GSConnect receives a lot of duplicate notifications, whether due to
reconnects, proactively requesting updates, etc.

Since GNOME 46, each of those repeats has been popped up as a new
banner notification every time it's received, even if nothing has
changed in the notification content. This is extremely disruptive.

To avoid that, don't blindly flag all repeated notifications as
"unacknowledged", which will cause them to be popped up again.
Only do so if the content has changed. (This covers re-displaying
banners for SMS conversations that receive an additional message,
since those are sent as updates to the existing notification.)

Fixes GSConnect#1855
@andyholmes
Copy link
Collaborator

This seems reasonable, but I'm not familiar with the implementation anymore. If it works in practice, I'd say it's fine 👍

@ferdnyc
Copy link
Member Author

ferdnyc commented Sep 15, 2024

If it works in practice

Yeah, that's the $64,000 question. I'm hoping it'll score at least one or two testers from #1855 to confirm.

Most I can say is, "it works for me"... but I don't feel like that represents nearly a wide enough range of scenarios. The apps I have notifications enabled for number fewer than a dozen — just enough for their constant re-displaying to annoy me into fixing it.

My limited local testing did include a Google Messages notification, which no longer endlessly repeated, but still popped up anew when a new SMS came in that was added to that conversation. But "saw it work exactly once" feels more like a reason not to halt further testing, than to declare unqualified success.

@ferdnyc
Copy link
Member Author

ferdnyc commented Sep 25, 2024

Reports from @michas79de after a few hours of testing match my own results:

the same notifications that popped up repeatedly before are not popping up again.
An SMS notification only appeared once on arrival and a second time hours later when a second SMS arrived from the same sender and the notification was updated/replaced.
So it seems to be working fine.

It sounds like this is good to go, it at least doesn't make things appreciably worse and appears to solve the notification-repetition issue.

@ferdnyc ferdnyc added this to the v58 milestone Sep 25, 2024
@ferdnyc
Copy link
Member Author

ferdnyc commented Sep 30, 2024

Just need an approval to be able to merge this one; reviewers?

@swsnr
Copy link

swsnr commented Sep 30, 2024

I'd like to report that I'm running gsconnect with this patch since a few days and haven't had any issues with notifications.

@andyholmes
Copy link
Collaborator

I'll approve this, given there's two confirmations.

@andyholmes andyholmes merged commit 0a59c4d into GSConnect:main Oct 2, 2024
3 checks passed
@BloodyIron
Copy link

How soon before we can get a new release with this baked in? :) Sorry I haven't been able to chip in on testing this. :(

@ferdnyc ferdnyc deleted the notification-repeats branch October 6, 2024 13:53
@ferdnyc
Copy link
Member Author

ferdnyc commented Oct 6, 2024

@BloodyIron

That's a question primarily for @daniellandau. Whose last activity in the GSConnect repo, I'm now seeing, was back in April (!). So, that's...not ideal.

@BloodyIron
Copy link

Only one person can push a release? Oof

@daniellandau
Copy link
Member

e.g.o only allows a single access, but others can do a release here too. I'll try to get around to it soon too if no one else does before me.

@ferdnyc
Copy link
Member Author

ferdnyc commented Oct 9, 2024

@daniellandau Welcome!

others can do a release here too.

True enough, honestly the only reason I haven't put together a release PR yet is that #1866 is still unmerged, and there's absolutely no point in doing a release until that goes in.

(I question whether there's any point to doing a release at all without it getting submitted to e.g.o. But I'm sure having the release already tagged and packaged makes the submission process easier for you, so happy to help out there.)

I'll check the v58 milestone I slapped together last week (we do milestones now, #SOORGANIZED!) and if there's nothing other than #1866 outstanding, I'll get started on a PR for the v58 release.

@mavit
Copy link
Contributor

mavit commented Oct 30, 2024

I question whether there's any point to doing a release at all without it getting submitted to e.g.o.

Here's one reason. Fedora provides this extension as an RPM package. The maintainer gets notified to update that package when a release happens here on GitHub, not when it hits extensions.gnome.org.

(On this occasion, that happened too late for the update to be included in the initial release of Fedora 41 (which includes Gnome 47), so it's currently not working for early adopters)

@daniellandau
Copy link
Member

But I'm sure having the release already tagged and packaged makes the submission process easier for you

This is a major reason too. Preparing a release is the bigger job, uploading a zip to e.g.o is just a tiny final part.

@ferdnyc
Copy link
Member Author

ferdnyc commented Nov 13, 2024

@mavit

Here's one reason. Fedora provides this extension as an RPM package. The maintainer gets notified to update that package when a release happens here on GitHub, not when it hits extensions.gnome.org.

(On this occasion, that happened too late for the update to be included in the initial release of Fedora 41 (which includes Gnome 47), so it's currently not working for early adopters)

Valid, though on our end that packaging is Not Recommended™ anyway. (Especially since the maintainer refuses to include the Nautilus/Nemo extensions by default, even as a weak dependency. A position that left me so speechless, I've failed to follow up on it for over 2 years already.)

But it also adds a 2-week delay to users receiving extension updates, vs. e.g.o — a delay that in normal Fedora packaging is justified as time for testing of the package before inflicting it on users, but in GSConnect's case I question whether that testing is more theoretical than actual, in reality.

Also, GSConnect does a lot of behind-the-scenes trickery with a backend daemon process and whatnot, and there's some evidence that updating in the user's home directory tends to be less fragile than updating a /usr/ install via DNF, in terms of already-running installs. (GSConnect's daemon.js running from the user space even watches for modifications to its code and automatically relaunches itself. Mostly meant as a developer feature, sure, but it helps with upgrades as well.)

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

Successfully merging this pull request may close these issues.

GSConnect notifications endlessly re-notify on GNOME 46/Ubuntu 24.04
6 participants