From c3a27a6bcfd4f9d3e7330e72a6f795a155c6bcca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Vay=C3=A1?= Date: Mon, 22 Dec 2025 10:58:43 +0100 Subject: [PATCH] [MM-66610] Restore playbook run renaming (#9359) * restore playbook run renaming * move rename function to modal * fix rebase * prevent saving a non-trimmed value * fix style * remove tests that are no longer needed --- .../playbooks/actions/local/checklist.test.ts | 19 +++++++ .../playbooks/actions/local/checklist.ts | 2 +- .../playbooks/actions/local/run.test.ts | 16 ++++++ app/products/playbooks/actions/local/run.ts | 2 +- .../playbooks/screens/navigation.test.ts | 6 +-- app/products/playbooks/screens/navigation.ts | 4 +- .../playbook_run/playbook_run.test.tsx | 50 +++++++++++++++++-- .../screens/playbook_run/playbook_run.tsx | 47 ++++++++--------- .../rename_playbook_run_bottom_sheet.test.tsx | 35 ++++++++----- .../rename_playbook_run_bottom_sheet.tsx | 20 +++++--- 10 files changed, 146 insertions(+), 55 deletions(-) diff --git a/app/products/playbooks/actions/local/checklist.test.ts b/app/products/playbooks/actions/local/checklist.test.ts index 7007bd12a..a89bb6f3a 100644 --- a/app/products/playbooks/actions/local/checklist.test.ts +++ b/app/products/playbooks/actions/local/checklist.test.ts @@ -374,4 +374,23 @@ describe('renameChecklist', () => { expect(updated).toBeDefined(); expect(updated!.title).toBe(longTitle); }); + + it('should trim leading and trailing whitespace from title', async () => { + const runId = 'runid'; + const checklist = { + ...TestHelper.createPlaybookChecklist(runId, 0, 0), + run_id: runId, + order: 0, + }; + await operator.handlePlaybookChecklist({checklists: [checklist], prepareRecordsOnly: false}); + + const titleWithSpaces = ' Updated Checklist Title '; + const {data, error} = await renameChecklist(serverUrl, checklist.id, titleWithSpaces); + expect(error).toBeUndefined(); + expect(data).toBe(true); + + const updated = await getPlaybookChecklistById(operator.database, checklist.id); + expect(updated).toBeDefined(); + expect(updated!.title).toBe('Updated Checklist Title'); + }); }); diff --git a/app/products/playbooks/actions/local/checklist.ts b/app/products/playbooks/actions/local/checklist.ts index 8f3f54e44..3af9edf94 100644 --- a/app/products/playbooks/actions/local/checklist.ts +++ b/app/products/playbooks/actions/local/checklist.ts @@ -105,7 +105,7 @@ export async function renameChecklist(serverUrl: string, checklistId: string, ti await database.write(async () => { checklist.update((c) => { - c.title = title; + c.title = title.trim(); }); }); diff --git a/app/products/playbooks/actions/local/run.test.ts b/app/products/playbooks/actions/local/run.test.ts index bb63cd089..6daed48d7 100644 --- a/app/products/playbooks/actions/local/run.test.ts +++ b/app/products/playbooks/actions/local/run.test.ts @@ -253,4 +253,20 @@ describe('renamePlaybookRun', () => { const updatedRun = await database.get(PLAYBOOK_TABLES.PLAYBOOK_RUN).find(playbookRunId); expect(updatedRun.name).toBe(longName); }); + + it('should trim leading and trailing whitespace from name', async () => { + const runs = TestHelper.createPlaybookRuns(1, 0, 0); + await handlePlaybookRuns(serverUrl, runs, false, false); + + const playbookRunId = runs[0].id; + const nameWithSpaces = ' Updated Run Name '; + + const {data, error} = await renamePlaybookRun(serverUrl, playbookRunId, nameWithSpaces); + + expect(error).toBeUndefined(); + expect(data).toBe(true); + + const updatedRun = await database.get(PLAYBOOK_TABLES.PLAYBOOK_RUN).find(playbookRunId); + expect(updatedRun.name).toBe('Updated Run Name'); + }); }); diff --git a/app/products/playbooks/actions/local/run.ts b/app/products/playbooks/actions/local/run.ts index 302b58e66..5003052d0 100644 --- a/app/products/playbooks/actions/local/run.ts +++ b/app/products/playbooks/actions/local/run.ts @@ -56,7 +56,7 @@ export async function renamePlaybookRun(serverUrl: string, playbookRunId: string await database.write(async () => { run.update((r) => { - r.name = name; + r.name = name.trim(); }); }); diff --git a/app/products/playbooks/screens/navigation.test.ts b/app/products/playbooks/screens/navigation.test.ts index d91aff641..97bfb764e 100644 --- a/app/products/playbooks/screens/navigation.test.ts +++ b/app/products/playbooks/screens/navigation.test.ts @@ -545,9 +545,9 @@ describe('Playbooks Navigation', () => { describe('goToRenamePlaybookRun', () => { it('should navigate to rename playbook run screen with correct parameters', async () => { const currentTitle = 'Playbook Run Title'; - const onSave = jest.fn(); + const playbookRunId = 'run-id-123'; - await goToRenamePlaybookRun(mockIntl, Preferences.THEMES.denim, currentTitle, onSave); + await goToRenamePlaybookRun(mockIntl, Preferences.THEMES.denim, currentTitle, playbookRunId); expect(mockIntl.formatMessage).toHaveBeenCalledWith({ id: 'playbooks.playbook_run.rename.title', @@ -558,7 +558,7 @@ describe('Playbooks Navigation', () => { 'Rename playbook run', { currentTitle, - onSave, + playbookRunId, }, ); }); diff --git a/app/products/playbooks/screens/navigation.ts b/app/products/playbooks/screens/navigation.ts index 7c778d46d..983e0a1b8 100644 --- a/app/products/playbooks/screens/navigation.ts +++ b/app/products/playbooks/screens/navigation.ts @@ -116,12 +116,12 @@ export async function goToRenamePlaybookRun( intl: IntlShape, theme: Theme, currentTitle: string, - onSave: (newTitle: string) => void, + playbookRunId: string, ) { const title = intl.formatMessage({id: 'playbooks.playbook_run.rename.title', defaultMessage: 'Rename playbook run'}); goToScreen(Screens.PLAYBOOK_RENAME_RUN, title, { currentTitle, - onSave, + playbookRunId, }); } diff --git a/app/products/playbooks/screens/playbook_run/playbook_run.test.tsx b/app/products/playbooks/screens/playbook_run/playbook_run.test.tsx index d4973d79a..f6d1e9a77 100644 --- a/app/products/playbooks/screens/playbook_run/playbook_run.test.tsx +++ b/app/products/playbooks/screens/playbook_run/playbook_run.test.tsx @@ -1,10 +1,11 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import React, {type ComponentProps} from 'react'; +import React, {act, type ComponentProps} from 'react'; import {Alert} from 'react-native'; import UserChip from '@components/chips/user_chip'; +import CompassIcon from '@components/compass_icon'; import UserAvatarsStack from '@components/user_avatars_stack'; import {General} from '@constants'; import {useServerUrl} from '@context/server'; @@ -17,7 +18,7 @@ import {fireEvent, renderWithEverything, waitFor} from '@test/intl-test-helper'; import TestHelper from '@test/test_helper'; import {showPlaybookErrorSnackbar} from '@utils/snack_bar'; -import {goToSelectUser} from '../navigation'; +import {goToRenamePlaybookRun, goToSelectUser} from '../navigation'; import ChecklistList from './checklist_list'; import ErrorState from './error_state'; @@ -32,6 +33,14 @@ const serverUrl = 'some.server.url'; jest.mock('@utils/snack_bar'); +jest.mock('@components/compass_icon', () => ({ + __esModule: true, + default: jest.fn(), +})); +jest.mocked(CompassIcon).mockImplementation( + (props) => React.createElement('CompassIcon', {testID: 'compass-icon', ...props}) as any, // override the type since it is expecting a class component +); + jest.mock('@context/server'); jest.mocked(useServerUrl).mockReturnValue(serverUrl); @@ -66,12 +75,14 @@ jest.mocked(StatusUpdateIndicator).mockImplementation( ); jest.mock('../navigation', () => ({ + goToRenamePlaybookRun: jest.fn(), goToSelectUser: jest.fn(), })); jest.mock('@playbooks/actions/remote/runs', () => ({ - setOwner: jest.fn(), finishRun: jest.fn(), + renamePlaybookRun: jest.fn(), + setOwner: jest.fn(), })); jest.mock('@hooks/android_back_handler'); @@ -613,4 +624,37 @@ describe('PlaybookRun', () => { expect(queryByTestId('status-update-indicator')).toBeNull(); }); + it('renders edit icon when not read only', () => { + const props = getBaseProps(); + props.participants.push(TestHelper.fakeUserModel({id: props.currentUserId})); + const {getByTestId} = renderWithEverything(, {database}); + + const editIcon = getByTestId('playbook-run.edit-icon'); + expect(editIcon).toBeTruthy(); + }); + + it('does not render edit icon when read only', () => { + const props = getBaseProps(); + const {queryByTestId} = renderWithEverything(, {database}); + + const editIcon = queryByTestId('playbook-run.edit-icon'); + expect(editIcon).toBeNull(); + }); + + it('handles edit icon press', () => { + const props = getBaseProps(); + props.participants.push(TestHelper.fakeUserModel({id: props.currentUserId})); + const {getByTestId} = renderWithEverything(, {database}); + + const editIcon = getByTestId('playbook-run.edit-icon'); + act(() => { + fireEvent.press(editIcon); + }); + expect(goToRenamePlaybookRun).toHaveBeenCalledWith( + expect.anything(), // intl + expect.anything(), // theme + 'Test Playbook Run', + props.playbookRun!.id, + ); + }); }); diff --git a/app/products/playbooks/screens/playbook_run/playbook_run.tsx b/app/products/playbooks/screens/playbook_run/playbook_run.tsx index c45ce9064..dbe441abb 100644 --- a/app/products/playbooks/screens/playbook_run/playbook_run.tsx +++ b/app/products/playbooks/screens/playbook_run/playbook_run.tsx @@ -3,11 +3,12 @@ import React, {useCallback, useMemo} from 'react'; import {defineMessages, useIntl} from 'react-intl'; -import {View, Text, ScrollView, Alert} from 'react-native'; +import {View, Text, ScrollView, Alert, TouchableOpacity} from 'react-native'; import {useSafeAreaInsets} from 'react-native-safe-area-context'; import Button from '@components/button'; import UserChip from '@components/chips/user_chip'; +import CompassIcon from '@components/compass_icon'; import Markdown from '@components/markdown'; import Tag from '@components/tag'; import UserAvatarsStack from '@components/user_avatars_stack'; @@ -22,7 +23,7 @@ import {showPlaybookErrorSnackbar} from '@utils/snack_bar'; import {makeStyleSheetFromTheme, changeOpacity} from '@utils/theme'; import {typography} from '@utils/typography'; -import {goToSelectUser} from '../navigation'; +import {goToRenamePlaybookRun, goToSelectUser} from '../navigation'; import ChecklistList from './checklist_list'; import ErrorState from './error_state'; @@ -258,25 +259,13 @@ export default function PlaybookRun({ ); }, [handleSelectOwner, intl, owner, participants, playbookRun?.name, theme]); - // this will be back once there is a rename function on the server side - // const handleRename = useCallback(async (newName: string) => { - // if (!playbookRun) { - // return; - // } + const handleEditPress = useCallback(() => { + if (!playbookRun) { + return; + } - // const res = await renamePlaybookRun(serverUrl, playbookRun.id, newName); - // if (res.error) { - // showPlaybookErrorSnackbar(); - // } - // }, [playbookRun, serverUrl]); - - // const handleEditPress = useCallback(() => { - // if (!playbookRun) { - // return; - // } - - // goToRenamePlaybookRun(intl, theme, playbookRun.name, handleRename); - // }, [intl, theme, playbookRun, handleRename]); + goToRenamePlaybookRun(intl, theme, playbookRun.name, playbookRun.id); + }, [intl, theme, playbookRun]); const handleFinishRun = useCallback(() => { if (!playbookRun) { @@ -333,13 +322,17 @@ export default function PlaybookRun({ {playbookRun.name} - {/* This will be back once there is a rename function on the server side - - - */} + {!readOnly && ( + + + + )} {isFinished && ( jest.fn()); jest.mock('@managers/security_manager', () => ({ getShieldScreenId: jest.fn((id) => `shield-${id}`), })); +jest.mock('@playbooks/actions/remote/runs', () => ({ + renamePlaybookRun: jest.fn(), +})); +jest.mock('@utils/snack_bar', () => ({ + showPlaybookErrorSnackbar: jest.fn(), +})); +jest.mock('@context/server', () => ({ + useServerUrl: jest.fn(() => 'some.server.url'), +})); describe('RenamePlaybookRunBottomSheet', () => { const componentId = 'test-component-id' as any; const currentTitle = 'Original Playbook Run'; - const mockOnSave = jest.fn(); + const playbookRunId = 'run-id-123'; const mockRightButton = { id: 'save-playbook-run-name', @@ -47,6 +57,7 @@ describe('RenamePlaybookRunBottomSheet', () => { beforeEach(() => { jest.clearAllMocks(); jest.mocked(buildNavigationButton).mockReturnValue(mockRightButton as any); + jest.mocked(renamePlaybookRun).mockResolvedValue({data: true}); jest.mocked(useNavButtonPressed).mockImplementation((buttonId, compId, callback) => { // Simulate button press when buttonId matches if (buttonId === 'save-playbook-run-name' && compId === componentId) { @@ -64,7 +75,7 @@ describe('RenamePlaybookRunBottomSheet', () => { return { componentId, currentTitle, - onSave: mockOnSave, + playbookRunId, }; } @@ -248,7 +259,7 @@ describe('RenamePlaybookRunBottomSheet', () => { }); }); - it('should call onSave and close when save button is pressed with valid title', () => { + it('should call renamePlaybookRun and close when save button is pressed with valid title', async () => { const props = getBaseProps(); const {getByTestId} = renderWithIntlAndTheme(); @@ -262,16 +273,16 @@ describe('RenamePlaybookRunBottomSheet', () => { // Trigger save button press const saveCallback = (useNavButtonPressed as any).lastCallback; - act(() => { - saveCallback(); + await act(async () => { + await saveCallback(); }); - expect(mockOnSave).toHaveBeenCalledWith(newTitle); + expect(renamePlaybookRun).toHaveBeenCalledWith('some.server.url', playbookRunId, newTitle); expect(Keyboard.dismiss).toHaveBeenCalled(); expect(popTopScreen).toHaveBeenCalledWith(componentId); }); - it('should trim title when saving', () => { + it('should trim title when saving', async () => { const props = getBaseProps(); const {getByTestId} = renderWithIntlAndTheme(); @@ -285,12 +296,12 @@ describe('RenamePlaybookRunBottomSheet', () => { // Trigger save button press const saveCallback = (useNavButtonPressed as any).lastCallback; - act(() => { - saveCallback(); + await act(async () => { + await saveCallback(); }); - expect(mockOnSave).toHaveBeenCalledWith('New Playbook Run Name'); - expect(mockOnSave).not.toHaveBeenCalledWith(titleWithSpaces); + expect(renamePlaybookRun).toHaveBeenCalledWith('some.server.url', playbookRunId, 'New Playbook Run Name'); + expect(renamePlaybookRun).not.toHaveBeenCalledWith('some.server.url', playbookRunId, titleWithSpaces); }); it('should close when Android back button is pressed', () => { @@ -305,7 +316,7 @@ describe('RenamePlaybookRunBottomSheet', () => { expect(Keyboard.dismiss).toHaveBeenCalled(); expect(popTopScreen).toHaveBeenCalledWith(componentId); - expect(mockOnSave).not.toHaveBeenCalled(); + expect(renamePlaybookRun).not.toHaveBeenCalled(); }); it('should update navigation button when canSave changes', () => { diff --git a/app/products/playbooks/screens/playbook_run/rename_playbook_run_bottom_sheet.tsx b/app/products/playbooks/screens/playbook_run/rename_playbook_run_bottom_sheet.tsx index f90bcda10..604be735e 100644 --- a/app/products/playbooks/screens/playbook_run/rename_playbook_run_bottom_sheet.tsx +++ b/app/products/playbooks/screens/playbook_run/rename_playbook_run_bottom_sheet.tsx @@ -6,18 +6,21 @@ import {useIntl} from 'react-intl'; import {Keyboard, StyleSheet, View} from 'react-native'; import FloatingTextInput from '@components/floating_input/floating_text_input_label'; +import {useServerUrl} from '@context/server'; import {useTheme} from '@context/theme'; import useAndroidHardwareBackHandler from '@hooks/android_back_handler'; import useNavButtonPressed from '@hooks/navigation_button_pressed'; import SecurityManager from '@managers/security_manager'; +import {renamePlaybookRun} from '@playbooks/actions/remote/runs'; import {buildNavigationButton, popTopScreen, setButtons} from '@screens/navigation'; +import {showPlaybookErrorSnackbar} from '@utils/snack_bar'; import type {AvailableScreens} from '@typings/screens/navigation'; type Props = { componentId: AvailableScreens; currentTitle: string; - onSave: (newTitle: string) => void; + playbookRunId: string; } const SAVE_BUTTON_ID = 'save-playbook-run-name'; @@ -38,11 +41,12 @@ const styles = StyleSheet.create({ const RenamePlaybookRunBottomSheet = ({ componentId, currentTitle, - onSave, + playbookRunId, }: Props) => { const intl = useIntl(); const {formatMessage} = intl; const theme = useTheme(); + const serverUrl = useServerUrl(); const [title, setTitle] = useState(currentTitle); @@ -72,12 +76,16 @@ const RenamePlaybookRunBottomSheet = ({ close(componentId); }, [componentId]); - const handleSave = useCallback(() => { + const handleSave = useCallback(async () => { if (canSave) { - onSave(title.trim()); - close(componentId); + const res = await renamePlaybookRun(serverUrl, playbookRunId, title.trim()); + if (res.error) { + showPlaybookErrorSnackbar(); + } else { + close(componentId); + } } - }, [canSave, title, componentId, onSave]); + }, [canSave, title, componentId, serverUrl, playbookRunId]); useNavButtonPressed(SAVE_BUTTON_ID, componentId, handleSave, [handleSave]); useAndroidHardwareBackHandler(componentId, handleClose);