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.

Sign up for GitHub

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

[IMPROVE] Don't send emails to online users and remove delay when away/idle #17907

Merged
merged 3 commits into from
Jun 15, 2020
Merged

[IMPROVE] Don't send emails to online users and remove delay when away/idle #17907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8cebe61
Do not send email if user online
sampaiodiego Jun 13, 2020
cf72790
Merge branch 'develop' into improve/dont-send-email-if-online
sampaiodiego Jun 13, 2020
94d7adf
Remove delay when away
sampaiodiego Jun 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions app/lib/server/functions/notifications/email.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ export function sendEmail({ message, user, subscription, room, emailAddress, has

export function shouldNotifyEmail({
disableAllMessageNotifications,
statusConnection,
emailNotifications,
isHighlighted,
hasMentionToUser,
Expand All @@ -183,6 +184,11 @@ export function shouldNotifyEmail({
return false;
}

// user connected (don't need to send him an email)
if (statusConnection === 'online') {
return false;
}

// no user or room preference
if (emailNotifications == null) {
if (disableAllMessageNotifications && !isHighlighted && !hasMentionToUser && !hasReplyToThread) {
Expand Down
1 change: 1 addition & 0 deletions app/lib/server/lib/sendNotificationsOnMessage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ export const sendNotification = async ({

if (queueItems.length) {
Notification.scheduleItem({
user: receiver,
uid: subscription.u._id,
rid: room._id,
mid: message._id,
Expand Down
52 changes: 19 additions & 33 deletions app/notification-queue/server/NotificationQueue.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ import { INotification, INotificationItemPush, INotificationItemEmail, Notificat
import { NotificationQueue, Users } from '../../models/server/raw';
import { sendEmailFromData } from '../../lib/server/functions/notifications/email';
import { PushNotification } from '../../push-notifications/server';
import { IUser } from '../../../definition/IUser';

const {
NOTIFICATIONS_WORKER_TIMEOUT = 2000,
NOTIFICATIONS_BATCH_SIZE = 100,
NOTIFICATIONS_SCHEDULE_DELAY_ONLINE = -1,
NOTIFICATIONS_SCHEDULE_DELAY_AWAY = 120,
NOTIFICATIONS_SCHEDULE_DELAY_ONLINE = 120,
NOTIFICATIONS_SCHEDULE_DELAY_AWAY = 0,
NOTIFICATIONS_SCHEDULE_DELAY_OFFLINE = 0,
} = process.env;

Expand All @@ -20,11 +21,11 @@ class NotificationClass {

private maxBatchSize = Number(NOTIFICATIONS_BATCH_SIZE);

private maxScheduleDelaySecondsOnline = Number(NOTIFICATIONS_SCHEDULE_DELAY_ONLINE);

private maxScheduleDelaySecondsAway = Number(NOTIFICATIONS_SCHEDULE_DELAY_AWAY);

private maxScheduleDelaySecondsOffline = Number(NOTIFICATIONS_SCHEDULE_DELAY_OFFLINE);
private maxScheduleDelaySeconds: {[key: string]: number} = {
online: Number(NOTIFICATIONS_SCHEDULE_DELAY_ONLINE),
away: Number(NOTIFICATIONS_SCHEDULE_DELAY_AWAY),
offline: Number(NOTIFICATIONS_SCHEDULE_DELAY_OFFLINE),
};

initWorker(): void {
this.running = true;
Expand Down Expand Up @@ -103,44 +104,29 @@ class NotificationClass {
sendEmailFromData(item.data);
}

async scheduleItem({ uid, rid, mid, items }: {uid: string; rid: string; mid: string; items: NotificationItem[]}): Promise<void> {
const user = await Users.findOneById(uid, {
async scheduleItem({ uid, rid, mid, items, user }: { uid: string; rid: string; mid: string; items: NotificationItem[]; user?: Partial<IUser> }): Promise<void> {
const receiver = user || await Users.findOneById(uid, {
projection: {
statusConnection: 1,
_updatedAt: 1,
},
});

if (!user) {
if (!receiver) {
return;
}

let schedule: Date | undefined;

if (user.statusConnection === 'online') {
if (this.maxScheduleDelaySecondsOnline === -1) {
return;
}
const { statusConnection } = receiver;

schedule = new Date();
schedule.setSeconds(schedule.getSeconds() + this.maxScheduleDelaySecondsOnline);
} else if (user.statusConnection === 'away') {
if (this.maxScheduleDelaySecondsAway === -1) {
return;
}
let schedule: Date | undefined;

const elapsedSeconds = Math.floor((Date.now() - user._updatedAt) / 1000);
if (elapsedSeconds < this.maxScheduleDelaySecondsAway) {
schedule = new Date();
schedule.setSeconds(schedule.getSeconds() + this.maxScheduleDelaySecondsAway - elapsedSeconds);
}
} else if (user.statusConnection === 'offline') {
if (this.maxScheduleDelaySecondsOffline === -1) {
return;
}
const delay = this.maxScheduleDelaySeconds[statusConnection];

if (delay < 0) {
return;
}
if (delay > 0) {
schedule = new Date();
schedule.setSeconds(schedule.getSeconds() + this.maxScheduleDelaySecondsOffline);
schedule.setSeconds(schedule.getSeconds() + delay);
}

await NotificationQueue.insertOne({
Expand Down