diff --git a/packages/backend/src/search/search.controller.spec.ts b/packages/backend/src/search/search.controller.spec.ts index f4be84d1..0bdf2452 100644 --- a/packages/backend/src/search/search.controller.spec.ts +++ b/packages/backend/src/search/search.controller.spec.ts @@ -67,7 +67,7 @@ describe('searchController', () => { }); }); - it('returns only public channel messages when persistence returns mixed channel types', async () => { + it('returns messages exactly as provided by persistence', async () => { searchMessagesMock.mockResolvedValue({ messages: [ { id: 1, message: 'public', name: 'alice', channel: 'C111' }, @@ -82,7 +82,11 @@ describe('searchController', () => { expect(res.status).toBe(200); expect(res.body).toEqual({ - messages: [{ id: 1, message: 'public', name: 'alice', channel: 'C111' }], + messages: [ + { id: 1, message: 'public', name: 'alice', channel: 'C111' }, + { id: 2, message: 'private', name: 'bob', channel: 'G222' }, + { id: 3, message: 'dm', name: 'carol', channel: 'D333' }, + ], mentions: {}, total: 3, }); diff --git a/packages/backend/src/search/search.controller.ts b/packages/backend/src/search/search.controller.ts index c4a1c4c0..72e4cdd1 100644 --- a/packages/backend/src/search/search.controller.ts +++ b/packages/backend/src/search/search.controller.ts @@ -11,8 +11,6 @@ export const searchController: Router = express.Router(); const searchPersistenceService = new SearchPersistenceService(); const searchLogger = logger.child({ module: 'SearchController' }); -const isPublicChannelId = (value: unknown): boolean => typeof value === 'string' && value.startsWith('C'); - searchController.get('/filters', (req: RequestWithAuthSession, res) => { const teamId = req.authSession?.teamId; if (!teamId) { @@ -72,11 +70,7 @@ searchController.get('/messages', (req: RequestWithAuthSession, res) => { limit: parsedLimit, offset: parsedOffset, }) - .then(({ messages, mentions, total }) => - res - .status(200) - .json({ messages: messages.filter((message) => isPublicChannelId(message.channel)), mentions, total }), - ) + .then(({ messages, mentions, total }) => res.status(200).json({ messages, mentions, total })) .catch((e: unknown) => { logError(searchLogger, 'Failed to search messages', e, { userName, diff --git a/packages/backend/src/search/search.persistence.service.spec.ts b/packages/backend/src/search/search.persistence.service.spec.ts index 40cad798..f8353015 100644 --- a/packages/backend/src/search/search.persistence.service.spec.ts +++ b/packages/backend/src/search/search.persistence.service.spec.ts @@ -55,7 +55,7 @@ describe('SearchPersistenceService', () => { expect(countSql).toContain("message.message != ''"); expect(countSql).toContain('message.teamId = ?'); expect(countSql).toContain('slack_user.teamId = ?'); - expect(countSql).toContain("message.channel LIKE 'C%'"); + expect(countSql).toContain('INNER JOIN slack_channel ON slack_channel.channelId = message.channel'); expect(countParams).toEqual(['T1', 'T1']); expect(dataSql).toContain("message.message != ''"); @@ -90,6 +90,19 @@ describe('SearchPersistenceService', () => { expect(dataParams).toEqual(['T1', 'T1', '%general%', '%general%', 100, 0]); }); + it('requires channel to exist in slack_channel via INNER JOIN', async () => { + query.mockResolvedValueOnce([{ total: 0 }]).mockResolvedValueOnce([]); + + await service.searchMessages({ teamId: 'T1' }); + + const [countSql] = (query as Mock).mock.calls[0] as [string, unknown[]]; + const [dataSql] = (query as Mock).mock.calls[1] as [string, unknown[]]; + + expect(countSql).toContain('INNER JOIN slack_channel ON slack_channel.channelId = message.channel'); + expect(dataSql).toContain('INNER JOIN slack_channel ON slack_channel.channelId = message.channel'); + expect(dataSql).toContain('slack_channel.name AS channelName'); + }); + it('applies content LIKE filter when content is provided', async () => { query.mockResolvedValueOnce([{ total: 0 }]).mockResolvedValueOnce([]); diff --git a/packages/backend/src/search/search.persistence.service.ts b/packages/backend/src/search/search.persistence.service.ts index c695dc2e..27ebf09c 100644 --- a/packages/backend/src/search/search.persistence.service.ts +++ b/packages/backend/src/search/search.persistence.service.ts @@ -46,12 +46,7 @@ export class SearchPersistenceService { // Each condition is joined with AND in the WHERE clause; filterParams holds the // positional `?` values in the same order as the conditions that use them. - const conditions: string[] = [ - "message.message != ''", - 'message.teamId = ?', - 'slack_user.teamId = ?', - "message.channel LIKE 'C%'", - ]; + const conditions: string[] = ["message.message != ''", 'message.teamId = ?', 'slack_user.teamId = ?']; const filterParams: (string | number)[] = [teamId, teamId]; if (userName) { @@ -75,14 +70,14 @@ export class SearchPersistenceService { const joins = ` FROM message INNER JOIN slack_user ON slack_user.id = message.userIdId - LEFT JOIN slack_channel ON slack_channel.channelId = message.channel AND slack_channel.teamId = message.teamId + INNER JOIN slack_channel ON slack_channel.channelId = message.channel AND slack_channel.teamId = message.teamId WHERE ${whereClause} `; const countQuery = `SELECT COUNT(*) AS total ${joins}`; const dataQuery = ` - SELECT message.*, slack_user.name, slack_user.slackId, COALESCE(slack_channel.name, message.channel) AS channelName + SELECT message.*, slack_user.name, slack_user.slackId, slack_channel.name AS channelName ${joins} ORDER BY message.createdAt DESC LIMIT ?