Fix Telegram notification spam with channel-level rate limiting (#951)
* Fix Telegram notification spam with channel-level rate limiting
Addresses issue #950 where Telegram notifications were sent excessively
(96-288 messages per day per task) due to the delete-and-recreate pattern
added in commit 105a913a to fix navbar notification pile-up.
Changes:
- Add channel_sent_at JSON field to notifications table to track when
each channel (telegram, email, push) was last sent
- Add helper methods to notification model:
- markChannelAsSent(channel): Records send timestamp
- wasChannelRecentlySent(channel, threshold): Checks if sent within 24h
- Modify sendTelegramNotification() to check rate limit before sending
- Update service layer (dueTaskService, deferredTaskService,
dueProjectService) to preserve channel_sent_at when recreating
notifications
- Add comprehensive unit and integration tests (20 tests, all passing)
Impact:
- Reduces Telegram notifications from 96-288/day to 1/day per item
- Preserves in-app notification refresh behavior (every 5-15 min)
- Maintains navbar pile-up fix from original commit
- Rate limit configurable (default: 24 hours)
Fixes #950
* Fix linting and formatting issues
* Fix integration test that was trying to access private function
* Fix prettier formatting in integration test
This commit is contained in:
parent
471d29e495
commit
11cd77bedd
8 changed files with 739 additions and 2 deletions
|
|
@ -301,5 +301,93 @@ describe('dueTaskService', () => {
|
|||
expect(result.notificationsCreated).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Telegram rate limiting', () => {
|
||||
const telegramNotificationService = require('../../../../modules/telegram/telegramNotificationService');
|
||||
|
||||
beforeEach(() => {
|
||||
// Mock Telegram service
|
||||
jest.spyOn(
|
||||
telegramNotificationService,
|
||||
'isTelegramConfigured'
|
||||
).mockReturnValue(true);
|
||||
jest.spyOn(
|
||||
telegramNotificationService,
|
||||
'sendTelegramNotification'
|
||||
).mockResolvedValue({ success: true });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
jest.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('should not resend Telegram if notification recreated within 24 hours', async () => {
|
||||
// Setup user with Telegram enabled
|
||||
user.telegram_bot_token =
|
||||
'123456789:ABCdefGHIjklMNOPQRSTUVwxyz-12345678';
|
||||
user.telegram_chat_id = '123456789';
|
||||
user.notification_preferences = {
|
||||
dueTasks: { inApp: true, telegram: true },
|
||||
overdueTasks: { inApp: true, telegram: true },
|
||||
};
|
||||
await user.save();
|
||||
|
||||
const tomorrow = new Date();
|
||||
tomorrow.setDate(tomorrow.getDate() + 1);
|
||||
|
||||
const task = await Task.create({
|
||||
name: 'Test Task',
|
||||
user_id: user.id,
|
||||
due_date: tomorrow,
|
||||
status: Task.STATUS.NOT_STARTED,
|
||||
});
|
||||
|
||||
const sendTelegramSpy = jest.spyOn(
|
||||
telegramNotificationService,
|
||||
'sendTelegramNotification'
|
||||
);
|
||||
|
||||
// First check - should send Telegram
|
||||
await checkDueTasks();
|
||||
|
||||
const firstCallCount = sendTelegramSpy.mock.calls.length;
|
||||
expect(firstCallCount).toBeGreaterThan(0);
|
||||
|
||||
// Get the created notification
|
||||
const notification = await Notification.findOne({
|
||||
where: {
|
||||
user_id: user.id,
|
||||
type: 'task_due_soon',
|
||||
},
|
||||
order: [['created_at', 'DESC']],
|
||||
});
|
||||
|
||||
expect(notification).not.toBeNull();
|
||||
expect(notification.channel_sent_at).toBeDefined();
|
||||
expect(notification.channel_sent_at.telegram).toBeDefined();
|
||||
|
||||
// Verify notification is still within 24h threshold
|
||||
expect(notification.wasChannelRecentlySent('telegram')).toBe(
|
||||
true
|
||||
);
|
||||
|
||||
sendTelegramSpy.mockClear();
|
||||
|
||||
// Second check within 24h - notification will be recreated but Telegram should NOT be resent
|
||||
await checkDueTasks();
|
||||
|
||||
const secondCallCount = sendTelegramSpy.mock.calls.length;
|
||||
expect(secondCallCount).toBe(0);
|
||||
|
||||
// Notification should still exist (recreated in-app)
|
||||
const notifications = await Notification.findAll({
|
||||
where: {
|
||||
user_id: user.id,
|
||||
type: 'task_due_soon',
|
||||
},
|
||||
});
|
||||
expect(notifications.length).toBe(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue