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

Notifications when Opened and Closed are different #4849

Open
kohlrak opened this issue Mar 27, 2024 · 9 comments
Open

Notifications when Opened and Closed are different #4849

kohlrak opened this issue Mar 27, 2024 · 9 comments
Labels
Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors.

Comments

@kohlrak
Copy link

kohlrak commented Mar 27, 2024

  • Node version: v21.7.1
  • Browser version: Chrome 123 on Android 10
  • Device, operating system: Android 10, Server is running Arch, btw
  • The Lounge version: v4.4.1

Pretty easy to reproduce. Enable notifications for all messages. Have another client send a message in a room you're in and it works. Swipe the app away, and it breaks, unless they mention your name. I am unaware if this applies to custom hilights or not, but i imagine it does. Attempts to prevent this by making the app never close doesn't work: Android (and like iOS) will background it, anyway. It seems that despite the settings existing server-side, the server doesn't take them into consideration when sending notifications.


@kohlrak kohlrak added the Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors. label Mar 27, 2024
@brunnre8
Copy link
Member

At the moment we differentiate between notifications (client side browser notification popup) vs push messages (the Android firebase messaging and so forth) that also triggers when TL is in the background.

You are correct, notifyall just controls the client side notifications and not push.

I'm however unsure if using them for push is really desirable, pretty sure you'd hit rate limits there for busy chans.

Also need to make sure that when TL is open you'd not get doubled notifications

@brunnre8
Copy link
Member

Looks like rate limit should not be a problem, or at least it should be easy to detect if the server complies to the spec.

So that part is moot then

@kohlrak
Copy link
Author

kohlrak commented Mar 27, 2024

At the moment we differentiate between notifications (client side browser notification popup) vs push messages (the Android firebase messaging and so forth) that also triggers when TL is in the background.

You are correct, notifyall just controls the client side notifications and not push.

I'm however unsure if using them for push is really desirable, pretty sure you'd hit rate limits there for busy chans.

Also need to make sure that when TL is open you'd not get doubled notifications

As with all things, it depends on use case. I'm switching over from an older setup where I had a bot sitting in the channel to both log the chat and send notifications to subscribed users. In my use case, we never hit the rate limit. For something like a #support channel or a small room for friends, it's certainly desirable.

When googling, i found a reddit post that suggests that for PWA on Android devices it's throttled (that is, delayed not denied) after 5000 per hour (not sure if that's true), but no one knows what it is for Apple devices.

Are notifications sent via the TheLounge host or is it global for all TheLounge by having a central notifications server? If it's per host limits, perhaps we could change this to a feature request with the ability for the user or host to set what channels this could be applied to, and specifically when the server is in private mode.

@brunnre8
Copy link
Member

one TL server cred is used for everything.
Let's try with the naive approach of just doing it and then see what breaks.

Care to send a patch?

@kohlrak
Copy link
Author

kohlrak commented Mar 27, 2024

one TL server cred is used for everything. Let's try with the naive approach of just doing it and then see what breaks.

Care to send a patch?

I'm afraid i'm only an end user (i mean, i run my own host for TheLounge). I tried making a plugin for other things, but I can't figure out how to get them to load. I don't know JS real well, but i've done alot of C and PHP. If you could point me in the right direction (at least a hello world plugin,and some idea how to hook into options to add something, how to save the details in the user file, and maybe some idea how I could get a server-side function to call my plugin whenever a message is sent in any joined channel) I could probably code the rest myself, including configurable rate limits and the like and either github the plugin or paste it here.

@brunnre8
Copy link
Member

I didn't mean as a plugin, just fix our notification code in core. It's a bug / undesired MOA so let's fix it.

@kohlrak
Copy link
Author

kohlrak commented Mar 27, 2024

I didn't mean as a plugin, just fix our notification code in core. It's a bug / undesired MOA so let's fix it.

Ok, where do i look? And given i'm using the arch package, how do i recompile, or do i have to get the GIT version and all that?

@kohlrak
Copy link
Author

kohlrak commented Apr 10, 2024

I'm providing the following workaround because i don't have the time right now to be going unguided through someone else's project in a language i don't use when i'm in the middle of a server migration that requires installation and configuration of entirely new software to replace the old software:

git clone https://git.kohlrak.net/repos/kohlrak/YAIRCL.git; git clone https://git.kohlrak.net/repos/kohlrak/YAETCPL.git; cd YAIRCL; cat readme.txt

@kohlrak kohlrak closed this as completed Apr 10, 2024
@brunnre8 brunnre8 reopened this Apr 10, 2024
@brunnre8
Copy link
Member

it's still a TL bug, keep this open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors.
Projects
None yet
Development

No branches or pull requests

2 participants