From 8b943b5f523b05e43b82bac4dd21989f7c823f1b Mon Sep 17 00:00:00 2001 From: alyaa_m Date: Sat, 2 May 2026 20:45:48 +0300 Subject: [PATCH 1/6] fix(notification)type comment -return referencetype,id as tracks not comments --- src/services/comment.service.js | 4 ++-- tests/notification.model.unit.test.js | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/services/comment.service.js b/src/services/comment.service.js index 0bece79..6b6f5b9 100644 --- a/src/services/comment.service.js +++ b/src/services/comment.service.js @@ -122,7 +122,7 @@ async function notifyTrackCommentIfNeeded({ actorUserId, trackId, commentId }) { userId: trackOwnerId, actionUserId: actorUserId, type: 'comment', - referenceId: commentId, - referenceType: 'comment', + referenceId: trackId, + referenceType: 'track', }); } diff --git a/tests/notification.model.unit.test.js b/tests/notification.model.unit.test.js index 70d86d4..052ae32 100644 --- a/tests/notification.model.unit.test.js +++ b/tests/notification.model.unit.test.js @@ -169,6 +169,7 @@ describe('notification.model', () => { }, ], }); + jest.spyOn(model, 'getUserEmailIdentity').mockResolvedValue({ display_name: '', username: 'bob', From a3740ac847dc3ed879adac745d5ee5ac2bf55594 Mon Sep 17 00:00:00 2001 From: alyaa_m Date: Sat, 2 May 2026 21:02:40 +0300 Subject: [PATCH 2/6] fix(notifications)spam push notifications for each new message) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added a tiny HTTP-only “recent activity” cache in conversation-activity.js. to avoid sending multiple pushs even when ur inside the chat --- src/services/messages.service.js | 10 ++++ src/services/push-notifications.service.js | 4 ++ src/utils/conversation-activity.js | 46 +++++++++++++++++++ tests/push-notifications.service.unit.test.js | 26 +++++++++++ 4 files changed, 86 insertions(+) create mode 100644 src/utils/conversation-activity.js diff --git a/src/services/messages.service.js b/src/services/messages.service.js index 37cc8cc..cc4637d 100644 --- a/src/services/messages.service.js +++ b/src/services/messages.service.js @@ -273,6 +273,8 @@ exports.listConversations = async ({ userId, page, limit }) => { // ------------------------------------------------------------ exports.getConversation = async ({ conversationId, userId, page, limit, offset: rawOffset }) => { + const conversationActivity = require('../utils/conversation-activity'); + // 1. Find conversation const conversation = await messageModel.findConversationById(conversationId); if (!conversation) { @@ -293,6 +295,9 @@ exports.getConversation = async ({ conversationId, userId, page, limit, offset: throw new AppError('Conversation not found.', 404, 'CONVERSATION_NOT_FOUND'); } + // Mark as active for push-notification suppression (best-effort) + conversationActivity.markActive({ userId, conversationId }); + // 4. Sanitize pagination inputs const safePage = Math.max(1, parseInt(page) || 1); const safeLimit = Math.min(100, Math.max(1, parseInt(limit) || 50)); @@ -439,6 +444,8 @@ exports.getUnreadCount = async ({ userId }) => { // ------------------------------------------------------------ exports.markMessageReadState = async ({ conversationId, messageId, userId, isRead }) => { + const conversationActivity = require('../utils/conversation-activity'); + // 1. Find conversation const conversation = await messageModel.findConversationById(conversationId); if (!conversation) { @@ -462,6 +469,9 @@ exports.markMessageReadState = async ({ conversationId, messageId, userId, isRea throw new AppError('You cannot change the read state of your own message.', 403, 'FORBIDDEN'); } + // Mark as active for push-notification suppression (best-effort) + conversationActivity.markActive({ userId, conversationId }); + // 5. Check if already in the requested state — 409 conflict if (message.is_read === isRead) { throw new AppError( diff --git a/src/services/push-notifications.service.js b/src/services/push-notifications.service.js index 651fe23..1134a57 100644 --- a/src/services/push-notifications.service.js +++ b/src/services/push-notifications.service.js @@ -104,12 +104,16 @@ exports.sendDirectMessagePushIfEligible = async ({ }) => { try { const notificationModel = require('../models/notification.model'); + const conversationActivity = require('../utils/conversation-activity'); // Check if user has push notifications enabled for new messages const recipientPrefs = await pushTokenModel.getPushPreferencesByUserId(recipientId); const shouldSendPush = recipientPrefs?.new_message_push !== false; if (!shouldSendPush) return; + // HTTP-only: if recipient fetched/updated this conversation very recently, skip push. + if (conversationActivity.isRecentlyActive({ userId: recipientId, conversationId })) return; + // Get sender info for title const sender = await notificationModel.getUserEmailIdentity(senderId); if (!sender) return; diff --git a/src/utils/conversation-activity.js b/src/utils/conversation-activity.js new file mode 100644 index 0000000..a3e6106 --- /dev/null +++ b/src/utils/conversation-activity.js @@ -0,0 +1,46 @@ + +// backend-only for "user is currently in a conversation" without relying on Socket.IO. to prevent edits for cross/front teams +// We mark a user as active when they hit conversation-related HTTP endpoints. +// +// This is per-node-process state (not shared across multiple server instances). + +const lastActiveByUserAndConversation = new Map(); // to avoid unbounded memory growth, we only store keys for active conversations and rely on expiration (see isRecentlyActive) + +const normalizeKey = (value) => { + if (typeof value !== 'string') return null; + const trimmed = value.trim(); + return trimmed ? trimmed : null; +}; + +const makeKey = (userId, conversationId) => { + const uid = normalizeKey(userId); + const cid = normalizeKey(conversationId); + if (!uid || !cid) return null; + return `${uid}:${cid}`; +}; + +exports.markActive = ({ userId, conversationId, now = Date.now() } = {}) => { + const key = makeKey(userId, conversationId); + if (!key) return; + lastActiveByUserAndConversation.set(key, now); +}; + +exports.isRecentlyActive = ({ + userId, + conversationId, + withinMs = 30_000, + now = Date.now(), +} = {}) => { + const key = makeKey(userId, conversationId); + if (!key) return false; + + const last = lastActiveByUserAndConversation.get(key); + if (!last) return false; + + return now - last <= withinMs; +}; + +// For unit tests only +exports._resetForTests = () => { + lastActiveByUserAndConversation.clear(); +}; diff --git a/tests/push-notifications.service.unit.test.js b/tests/push-notifications.service.unit.test.js index ade7121..2e7058b 100644 --- a/tests/push-notifications.service.unit.test.js +++ b/tests/push-notifications.service.unit.test.js @@ -119,6 +119,32 @@ describe('push-notifications.service', () => { jest.resetModules(); }); + it('does nothing when recipient is active in the same conversation', async () => { + const tokenModel = require('../src/models/push-token.model'); + tokenModel.getPushPreferencesByUserId.mockResolvedValue({ new_message_push: true }); + + jest.doMock('../src/utils/conversation-activity', () => ({ + isRecentlyActive: jest.fn().mockReturnValue(true), + })); + + jest.doMock('../src/models/notification.model', () => ({ + getUserEmailIdentity: jest.fn().mockResolvedValue({ display_name: 'Alice' }), + })); + + const svc = require('../src/services/push-notifications.service'); + const spy = jest.spyOn(svc, 'sendPushToUser').mockResolvedValue(undefined); + + await svc.sendDirectMessagePushIfEligible({ + conversationId: 'c1', + senderId: 'u1', + recipientId: 'u2', + messageBody: 'hi', + embedType: null, + }); + + expect(spy).not.toHaveBeenCalled(); + }); + it('does nothing when recipient has new_message_push disabled', async () => { const tokenModel = require('../src/models/push-token.model'); tokenModel.getPushPreferencesByUserId.mockResolvedValue({ new_message_push: false }); From 9227698b9c1f124eca4210215032f09e94f76390 Mon Sep 17 00:00:00 2001 From: alyaa_m Date: Sat, 2 May 2026 21:03:11 +0300 Subject: [PATCH 3/6] prettier format --- src/utils/conversation-activity.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/utils/conversation-activity.js b/src/utils/conversation-activity.js index a3e6106..3870a8c 100644 --- a/src/utils/conversation-activity.js +++ b/src/utils/conversation-activity.js @@ -1,4 +1,3 @@ - // backend-only for "user is currently in a conversation" without relying on Socket.IO. to prevent edits for cross/front teams // We mark a user as active when they hit conversation-related HTTP endpoints. // From 9b21811f6db75f52a93f0b8c2fd3682476235acc Mon Sep 17 00:00:00 2001 From: alyaa_m Date: Sat, 2 May 2026 21:24:41 +0300 Subject: [PATCH 4/6] prevent spam push notifications -prevent general spam push notifications for every single message sent by adding a throttle file -enhanced unit tests coverage for playlists,notifications --- src/services/push-notifications.service.js | 7 + src/utils/dm-push-throttle.js | 38 ++++ tests/dm-push-throttle.unit.test.js | 63 +++++++ tests/messages.service.unit.test.js | 14 +- tests/playlists.service.unit.test.js | 170 ++++++++++++++++++ tests/push-notifications.service.unit.test.js | 80 +++++++++ 6 files changed, 368 insertions(+), 4 deletions(-) create mode 100644 src/utils/dm-push-throttle.js create mode 100644 tests/dm-push-throttle.unit.test.js diff --git a/src/services/push-notifications.service.js b/src/services/push-notifications.service.js index 1134a57..0234534 100644 --- a/src/services/push-notifications.service.js +++ b/src/services/push-notifications.service.js @@ -105,6 +105,7 @@ exports.sendDirectMessagePushIfEligible = async ({ try { const notificationModel = require('../models/notification.model'); const conversationActivity = require('../utils/conversation-activity'); + const dmPushThrottle = require('../utils/dm-push-throttle'); // Check if user has push notifications enabled for new messages const recipientPrefs = await pushTokenModel.getPushPreferencesByUserId(recipientId); @@ -114,6 +115,10 @@ exports.sendDirectMessagePushIfEligible = async ({ // HTTP-only: if recipient fetched/updated this conversation very recently, skip push. if (conversationActivity.isRecentlyActive({ userId: recipientId, conversationId })) return; + // Anti-spam: throttle DM pushes per conversation per recipient. + // First message sends a push; subsequent messages within the cooldown window do not. + if (dmPushThrottle.isThrottled({ recipientId, conversationId })) return; + // Get sender info for title const sender = await notificationModel.getUserEmailIdentity(senderId); if (!sender) return; @@ -140,6 +145,8 @@ exports.sendDirectMessagePushIfEligible = async ({ data: { type: 'new_message', conversationId }, }) .catch((err) => console.error('[Push] Direct message push failed:', err?.message)); + + dmPushThrottle.markSent({ recipientId, conversationId }); } catch (err) { console.error('Direct message push notification skipped:', err?.message || err); } diff --git a/src/utils/dm-push-throttle.js b/src/utils/dm-push-throttle.js new file mode 100644 index 0000000..48ad91f --- /dev/null +++ b/src/utils/dm-push-throttle.js @@ -0,0 +1,38 @@ +// Simple in-memory throttle to avoid spamming DM push notifications +// For each recipient+conversation, we allow one push within the cooldown window (e.g. 60s) + +const lastPushByRecipientAndConversation = new Map(); // `${recipientId}:${conversationId}` -> timestampMs + +const normalizeKey = (value) => { + if (typeof value !== 'string') return null; + const trimmed = value.trim(); + return trimmed ? trimmed : null; +}; + +const makeKey = (recipientId, conversationId) => { + const rid = normalizeKey(recipientId); + const cid = normalizeKey(conversationId); + if (!rid || !cid) return null; + return `${rid}:${cid}`; +}; + +exports.isThrottled = ({ recipientId, conversationId, withinMs = 60_000, now = Date.now() } = {}) => { + const key = makeKey(recipientId, conversationId); + if (!key) return false; + + const last = lastPushByRecipientAndConversation.get(key); + if (!last) return false; + + return now - last <= withinMs; +}; + +exports.markSent = ({ recipientId, conversationId, now = Date.now() } = {}) => { + const key = makeKey(recipientId, conversationId); + if (!key) return; + lastPushByRecipientAndConversation.set(key, now); +}; + +// For unit tests only +exports._resetForTests = () => { + lastPushByRecipientAndConversation.clear(); +}; diff --git a/tests/dm-push-throttle.unit.test.js b/tests/dm-push-throttle.unit.test.js new file mode 100644 index 0000000..6cce459 --- /dev/null +++ b/tests/dm-push-throttle.unit.test.js @@ -0,0 +1,63 @@ +const throttle = require('../src/utils/dm-push-throttle'); + +describe('dm-push-throttle', () => { + beforeEach(() => throttle._resetForTests()); + + it('is not throttled before any push is sent', () => { + expect( + throttle.isThrottled({ recipientId: 'u1', conversationId: 'c1', withinMs: 60_000, now: 1000 }) + ).toBe(false); + }); + + it('throttles within window after markSent', () => { + throttle.markSent({ recipientId: 'u1', conversationId: 'c1', now: 1000 }); + + expect( + throttle.isThrottled({ recipientId: 'u1', conversationId: 'c1', withinMs: 60_000, now: 2000 }) + ).toBe(true); + }); + + it('does not throttle after window passes', () => { + throttle.markSent({ recipientId: 'u1', conversationId: 'c1', now: 1000 }); + + expect( + throttle.isThrottled({ recipientId: 'u1', conversationId: 'c1', withinMs: 60_000, now: 62_000 }) + ).toBe(false); + }); + + it('does not throttle when ids are missing/invalid', () => { + expect(throttle.isThrottled()).toBe(false); + expect(throttle.isThrottled({ recipientId: null, conversationId: 'c1' })).toBe(false); + expect(throttle.isThrottled({ recipientId: 'u1', conversationId: null })).toBe(false); + expect(throttle.isThrottled({ recipientId: 123, conversationId: 'c1' })).toBe(false); + expect(throttle.isThrottled({ recipientId: 'u1', conversationId: {} })).toBe(false); + expect(throttle.isThrottled({ recipientId: ' ', conversationId: 'c1' })).toBe(false); + expect(throttle.isThrottled({ recipientId: 'u1', conversationId: ' ' })).toBe(false); + }); + + it('ignores markSent when ids are invalid (no throw)', () => { + throttle.markSent(); + throttle.markSent({ recipientId: null, conversationId: 'c1' }); + throttle.markSent({ recipientId: 'u1', conversationId: null }); + throttle.markSent({ recipientId: ' ', conversationId: 'c1' }); + throttle.markSent({ recipientId: 'u1', conversationId: ' ' }); + + expect(throttle.isThrottled({ recipientId: 'u1', conversationId: 'c1' })).toBe(false); + }); + + it('trims ids so whitespace does not bypass throttling', () => { + throttle.markSent({ recipientId: ' u1 ', conversationId: ' c1 ', now: 1000 }); + + expect( + throttle.isThrottled({ recipientId: 'u1', conversationId: 'c1', withinMs: 60_000, now: 2000 }) + ).toBe(true); + }); + + it('throttles per recipient+conversation key (different conversation not throttled)', () => { + throttle.markSent({ recipientId: 'u1', conversationId: 'c1', now: 1000 }); + + expect( + throttle.isThrottled({ recipientId: 'u1', conversationId: 'c2', withinMs: 60_000, now: 2000 }) + ).toBe(false); + }); +}); diff --git a/tests/messages.service.unit.test.js b/tests/messages.service.unit.test.js index ad3abed..bf04cca 100644 --- a/tests/messages.service.unit.test.js +++ b/tests/messages.service.unit.test.js @@ -7,6 +7,12 @@ jest.mock('../src/models/message.model'); jest.mock('../src/services/tracks.service', () => ({ getTrackById: jest.fn(), })); +jest.mock('../src/services/email-notifications.service', () => ({ + sendDirectMessageEmailIfEligible: jest.fn().mockResolvedValue(undefined), +})); +jest.mock('../src/services/push-notifications.service', () => ({ + sendDirectMessagePushIfEligible: jest.fn().mockResolvedValue(undefined), +})); beforeEach(() => jest.clearAllMocks()); @@ -366,7 +372,7 @@ describe('messages.service', () => { const out = await service.listConversations({ userId: 'u1', page: '0', limit: '999' }); - expect(model.findConversationsByUserId).toHaveBeenCalledWith('u1', 8, 0); + expect(model.findConversationsByUserId).toHaveBeenCalledWith('u1', 50, 0); expect(out.items[0].last_message.id).toBe('m1'); expect(out.pagination.total_pages).toBe(1); }); @@ -397,12 +403,12 @@ describe('messages.service', () => { const out = await service.listConversations({ userId: 'u1', page: 'abc', limit: 'xyz' }); - expect(model.findConversationsByUserId).toHaveBeenCalledWith('u1', 8, 0); + expect(model.findConversationsByUserId).toHaveBeenCalledWith('u1', 20, 0); expect(out.pagination).toEqual({ page: 1, - limit: 8, + limit: 20, total: 0, - total_pages: 0, + total_pages: 1, }); }); }); diff --git a/tests/playlists.service.unit.test.js b/tests/playlists.service.unit.test.js index c05201a..8bed200 100644 --- a/tests/playlists.service.unit.test.js +++ b/tests/playlists.service.unit.test.js @@ -203,6 +203,38 @@ describe('Playlist Service', () => { }) ).resolves.toEqual(expect.objectContaining({ playlist: expect.any(Object) })); }); + + it('maps playlist_id and owner_user_id fallbacks when model returns legacy shape', async () => { + playlistModel.findBySlug.mockResolvedValue(null); + playlistModel.create.mockResolvedValue({ + id: mockPlaylistId, + user_id: mockUserId, + name: 'Legacy', + description: null, + cover_image: null, + type: 'regular', + subtype: 'playlist', + slug: 'legacy', + is_public: true, + secret_token: 's', + release_date: null, + genre_id: null, + like_count: 0, + repost_count: 0, + track_count: 0, + created_at: new Date().toISOString(), + updated_at: new Date().toISOString(), + }); + + const result = await playlistService.createPlaylist({ + userId: mockUserId, + name: 'Legacy', + isPublic: true, + }); + + expect(result.playlist.playlist_id).toBe(mockPlaylistId); + expect(result.playlist.owner_user_id).toBe(mockUserId); + }); }); // ──────────────────────────────────── @@ -342,6 +374,60 @@ describe('Playlist Service', () => { expect(result.items[0].cover_image).toBe('https://example.com/fallback.jpg'); }); + + it('does not query top track art when cover already exists', async () => { + playlistModel.findPublicPlaylists.mockResolvedValue([mockPlaylist]); + playlistModel.countPublicPlaylists.mockResolvedValue(1); + + const result = await playlistService.listPlaylists({ + requesterId: mockUserId, + mine: false, + limit: 20, + offset: 0, + }); + + expect(result.items).toHaveLength(1); + expect(playlistModel.getTopTrackArt).not.toHaveBeenCalled(); + }); + + it('formats playlist with fallback ids when owner_user_id is missing', async () => { + playlistModel.findPublicPlaylists.mockResolvedValue([ + { + id: mockPlaylistId, + user_id: mockUserId, + name: 'Legacy shape', + description: null, + cover_image: null, + type: 'regular', + subtype: 'playlist', + slug: 'legacy-shape', + is_public: true, + release_date: null, + genre_id: null, + like_count: 0, + repost_count: 0, + track_count: 0, + created_at: new Date().toISOString(), + updated_at: new Date().toISOString(), + }, + ]); + playlistModel.countPublicPlaylists.mockResolvedValue(1); + playlistModel.getTopTrackArt.mockResolvedValue(null); + + const result = await playlistService.listPlaylists({ + requesterId: mockUserId, + mine: false, + limit: 20, + offset: 0, + }); + + expect(result.items[0]).toEqual( + expect.objectContaining({ + playlist_id: mockPlaylistId, + owner_user_id: mockUserId, + }) + ); + }); }); // ──────────────────────────────────── @@ -806,6 +892,23 @@ describe('Playlist Service', () => { expect(result.playlist.name).toBe('New Name'); expect(result.playlist.description).toBe('New Desc'); }); + + it('generates unique slug when explicit slug collides', async () => { + playlistModel.findPlaylistById.mockResolvedValue(mockPlaylist); + playlistModel.findBySlug.mockResolvedValueOnce({ id: uuidv4() }).mockResolvedValueOnce(null); + playlistModel.updatePlaylist.mockResolvedValue({ ...mockPlaylist, slug: 'my-slug-1' }); + playlistModel.getTopTrackArt.mockResolvedValue(null); + playlistModel.replacePlaylistTags.mockResolvedValue([]); + + const result = await playlistService.updatePlaylist({ + playlistId: mockPlaylistId, + userId: mockUserId, + slug: 'My Slug', + }); + + expect(playlistModel.findBySlug).toHaveBeenCalled(); + expect(result.playlist.slug).toBe('my-slug-1'); + }); }); // ──────────────────────────────────── @@ -1453,6 +1556,73 @@ describe('Playlist Service', () => { expect(result).toBeDefined(); }); + it('converts even when generated playlist has no tracks (skips bulk insert)', async () => { + const generated = { ...mockPlaylist, type: 'curated_daily' }; + playlistModel.findPlaylistById.mockResolvedValue(generated); + feedModel.getDailyTracks.mockResolvedValue([]); + playlistModel.create.mockResolvedValue(mockPlaylist); + playlistModel.findBySlug.mockResolvedValue(null); + playlistModel.bulkInsertTracks.mockResolvedValue(true); + db.query.mockResolvedValue({ rows: [] }); + playlistModel.hardDelete.mockResolvedValue(true); + playlistModel.findPlaylistTracks.mockResolvedValue([]); + playlistModel.getTopTrackArt.mockResolvedValue(null); + + const result = await playlistService.convertPlaylist({ + playlistId: mockPlaylistId, + userId: mockUserId, + name: 'Empty Mix', + isPublic: true, + }); + + expect(result).toBeDefined(); + expect(playlistModel.bulkInsertTracks).not.toHaveBeenCalled(); + }); + + it('auto_generated with missing genre_id snapshots with no tracks', async () => { + const generated = { ...mockPlaylist, type: 'auto_generated', genre_id: null }; + playlistModel.findPlaylistById.mockResolvedValue(generated); + playlistModel.create.mockResolvedValue(mockPlaylist); + playlistModel.findBySlug.mockResolvedValue(null); + db.query.mockResolvedValue({ rows: [] }); + playlistModel.hardDelete.mockResolvedValue(true); + playlistModel.findPlaylistTracks.mockResolvedValue([]); + playlistModel.getTopTrackArt.mockResolvedValue(null); + + const result = await playlistService.convertPlaylist({ + playlistId: mockPlaylistId, + userId: mockUserId, + name: 'No Genre', + isPublic: false, + }); + + expect(result).toBeDefined(); + expect(feedModel.findTracksByGenreId).not.toHaveBeenCalled(); + expect(playlistModel.bulkInsertTracks).not.toHaveBeenCalled(); + }); + + it('auto_generated handles non-array genre result defensively', async () => { + const generated = { ...mockPlaylist, type: 'auto_generated', genre_id: mockGenreId }; + playlistModel.findPlaylistById.mockResolvedValue(generated); + feedModel.findTracksByGenreId.mockResolvedValue('not-an-array'); + playlistModel.create.mockResolvedValue(mockPlaylist); + playlistModel.findBySlug.mockResolvedValue(null); + db.query.mockResolvedValue({ rows: [] }); + playlistModel.hardDelete.mockResolvedValue(true); + playlistModel.findPlaylistTracks.mockResolvedValue([]); + playlistModel.getTopTrackArt.mockResolvedValue(null); + + const result = await playlistService.convertPlaylist({ + playlistId: mockPlaylistId, + userId: mockUserId, + name: 'Defensive', + isPublic: true, + }); + + expect(result).toBeDefined(); + expect(playlistModel.bulkInsertTracks).not.toHaveBeenCalled(); + }); + it('should handle other playlist types', async () => { const generated = { ...mockPlaylist, type: 'track_radio' }; playlistModel.findPlaylistById.mockResolvedValue(generated); diff --git a/tests/push-notifications.service.unit.test.js b/tests/push-notifications.service.unit.test.js index 2e7058b..58b16b3 100644 --- a/tests/push-notifications.service.unit.test.js +++ b/tests/push-notifications.service.unit.test.js @@ -102,6 +102,19 @@ describe('push-notifications.service', () => { ); }); + it('sends when preferences row is missing (legacy default allow)', async () => { + pushTokenModel.getPushPreferencesByUserId.mockResolvedValue(null); + pushTokenModel.getTokensByUserId.mockResolvedValue([{ token: 'a' }]); + sendPushNotification.mockResolvedValue(undefined); + + await service.sendPushToUser({ userId: 'u1', title: 'Hi', body: 'There', data: { type: 'like' } }); + + expect(sendPushNotification).toHaveBeenCalledTimes(1); + expect(sendPushNotification).toHaveBeenCalledWith( + expect.objectContaining({ token: 'a', title: 'Hi', body: 'There', data: { type: 'like' } }) + ); + }); + it('never throws when model layer throws', async () => { const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); pushTokenModel.getPushPreferencesByUserId.mockRejectedValue(new Error('db down')); @@ -117,6 +130,18 @@ describe('push-notifications.service', () => { describe('sendDirectMessagePushIfEligible', () => { beforeEach(() => { jest.resetModules(); + + // Default: recipient is NOT actively viewing the conversation. + // Individual tests can override this with their own jest.doMock. + jest.doMock('../src/utils/conversation-activity', () => ({ + isRecentlyActive: jest.fn().mockReturnValue(false), + })); + + // Default: do not throttle DM pushes. + jest.doMock('../src/utils/dm-push-throttle', () => ({ + isThrottled: jest.fn().mockReturnValue(false), + markSent: jest.fn(), + })); }); it('does nothing when recipient is active in the same conversation', async () => { @@ -145,6 +170,33 @@ describe('push-notifications.service', () => { expect(spy).not.toHaveBeenCalled(); }); + it('does nothing when DM push is throttled', async () => { + const tokenModel = require('../src/models/push-token.model'); + tokenModel.getPushPreferencesByUserId.mockResolvedValue({ new_message_push: true }); + + jest.doMock('../src/utils/dm-push-throttle', () => ({ + isThrottled: jest.fn().mockReturnValue(true), + markSent: jest.fn(), + })); + + jest.doMock('../src/models/notification.model', () => ({ + getUserEmailIdentity: jest.fn().mockResolvedValue({ display_name: 'Alice' }), + })); + + const svc = require('../src/services/push-notifications.service'); + const spy = jest.spyOn(svc, 'sendPushToUser').mockResolvedValue(undefined); + + await svc.sendDirectMessagePushIfEligible({ + conversationId: 'c1', + senderId: 'u1', + recipientId: 'u2', + messageBody: 'hi', + embedType: null, + }); + + expect(spy).not.toHaveBeenCalled(); + }); + it('does nothing when recipient has new_message_push disabled', async () => { const tokenModel = require('../src/models/push-token.model'); tokenModel.getPushPreferencesByUserId.mockResolvedValue({ new_message_push: false }); @@ -245,5 +297,33 @@ describe('push-notifications.service', () => { }) ); }); + + it('uses generic preview when body and embed are missing', async () => { + const tokenModel = require('../src/models/push-token.model'); + tokenModel.getPushPreferencesByUserId.mockResolvedValue({ new_message_push: true }); + + jest.doMock('../src/models/notification.model', () => ({ + getUserEmailIdentity: jest.fn().mockResolvedValue({ display_name: 'Alice' }), + })); + + const svc = require('../src/services/push-notifications.service'); + const spy = jest.spyOn(svc, 'sendPushToUser').mockResolvedValue(undefined); + + await svc.sendDirectMessagePushIfEligible({ + conversationId: 'c1', + senderId: 'u1', + recipientId: 'u2', + messageBody: '', + embedType: null, + }); + + expect(spy).toHaveBeenCalledWith( + expect.objectContaining({ + title: 'Message from Alice', + body: 'New message', + data: { type: 'new_message', conversationId: 'c1' }, + }) + ); + }); }); }); From 4ebf8675f4cbf55afbce5b91b5123fb8f6ba3e3a Mon Sep 17 00:00:00 2001 From: alyaa_m Date: Sat, 2 May 2026 21:32:35 +0300 Subject: [PATCH 5/6] refactor(messages)speed of sending messages make sending messages independant from sending notifications --- src/services/messages.service.js | 46 +++++++++++------ tests/messages.service.unit.test.js | 76 +++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 14 deletions(-) diff --git a/src/services/messages.service.js b/src/services/messages.service.js index cc4637d..f74fabe 100644 --- a/src/services/messages.service.js +++ b/src/services/messages.service.js @@ -6,6 +6,36 @@ const pushNotificationsService = require('./push-notifications.service'); const ALLOWED_EMBED_TYPES = ['track', 'playlist']; +const logNotificationFailure = (channel, err) => { + console.error(`[Messages] ${channel} notification failed:`, err?.message || err); +}; + +const sendDirectMessageNotifications = ({ + conversationId, + senderId, + recipientId, + messageBody, + embedType, +}) => { + emailNotificationsService + .sendDirectMessageEmailIfEligible({ + conversationId, + senderId, + recipientId, + }) + .catch((err) => logNotificationFailure('Email', err)); + + pushNotificationsService + .sendDirectMessagePushIfEligible({ + conversationId, + senderId, + recipientId, + messageBody, + embedType, + }) + .catch((err) => logNotificationFailure('Push', err)); +}; + const validateSenderId = (senderId) => { if (!senderId) { throw new AppError('Authenticated sender is required.', 401, 'UNAUTHORIZED'); @@ -174,13 +204,7 @@ exports.startConversation = async ({ senderId, recipientId, body, resource }) => embedId: payload.resource?.id ?? null, }); - await emailNotificationsService.sendDirectMessageEmailIfEligible({ - conversationId: conversation.id, - senderId, - recipientId, - }); - - await pushNotificationsService.sendDirectMessagePushIfEligible({ + sendDirectMessageNotifications({ conversationId: conversation.id, senderId, recipientId, @@ -411,13 +435,7 @@ exports.sendMessage = async ({ conversationId, senderId, body, resource }) => { embedId: payload.resource?.id ?? null, }); - await emailNotificationsService.sendDirectMessageEmailIfEligible({ - conversationId, - senderId, - recipientId, - }); - - await pushNotificationsService.sendDirectMessagePushIfEligible({ + sendDirectMessageNotifications({ conversationId, senderId, recipientId, diff --git a/tests/messages.service.unit.test.js b/tests/messages.service.unit.test.js index bf04cca..74b34e2 100644 --- a/tests/messages.service.unit.test.js +++ b/tests/messages.service.unit.test.js @@ -1,6 +1,8 @@ const service = require('../src/services/messages.service'); const model = require('../src/models/message.model'); const tracksService = require('../src/services/tracks.service'); +const emailNotificationsService = require('../src/services/email-notifications.service'); +const pushNotificationsService = require('../src/services/push-notifications.service'); const AppError = require('../src/utils/app-error'); jest.mock('../src/models/message.model'); @@ -217,6 +219,46 @@ describe('messages.service', () => { expect(model.createMessage).toHaveBeenCalledWith(expect.objectContaining({ body: 'hi' })); }); + it('returns created conversation without waiting for direct-message notifications', async () => { + model.findActiveUserById.mockResolvedValue({ id: 'u2' }); + model.isBlocked.mockResolvedValue(false); + model.getMessagesFromPreference.mockResolvedValue('everyone'); + model.findConversationByPair.mockResolvedValue(null); + model.createConversation.mockResolvedValue({ id: 'c1' }); + model.createMessage.mockResolvedValue({ id: 'm1' }); + emailNotificationsService.sendDirectMessageEmailIfEligible.mockReturnValue( + new Promise(() => {}) + ); + pushNotificationsService.sendDirectMessagePushIfEligible.mockReturnValue( + new Promise(() => {}) + ); + + await expect( + service.startConversation({ + senderId: 'u1', + recipientId: 'u2', + body: 'hi', + }) + ).resolves.toEqual({ + conversation: { id: 'c1' }, + message: { id: 'm1' }, + isNew: true, + }); + + expect(emailNotificationsService.sendDirectMessageEmailIfEligible).toHaveBeenCalledWith({ + conversationId: 'c1', + senderId: 'u1', + recipientId: 'u2', + }); + expect(pushNotificationsService.sendDirectMessagePushIfEligible).toHaveBeenCalledWith({ + conversationId: 'c1', + senderId: 'u1', + recipientId: 'u2', + messageBody: 'hi', + embedType: null, + }); + }); + it('uses existing conversation', async () => { model.findActiveUserById.mockResolvedValue({ id: 'u2' }); model.isBlocked.mockResolvedValue(false); @@ -630,6 +672,40 @@ describe('messages.service', () => { expect(model.createMessage).toHaveBeenCalledWith(expect.objectContaining({ body: 'hi' })); }); + it('returns sent message without waiting for direct-message notifications', async () => { + model.findConversationById.mockResolvedValue(baseConversation); + model.isBlocked.mockResolvedValue(false); + model.getMessagesFromPreference.mockResolvedValue('everyone'); + model.createMessage.mockResolvedValue({ id: 'm1', body: 'hi' }); + emailNotificationsService.sendDirectMessageEmailIfEligible.mockReturnValue( + new Promise(() => {}) + ); + pushNotificationsService.sendDirectMessagePushIfEligible.mockReturnValue( + new Promise(() => {}) + ); + + await expect( + service.sendMessage({ + conversationId: 'c1', + senderId: 'u1', + body: 'hi', + }) + ).resolves.toEqual({ id: 'm1', body: 'hi' }); + + expect(emailNotificationsService.sendDirectMessageEmailIfEligible).toHaveBeenCalledWith({ + conversationId: 'c1', + senderId: 'u1', + recipientId: 'u2', + }); + expect(pushNotificationsService.sendDirectMessagePushIfEligible).toHaveBeenCalledWith({ + conversationId: 'c1', + senderId: 'u1', + recipientId: 'u2', + messageBody: 'hi', + embedType: null, + }); + }); + it('rejects invalid track resource id', async () => { model.findConversationById.mockResolvedValue(baseConversation); model.isBlocked.mockResolvedValue(false); From b46ba3e23332e6e5e18360ab44be4e0f9cb3d879 Mon Sep 17 00:00:00 2001 From: alyaa_m Date: Sat, 2 May 2026 21:32:49 +0300 Subject: [PATCH 6/6] prettier format --- src/utils/dm-push-throttle.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/utils/dm-push-throttle.js b/src/utils/dm-push-throttle.js index 48ad91f..b500913 100644 --- a/src/utils/dm-push-throttle.js +++ b/src/utils/dm-push-throttle.js @@ -16,7 +16,12 @@ const makeKey = (recipientId, conversationId) => { return `${rid}:${cid}`; }; -exports.isThrottled = ({ recipientId, conversationId, withinMs = 60_000, now = Date.now() } = {}) => { +exports.isThrottled = ({ + recipientId, + conversationId, + withinMs = 60_000, + now = Date.now(), +} = {}) => { const key = makeKey(recipientId, conversationId); if (!key) return false;