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

Change Push Notification logic #1602

Closed
Morgul opened this issue Dec 8, 2015 · 8 comments · Fixed by #7641
Closed

Change Push Notification logic #1602

Morgul opened this issue Dec 8, 2015 · 8 comments · Fixed by #7641

Comments

@Morgul
Copy link
Contributor

Morgul commented Dec 8, 2015

Currently, the only way you will ever get a push notification is if your connection is considered "away". (This is different from your status. Basically, it's controlled by how long it's been since you last interacted with anything.)

My suggestion is to make the logic instead work like this:

  • If a message that would generate a notification (DM, mention, etc.) has not been read in X minutes, send a push notification.
  • Add an option for how many minutes (2 feels like a good default, minimum should be 0, aka "immediately")
  • Add an option to disable sending push notification at all.

We may also want an option for the current behavior: "Do not send a notification if I have been active in the last X minutes."

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@himojutsu
Copy link

Until this feature gets added, I found a workaround to get notifications regardless of your status.

Change the following lines in Rocket.Chat/programs/server/packages/rocketchat_lib.js
1373 if (Push.enabled === true && userOfMention.statusConnection !== 'online') {
to
1373 if (Push.enabled === true) {

1490 if (user.statusConnection !== 'online' && (ref4 = user._id, indexOf.call(settings.dontNotifyMobileUsers, ref4) < 0)) {
to
1490 if ((ref4 = user._id, indexOf.call(settings.dontNotifyMobileUsers, ref4) < 0)) {

The line numbers may change by version. Those line numbers are from 0.34.0.

@rasos
Copy link
Contributor

rasos commented Jun 29, 2016

Thank you, we have also applied this patch.
Our users were confused, when they get notifications in groups. The current logic is "too intelligent".
Ideally we have an option for getting push notifications, even when online with another device.
The drawback of this patch is, that you get even notifications of your own messages.

@rasos
Copy link
Contributor

rasos commented Nov 8, 2016

Great, I assume this should make the patch obsolete, which some have applied: #1602 (comment)?
Our users want to be able to get notifications always on the mobile device, even if also notified on the desktop.

@engelgabriel
Copy link
Member

We need to turn this into a setting

@p-try
Copy link

p-try commented Jan 8, 2017

Thank you very much for this patch. Here are the updated line numbers for version 0.48.2:
1611 and 1732.

I agree that this should be configurable. In our case users will frequently leave their desk to visit clients without logging out and they will want to get push notifications anyway.

@rasos
Copy link
Contributor

rasos commented Jul 22, 2017

Push patch for RC 0.56 in /var/lib/rocket.chat/bundle/programs/server/packages/rocketchat_lib.js line 1821:

   if (userOfMention != null && canBeNotified(userOfMentionId, 'desktop')) {                                            // 149
//                      if (Push.enabled === true && userOfMention.statusConnection !== 'online') {                                         // 150
// Push patch - pushes always ucom issue #115

line 2000:

userIdsToPushNotify = _.pluck(_.filter(usersOfMobileMentions, function (user) {                                     // 291
//              return user.statusConnection !== 'online';                                                                         // 292
// always send push
return true;
            }), '_id');                                                                                                         // 293
        }

@rasos
Copy link
Contributor

rasos commented Sep 27, 2017

We should have a setting for
UserPresence.awayTime = 300000;
which is a Meteor.startup(function() in client/startup/startup.js
and reduce it to 10 seconds as default = 10000;

rasos added a commit to rasos/Rocket.Chat that referenced this issue Sep 27, 2017
Users are not getting push notifications, when leaving desk within 5 minutes partially resolves RocketChat#1602
@kholschevnikov
Copy link

@rodrigok In the end, you just decided not to implement this improvement?
Because it is very uncomfortable to receive push on the phone when you have read the messages on the desktop.
I looked for the described functionality about "If a message .. has not been read in X minutes, send a push notification.", but I could not find it, although the problem is closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment