From a3248d2a4133cc87bf31e7361c96d6eab26dcb59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Wed, 14 Jan 2026 15:06:44 +0100 Subject: [PATCH] [MM-66753] Scope unified playbook view to team (#9366) * [MM-66753] Scope unified playbook view to team * Rename function --- .../playbooks/actions/remote/runs.test.ts | 14 ++-- app/products/playbooks/actions/remote/runs.ts | 3 +- .../playbooks/database/queries/run.test.ts | 64 +++++++++++++++++-- .../playbooks/database/queries/run.ts | 6 +- .../participant_playbooks/index.test.tsx | 63 ++++++++++++++++-- .../screens/participant_playbooks/index.ts | 15 +++-- .../participant_playbooks.test.tsx | 17 ++--- .../participant_playbooks.tsx | 6 +- 8 files changed, 152 insertions(+), 36 deletions(-) diff --git a/app/products/playbooks/actions/remote/runs.test.ts b/app/products/playbooks/actions/remote/runs.test.ts index 5dbf2dec2..4d1ce1c8b 100644 --- a/app/products/playbooks/actions/remote/runs.test.ts +++ b/app/products/playbooks/actions/remote/runs.test.ts @@ -449,6 +449,7 @@ describe('finishRun', () => { describe('fetchPlaybookRunsPageForParticipant', () => { const participantId = 'participant-id-1'; + const teamId = 'team-id-1'; beforeEach(() => { jest.clearAllMocks(); @@ -461,7 +462,7 @@ describe('fetchPlaybookRunsPageForParticipant', () => { has_more: false, }); - const result = await fetchPlaybookRunsPageForParticipant(serverUrl, participantId); + const result = await fetchPlaybookRunsPageForParticipant(serverUrl, participantId, teamId); expect(result).toBeDefined(); expect(result.error).toBeUndefined(); @@ -473,6 +474,7 @@ describe('fetchPlaybookRunsPageForParticipant', () => { participant_id: participantId, sort: 'create_at', direction: 'desc', + team_id: teamId, }); }); @@ -483,7 +485,7 @@ describe('fetchPlaybookRunsPageForParticipant', () => { has_more: true, }); - const result = await fetchPlaybookRunsPageForParticipant(serverUrl, participantId, 2); + const result = await fetchPlaybookRunsPageForParticipant(serverUrl, participantId, teamId, 2); expect(result).toBeDefined(); expect(result.error).toBeUndefined(); @@ -495,13 +497,14 @@ describe('fetchPlaybookRunsPageForParticipant', () => { participant_id: participantId, sort: 'create_at', direction: 'desc', + team_id: teamId, }); }); it('should handle network error', async () => { mockClient.fetchPlaybookRuns.mockRejectedValue(new Error('Network error')); - const result = await fetchPlaybookRunsPageForParticipant(serverUrl, participantId); + const result = await fetchPlaybookRunsPageForParticipant(serverUrl, participantId, teamId); expect(result).toBeDefined(); expect(result.error).toBeDefined(); @@ -515,7 +518,7 @@ describe('fetchPlaybookRunsPageForParticipant', () => { has_more: false, }); - const result = await fetchPlaybookRunsPageForParticipant(serverUrl, participantId); + const result = await fetchPlaybookRunsPageForParticipant(serverUrl, participantId, teamId); expect(result).toBeDefined(); expect(result.error).toBeUndefined(); @@ -530,7 +533,7 @@ describe('fetchPlaybookRunsPageForParticipant', () => { has_more: false, }); - await fetchPlaybookRunsPageForParticipant(serverUrl, participantId); + await fetchPlaybookRunsPageForParticipant(serverUrl, participantId, teamId); expect(mockClient.fetchPlaybookRuns).toHaveBeenCalledWith({ page: 0, @@ -538,6 +541,7 @@ describe('fetchPlaybookRunsPageForParticipant', () => { participant_id: participantId, sort: 'create_at', direction: 'desc', + team_id: teamId, }); }); }); diff --git a/app/products/playbooks/actions/remote/runs.ts b/app/products/playbooks/actions/remote/runs.ts index 67fa9116f..b390e2346 100644 --- a/app/products/playbooks/actions/remote/runs.ts +++ b/app/products/playbooks/actions/remote/runs.ts @@ -206,7 +206,7 @@ export const finishRun = async (serverUrl: string, playbookRunId: string) => { } }; -export const fetchPlaybookRunsPageForParticipant = async (serverUrl: string, participantId: string, page = 0) => { +export const fetchPlaybookRunsPageForParticipant = async (serverUrl: string, participantId: string, teamId: string, page = 0) => { try { const client = NetworkManager.getClient(serverUrl); @@ -216,6 +216,7 @@ export const fetchPlaybookRunsPageForParticipant = async (serverUrl: string, par participant_id: participantId, sort: 'create_at', direction: 'desc', + team_id: teamId, }); return {runs, hasMore: has_more}; diff --git a/app/products/playbooks/database/queries/run.test.ts b/app/products/playbooks/database/queries/run.test.ts index 352d7a1fd..ed61cede4 100644 --- a/app/products/playbooks/database/queries/run.test.ts +++ b/app/products/playbooks/database/queries/run.test.ts @@ -1,6 +1,8 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. +/* eslint-disable max-lines */ + import {of as of$} from 'rxjs'; import {MM_TABLES} from '@constants/database'; @@ -9,7 +11,7 @@ import TestHelper from '@test/test_helper'; import { queryPlaybookRunsPerChannel, - queryPlaybookRunsByParticipant, + queryPlaybookRunsByParticipantAndTeam, getPlaybookRunById, observePlaybookRunById, observePlaybookRunProgress, @@ -630,10 +632,12 @@ describe('Playbook Run Queries', () => { describe('queryPlaybookRunsByParticipant', () => { it('should query playbook runs by participant ID', async () => { const participantId = 'participant-id-1'; + const teamId = 'team-id-1'; const mockRuns = TestHelper.createPlaybookRuns(2, 0, 0).map((run, index) => ({ ...run, participant_ids: [participantId, 'other-participant'], id: `run-${index}`, + channel_id: 'channel-id-1', })); await operator.handlePlaybookRun({ @@ -642,7 +646,13 @@ describe('Playbook Run Queries', () => { removeAssociatedRecords: false, }); - const result = queryPlaybookRunsByParticipant(operator.database, participantId); + const channel = TestHelper.fakeChannel({id: 'channel-id-1', team_id: teamId}); + await operator.handleChannel({ + channels: [channel], + prepareRecordsOnly: false, + }); + + const result = queryPlaybookRunsByParticipantAndTeam(operator.database, participantId, teamId); const fetchedRuns = await result.fetch(); expect(fetchedRuns.length).toBe(2); @@ -656,10 +666,12 @@ describe('Playbook Run Queries', () => { it('should return empty array when no runs match participant ID', async () => { const participantId = 'participant-id-1'; + const teamId = 'team-id-1'; const nonMatchingParticipantId = 'other-participant'; const mockRuns = TestHelper.createPlaybookRuns(1, 0, 0).map((run) => ({ ...run, participant_ids: [nonMatchingParticipantId], + channel_id: 'channel-id-1', })); await operator.handlePlaybookRun({ @@ -668,7 +680,13 @@ describe('Playbook Run Queries', () => { removeAssociatedRecords: false, }); - const result = queryPlaybookRunsByParticipant(operator.database, participantId); + const channel = TestHelper.fakeChannel({id: 'channel-id-1', team_id: teamId}); + await operator.handleChannel({ + channels: [channel], + prepareRecordsOnly: false, + }); + + const result = queryPlaybookRunsByParticipantAndTeam(operator.database, participantId, teamId); const fetchedRuns = await result.fetch(); expect(fetchedRuns.length).toBe(0); @@ -676,6 +694,7 @@ describe('Playbook Run Queries', () => { it('should handle JSON array participant_ids properly', async () => { const participantId = 'user123'; + const teamId = 'team-id-1'; const mockRuns = TestHelper.createPlaybookRuns(3, 0, 0).map((run, index) => ({ ...run, participant_ids: (() => { @@ -688,6 +707,7 @@ describe('Playbook Run Queries', () => { return ['different']; })(), id: `run-${index}`, + channel_id: 'channel-id-1', })); await operator.handlePlaybookRun({ @@ -696,7 +716,13 @@ describe('Playbook Run Queries', () => { removeAssociatedRecords: false, }); - const result = queryPlaybookRunsByParticipant(operator.database, participantId); + const channel = TestHelper.fakeChannel({id: 'channel-id-1', team_id: teamId}); + await operator.handleChannel({ + channels: [channel], + prepareRecordsOnly: false, + }); + + const result = queryPlaybookRunsByParticipantAndTeam(operator.database, participantId, teamId); const fetchedRuns = await result.fetch(); // Should only return the first two runs (index 0 and 1) @@ -706,5 +732,35 @@ describe('Playbook Run Queries', () => { expect(runIds).toContain(mockRuns[1].id); expect(runIds).not.toContain(mockRuns[2].id); }); + + it('should filter out runs from other teams', async () => { + const participantId = 'participant-id-1'; + const teamId = 'team-id-1'; + const mockRuns = TestHelper.createPlaybookRuns(2, 0, 0).map((run, index) => ({ + ...run, + participant_ids: [participantId], + channel_id: index === 0 ? 'channel-id-1' : 'channel-id-2', + })); + + await operator.handlePlaybookRun({ + runs: mockRuns, + prepareRecordsOnly: false, + removeAssociatedRecords: false, + }); + + const channel = TestHelper.fakeChannel({id: 'channel-id-1', team_id: teamId}); + await operator.handleChannel({ + channels: [channel], + prepareRecordsOnly: false, + }); + + const result = queryPlaybookRunsByParticipantAndTeam(operator.database, participantId, teamId); + const fetchedRuns = await result.fetch(); + + expect(fetchedRuns.length).toBe(1); + const runIds = fetchedRuns.map((run) => run.id); + expect(runIds).toContain(mockRuns[0].id); + expect(runIds).not.toContain(mockRuns[1].id); + }); }); }); diff --git a/app/products/playbooks/database/queries/run.ts b/app/products/playbooks/database/queries/run.ts index 50397423e..12187f4da 100644 --- a/app/products/playbooks/database/queries/run.ts +++ b/app/products/playbooks/database/queries/run.ts @@ -15,7 +15,7 @@ import type PlaybookRunModel from '@playbooks/types/database/models/playbook_run import type MyChannelModel from '@typings/database/models/servers/my_channel'; const {PLAYBOOK_RUN, PLAYBOOK_CHECKLIST, PLAYBOOK_CHECKLIST_ITEM} = PLAYBOOK_TABLES; -const {MY_CHANNEL} = MM_TABLES.SERVER; +const {MY_CHANNEL, CHANNEL} = MM_TABLES.SERVER; export const queryPlaybookRunsPerChannel = (database: Database, channelId: string, finished?: boolean) => { const conditions = [Q.where('channel_id', channelId)]; @@ -89,8 +89,10 @@ export const observeParticipantsIdsFromPlaybookModel = (runModel: PlaybookRunMod ); }; -export const queryPlaybookRunsByParticipant = (database: Database, participantId: string) => { +export const queryPlaybookRunsByParticipantAndTeam = (database: Database, participantId: string, teamId: string) => { return database.get(PLAYBOOK_RUN).query( + Q.experimentalJoinTables([CHANNEL]), + Q.on(CHANNEL, 'team_id', Q.eq(teamId)), Q.where('participant_ids', Q.like(`%"${participantId}"%`)), Q.sortBy('create_at', 'desc'), ); diff --git a/app/products/playbooks/screens/participant_playbooks/index.test.tsx b/app/products/playbooks/screens/participant_playbooks/index.test.tsx index 7cb5991bf..fd9629ea6 100644 --- a/app/products/playbooks/screens/participant_playbooks/index.test.tsx +++ b/app/products/playbooks/screens/participant_playbooks/index.test.tsx @@ -59,29 +59,52 @@ describe('ParticipantPlaybooks Index', () => { expect(component).toBeTruthy(); expect(component.props.componentId).toBe('ParticipantPlaybooks'); expect(component.props.currentUserId).toBe(''); + expect(component.props.currentTeamId).toBe(''); expect(component.props.cachedPlaybookRuns).toEqual([]); }); it('renders ParticipantPlaybooks component with current user data', async () => { + const currentTeamId = 'current-team-id'; await operator.handleSystem({ - systems: [{ - id: SYSTEM_IDENTIFIERS.CURRENT_USER_ID, - value: currentUserId, - }], + systems: [ + { + id: SYSTEM_IDENTIFIERS.CURRENT_USER_ID, + value: currentUserId, + }, + { + id: SYSTEM_IDENTIFIERS.CURRENT_TEAM_ID, + value: currentTeamId, + }, + ], prepareRecordsOnly: false, }); - const runs = TestHelper.createPlaybookRuns(4, 1, 1); + const runs = TestHelper.createPlaybookRuns(5, 1, 1); runs[0].participant_ids = [currentUserId]; runs[1].participant_ids = ['other-user-id']; runs[2].participant_ids = [currentUserId]; runs[3].participant_ids = ['other-user-id']; + runs[4].participant_ids = [currentUserId]; + + runs[0].channel_id = 'channel-id-1'; + runs[1].channel_id = 'channel-id-1'; + runs[2].channel_id = 'channel-id-1'; + runs[3].channel_id = 'channel-id-2'; + runs[4].channel_id = 'channel-id-2'; await operator.handlePlaybookRun({ prepareRecordsOnly: false, runs, }); + const channel1 = TestHelper.fakeChannel({id: 'channel-id-1', team_id: currentTeamId}); + const channel2 = TestHelper.fakeChannel({id: 'channel-id-2', team_id: 'other-team-id'}); + + await operator.handleChannel({ + channels: [channel1, channel2], + prepareRecordsOnly: false, + }); + const props = { componentId, }; @@ -95,6 +118,7 @@ describe('ParticipantPlaybooks Index', () => { expect(component).toBeTruthy(); expect(component.props.componentId).toBe('ParticipantPlaybooks'); expect(component.props.currentUserId).toBe(currentUserId); + expect(component.props.currentTeamId).toBe(currentTeamId); expect(component.props.cachedPlaybookRuns).toHaveLength(2); const runIds = component.props.cachedPlaybookRuns.map((r: PlaybookRunModel) => r.id); expect(runIds).toContain(runs[0].id); @@ -102,6 +126,17 @@ describe('ParticipantPlaybooks Index', () => { }); it('reacts to current user changes', async () => { + const currentTeamId = 'current-team-id'; + await operator.handleSystem({ + systems: [ + { + id: SYSTEM_IDENTIFIERS.CURRENT_TEAM_ID, + value: currentTeamId, + }, + ], + prepareRecordsOnly: false, + }); + const props = { componentId, }; @@ -112,11 +147,25 @@ describe('ParticipantPlaybooks Index', () => { runs[1].participant_ids = [otherUserId]; runs[2].participant_ids = [currentUserId]; runs[3].participant_ids = [otherUserId]; + + runs[0].channel_id = 'channel-id-1'; + runs[1].channel_id = 'channel-id-1'; + runs[2].channel_id = 'channel-id-1'; + runs[3].channel_id = 'channel-id-2'; + await operator.handlePlaybookRun({ prepareRecordsOnly: false, runs, }); + const channel1 = TestHelper.fakeChannel({id: 'channel-id-1', team_id: currentTeamId}); + const channel2 = TestHelper.fakeChannel({id: 'channel-id-2', team_id: 'other-team-id'}); + + await operator.handleChannel({ + channels: [channel1, channel2], + prepareRecordsOnly: false, + }); + const {getByTestId} = renderWithEverything( , {database}, @@ -124,6 +173,7 @@ describe('ParticipantPlaybooks Index', () => { let component = getByTestId('participant-playbooks'); expect(component.props.currentUserId).toBe(''); + expect(component.props.currentTeamId).toBe(currentTeamId); expect(component.props.cachedPlaybookRuns).toEqual([]); // Add current user to database @@ -162,10 +212,9 @@ describe('ParticipantPlaybooks Index', () => { await waitFor(() => { component = getByTestId('participant-playbooks'); expect(component.props.currentUserId).toBe(otherUserId); - expect(component.props.cachedPlaybookRuns).toHaveLength(2); + expect(component.props.cachedPlaybookRuns).toHaveLength(1); const runIds = component.props.cachedPlaybookRuns.map((r: PlaybookRunModel) => r.id); expect(runIds).toContain(runs[1].id); - expect(runIds).toContain(runs[3].id); }); }); }); diff --git a/app/products/playbooks/screens/participant_playbooks/index.ts b/app/products/playbooks/screens/participant_playbooks/index.ts index 5fc0d947a..e8003731c 100644 --- a/app/products/playbooks/screens/participant_playbooks/index.ts +++ b/app/products/playbooks/screens/participant_playbooks/index.ts @@ -2,11 +2,11 @@ // See LICENSE.txt for license information. import {withDatabase, withObservables} from '@nozbe/watermelondb/react'; -import {of} from 'rxjs'; +import {combineLatest, of} from 'rxjs'; import {switchMap} from 'rxjs/operators'; -import {queryPlaybookRunsByParticipant} from '@playbooks/database/queries/run'; -import {observeCurrentUserId} from '@queries/servers/system'; +import {queryPlaybookRunsByParticipantAndTeam} from '@playbooks/database/queries/run'; +import {observeCurrentTeamId, observeCurrentUserId} from '@queries/servers/system'; import ParticipantPlaybooks from './participant_playbooks'; @@ -16,12 +16,13 @@ type OwnProps = WithDatabaseArgs; const enhanced = withObservables([], ({database}: OwnProps) => { const currentUserId = observeCurrentUserId(database); - + const currentTeamId = observeCurrentTeamId(database); return { currentUserId, - cachedPlaybookRuns: currentUserId.pipe( - switchMap((userId) => - (userId ? queryPlaybookRunsByParticipant(database, userId).observe() : of([])), + currentTeamId, + cachedPlaybookRuns: combineLatest([currentUserId, currentTeamId]).pipe( + switchMap(([userId, teamId]) => + (userId ? queryPlaybookRunsByParticipantAndTeam(database, userId, teamId).observe() : of([])), ), ), }; diff --git a/app/products/playbooks/screens/participant_playbooks/participant_playbooks.test.tsx b/app/products/playbooks/screens/participant_playbooks/participant_playbooks.test.tsx index b41590dbe..3fa8b0861 100644 --- a/app/products/playbooks/screens/participant_playbooks/participant_playbooks.test.tsx +++ b/app/products/playbooks/screens/participant_playbooks/participant_playbooks.test.tsx @@ -39,6 +39,7 @@ describe('ParticipantPlaybooks', () => { currentUserId: 'test-user-id', componentId: 'ParticipantPlaybooks', cachedPlaybookRuns: [], + currentTeamId: 'test-team-id', }; } @@ -92,7 +93,7 @@ describe('ParticipantPlaybooks', () => { // Check that fetch was called await waitFor(() => { - expect(fetchPlaybookRunsPageForParticipant).toHaveBeenCalledWith('server-url', 'test-user-id', 0); + expect(fetchPlaybookRunsPageForParticipant).toHaveBeenCalledWith('server-url', 'test-user-id', 'test-team-id', 0); }); expect(runList).toHaveProp('loading', true); @@ -203,7 +204,7 @@ describe('ParticipantPlaybooks', () => { const {findByTestId} = renderWithEverything(, {database, serverUrl}); - expect(fetchPlaybookRunsPageForParticipant).toHaveBeenCalledWith('server-url', 'test-user-id', 0); + expect(fetchPlaybookRunsPageForParticipant).toHaveBeenCalledWith('server-url', 'test-user-id', 'test-team-id', 0); const runList = await findByTestId('run_list'); act(() => { @@ -212,7 +213,7 @@ describe('ParticipantPlaybooks', () => { }); await waitFor(() => { - expect(fetchPlaybookRunsPageForParticipant).toHaveBeenCalledWith('server-url', 'test-user-id', 1); + expect(fetchPlaybookRunsPageForParticipant).toHaveBeenCalledWith('server-url', 'test-user-id', 'test-team-id', 1); expect(runList.props.inProgressRuns).toHaveLength(20); }); @@ -222,7 +223,7 @@ describe('ParticipantPlaybooks', () => { }); await waitFor(() => { - expect(fetchPlaybookRunsPageForParticipant).toHaveBeenCalledWith('server-url', 'test-user-id', 2); + expect(fetchPlaybookRunsPageForParticipant).toHaveBeenCalledWith('server-url', 'test-user-id', 'test-team-id', 2); expect(runList.props.inProgressRuns).toHaveLength(30); }); }); @@ -233,7 +234,7 @@ describe('ParticipantPlaybooks', () => { jest.mocked(fetchPlaybookRunsPageForParticipant).mockResolvedValue({runs, hasMore: true}); const {findByTestId} = renderWithEverything(, {database, serverUrl}); - expect(fetchPlaybookRunsPageForParticipant).toHaveBeenCalledWith('server-url', 'test-user-id', 0); + expect(fetchPlaybookRunsPageForParticipant).toHaveBeenCalledWith('server-url', 'test-user-id', 'test-team-id', 0); jest.mocked(fetchPlaybookRunsPageForParticipant).mockResolvedValue({error: 'Network error'}); const runList = await findByTestId('run_list'); @@ -244,7 +245,7 @@ describe('ParticipantPlaybooks', () => { await waitFor(() => { expect(runList.props.fetching).toBe(false); - expect(fetchPlaybookRunsPageForParticipant).toHaveBeenCalledWith('server-url', 'test-user-id', 1); + expect(fetchPlaybookRunsPageForParticipant).toHaveBeenCalledWith('server-url', 'test-user-id', 'test-team-id', 1); expect(runList.props.inProgressRuns).toHaveLength(10); }); }); @@ -255,7 +256,7 @@ describe('ParticipantPlaybooks', () => { jest.mocked(fetchPlaybookRunsPageForParticipant).mockResolvedValue({runs, hasMore: true}); const {findByTestId} = renderWithEverything(, {database, serverUrl}); - expect(fetchPlaybookRunsPageForParticipant).toHaveBeenCalledWith('server-url', 'test-user-id', 0); + expect(fetchPlaybookRunsPageForParticipant).toHaveBeenCalledWith('server-url', 'test-user-id', 'test-team-id', 0); const runList = await findByTestId('run_list'); act(() => { @@ -351,7 +352,7 @@ describe('ParticipantPlaybooks', () => { jest.mocked(fetchPlaybookRunsPageForParticipant).mockResolvedValue({runs, hasMore: false}); const {findByTestId} = renderWithEverything(, {database, serverUrl}); - expect(fetchPlaybookRunsPageForParticipant).toHaveBeenCalledWith('server-url', 'test-user-id', 0); + expect(fetchPlaybookRunsPageForParticipant).toHaveBeenCalledWith('server-url', 'test-user-id', 'test-team-id', 0); const runList = await findByTestId('run_list'); act(() => { diff --git a/app/products/playbooks/screens/participant_playbooks/participant_playbooks.tsx b/app/products/playbooks/screens/participant_playbooks/participant_playbooks.tsx index 43e704f20..27cc21a5c 100644 --- a/app/products/playbooks/screens/participant_playbooks/participant_playbooks.tsx +++ b/app/products/playbooks/screens/participant_playbooks/participant_playbooks.tsx @@ -17,12 +17,14 @@ type Props = { currentUserId: string; componentId: AvailableScreens; cachedPlaybookRuns: PlaybookRunModel[]; + currentTeamId: string; }; const ParticipantPlaybooks = ({ currentUserId, componentId, cachedPlaybookRuns, + currentTeamId, }: Props) => { const serverUrl = useServerUrl(); @@ -51,7 +53,7 @@ const ParticipantPlaybooks = ({ setLoading(true); } - const {runs = [], hasMore: hasMoreFromResult = false, error} = await fetchPlaybookRunsPageForParticipant(serverUrl, currentUserId, page); + const {runs = [], hasMore: hasMoreFromResult = false, error} = await fetchPlaybookRunsPageForParticipant(serverUrl, currentUserId, currentTeamId, page); if (error) { // Fallback to database cache only for the first page @@ -76,7 +78,7 @@ const ParticipantPlaybooks = ({ } else { setLoading(false); } - }, [currentUserId, serverUrl, cachedPlaybookRuns]); + }, [currentUserId, currentTeamId, serverUrl, cachedPlaybookRuns]); const loadMore = useCallback(() => { if (!loadingMore && hasMore) {