From 93b749eb793018581b565448490a9082e57ff67d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Vay=C3=A1?= Date: Wed, 19 Nov 2025 16:14:03 +0100 Subject: [PATCH] [MM-66690][MM-66691] Checklist improvements (#9294) * fix no response + handle error * address comment review * add a screen to allow user to name the checklist * handle local update errors * can save as a memoized item * change touchableopacity into a button * fix typo * memoize canSave --- .../actions/remote/checklist.test.ts | 33 +++- .../playbooks/actions/remote/checklist.ts | 11 +- .../playbooks/actions/remote/runs.test.ts | 11 ++ app/products/playbooks/actions/remote/runs.ts | 5 +- app/products/playbooks/constants/screens.ts | 2 + .../create_quick_checklist.tsx | 182 ++++++++++++++++++ .../screens/create_quick_checklist/index.ts | 7 + app/products/playbooks/screens/index.test.tsx | 7 + app/products/playbooks/screens/index.tsx | 2 + app/products/playbooks/screens/navigation.ts | 20 ++ .../add_checklist_item_bottom_sheet.tsx | 15 +- .../playbook_run/checklist/checklist.test.tsx | 62 ++++++ .../playbook_run/checklist/checklist.tsx | 42 ++-- .../rename_playbook_run_bottom_sheet.tsx | 15 +- .../status_update_indicator.test.tsx | 2 +- app/screens/channel/header/header.test.tsx | 37 ++-- app/screens/channel/header/header.tsx | 41 ++-- assets/base/i18n/en.json | 2 + 18 files changed, 394 insertions(+), 102 deletions(-) create mode 100644 app/products/playbooks/screens/create_quick_checklist/create_quick_checklist.tsx create mode 100644 app/products/playbooks/screens/create_quick_checklist/index.ts diff --git a/app/products/playbooks/actions/remote/checklist.test.ts b/app/products/playbooks/actions/remote/checklist.test.ts index df83305ef..ab75f2891 100644 --- a/app/products/playbooks/actions/remote/checklist.test.ts +++ b/app/products/playbooks/actions/remote/checklist.test.ts @@ -11,6 +11,7 @@ import { setDueDate as localSetDueDate, renameChecklist as localRenameChecklist, } from '@playbooks/actions/local/checklist'; +import {handlePlaybookRuns} from '@playbooks/actions/local/run'; import { updateChecklistItem, @@ -42,9 +43,11 @@ const mockClient = { setDueDate: jest.fn(), renameChecklist: jest.fn(), addChecklistItem: jest.fn(), + fetchPlaybookRun: jest.fn(), }; jest.mock('@playbooks/actions/local/checklist'); +jest.mock('@playbooks/actions/local/run'); const throwFunc = () => { throw Error('error'); @@ -353,7 +356,7 @@ describe('checklist', () => { it('should handle local DB update failure', async () => { mockClient.renameChecklist.mockResolvedValueOnce({}); - (localRenameChecklist as jest.Mock).mockRejectedValueOnce(new Error('DB error')); + jest.mocked(localRenameChecklist).mockResolvedValueOnce({error: 'DB error'}); const result = await renameChecklist(serverUrl, playbookRunId, checklistNumber, checklistId, newTitle); expect(result).toBeDefined(); @@ -364,6 +367,7 @@ describe('checklist', () => { it('should rename checklist successfully', async () => { mockClient.renameChecklist.mockResolvedValueOnce({}); + jest.mocked(localRenameChecklist).mockResolvedValueOnce({data: true}); const result = await renameChecklist(serverUrl, playbookRunId, checklistNumber, checklistId, newTitle); expect(result).toBeDefined(); @@ -375,6 +379,7 @@ describe('checklist', () => { it('should rename checklist with empty title', async () => { mockClient.renameChecklist.mockResolvedValueOnce({}); + jest.mocked(localRenameChecklist).mockResolvedValueOnce({data: true}); const emptyTitle = ''; const result = await renameChecklist(serverUrl, playbookRunId, checklistNumber, checklistId, emptyTitle); @@ -394,7 +399,7 @@ describe('checklist', () => { const result = await addChecklistItem(serverUrl, playbookRunId, checklistNumber, title); expect(result).toBeDefined(); - expect(result.error).toBeDefined(); + expect('error' in result && result.error).toBeDefined(); }); it('should handle API exception', async () => { @@ -402,29 +407,39 @@ describe('checklist', () => { const result = await addChecklistItem(serverUrl, playbookRunId, checklistNumber, title); expect(result).toBeDefined(); - expect(result.error).toBeDefined(); + expect('error' in result && result.error).toBeDefined(); expect(mockClient.addChecklistItem).toHaveBeenCalledWith(playbookRunId, checklistNumber, title); }); it('should add checklist item successfully', async () => { - mockClient.addChecklistItem.mockResolvedValueOnce({}); + const mockRun = {id: playbookRunId, checklists: []}; + mockClient.addChecklistItem.mockResolvedValueOnce(undefined); + mockClient.fetchPlaybookRun.mockResolvedValueOnce(mockRun); + (handlePlaybookRuns as jest.Mock).mockResolvedValueOnce({data: true}); const result = await addChecklistItem(serverUrl, playbookRunId, checklistNumber, title); expect(result).toBeDefined(); - expect(result.error).toBeUndefined(); - expect(result.data).toBe(true); + expect('error' in result ? result.error : undefined).toBeUndefined(); + expect('data' in result && result.data).toBe(true); expect(mockClient.addChecklistItem).toHaveBeenCalledWith(playbookRunId, checklistNumber, title); + expect(mockClient.fetchPlaybookRun).toHaveBeenCalledWith(playbookRunId); + expect(handlePlaybookRuns).toHaveBeenCalledWith(serverUrl, [mockRun], false, true); }); it('should add checklist item with empty title', async () => { - mockClient.addChecklistItem.mockResolvedValueOnce({}); + const mockRun = {id: playbookRunId, checklists: []}; const emptyTitle = ''; + mockClient.addChecklistItem.mockResolvedValueOnce(undefined); + mockClient.fetchPlaybookRun.mockResolvedValueOnce(mockRun); + (handlePlaybookRuns as jest.Mock).mockResolvedValueOnce({data: true}); const result = await addChecklistItem(serverUrl, playbookRunId, checklistNumber, emptyTitle); expect(result).toBeDefined(); - expect(result.error).toBeUndefined(); - expect(result.data).toBe(true); + expect('error' in result ? result.error : undefined).toBeUndefined(); + expect('data' in result && result.data).toBe(true); expect(mockClient.addChecklistItem).toHaveBeenCalledWith(playbookRunId, checklistNumber, emptyTitle); + expect(mockClient.fetchPlaybookRun).toHaveBeenCalledWith(playbookRunId); + expect(handlePlaybookRuns).toHaveBeenCalledWith(serverUrl, [mockRun], false, true); }); }); }); diff --git a/app/products/playbooks/actions/remote/checklist.ts b/app/products/playbooks/actions/remote/checklist.ts index 272ea4a23..3b5bc6176 100644 --- a/app/products/playbooks/actions/remote/checklist.ts +++ b/app/products/playbooks/actions/remote/checklist.ts @@ -11,6 +11,7 @@ import { setDueDate as localSetDueDate, renameChecklist as localRenameChecklist, } from '@playbooks/actions/local/checklist'; +import {handlePlaybookRuns} from '@playbooks/actions/local/run'; import {getFullErrorMessage} from '@utils/errors'; import {logDebug} from '@utils/log'; @@ -172,8 +173,8 @@ export const renameChecklist = async ( await client.renameChecklist(playbookRunId, checklistNumber, newTitle); // Update local database - await localRenameChecklist(serverUrl, checklistId, newTitle); - return {data: true}; + const result = await localRenameChecklist(serverUrl, checklistId, newTitle); + return result.error ? result : {data: true}; } catch (error) { logDebug('error on renameChecklist', getFullErrorMessage(error)); forceLogoutIfNecessary(serverUrl, error); @@ -190,7 +191,11 @@ export const addChecklistItem = async ( try { const client = NetworkManager.getClient(serverUrl); await client.addChecklistItem(playbookRunId, checklistNumber, title); - return {data: true}; + + // Fetch and sync the entire run to get the created item with server-generated ID + const run = await client.fetchPlaybookRun(playbookRunId); + const result = await handlePlaybookRuns(serverUrl, [run], false, true); + return result.error ? result : {data: true}; } catch (error) { logDebug('error on addChecklistItem', getFullErrorMessage(error)); forceLogoutIfNecessary(serverUrl, error); diff --git a/app/products/playbooks/actions/remote/runs.test.ts b/app/products/playbooks/actions/remote/runs.test.ts index 44b18cad7..311f7c925 100644 --- a/app/products/playbooks/actions/remote/runs.test.ts +++ b/app/products/playbooks/actions/remote/runs.test.ts @@ -753,6 +753,17 @@ describe('renamePlaybookRun', () => { expect(localRenamePlaybookRun).not.toHaveBeenCalled(); }); + it('should handle local DB update failure', async () => { + mockClient.patchPlaybookRun.mockResolvedValueOnce(undefined); + jest.mocked(localRenamePlaybookRun).mockResolvedValueOnce({error: 'DB error'}); + + const result = await renamePlaybookRun(serverUrl, playbookRunId, newName); + expect(result).toBeDefined(); + expect(result.error).toBeDefined(); + expect(mockClient.patchPlaybookRun).toHaveBeenCalledWith(playbookRunId, {name: newName}); + expect(localRenamePlaybookRun).toHaveBeenCalledWith(serverUrl, playbookRunId, newName); + }); + it('should rename playbook run successfully', async () => { mockClient.patchPlaybookRun.mockResolvedValueOnce(undefined); diff --git a/app/products/playbooks/actions/remote/runs.ts b/app/products/playbooks/actions/remote/runs.ts index c7634adae..c4bd245ed 100644 --- a/app/products/playbooks/actions/remote/runs.ts +++ b/app/products/playbooks/actions/remote/runs.ts @@ -141,8 +141,9 @@ export const renamePlaybookRun = async (serverUrl: string, playbookRunId: string const client = NetworkManager.getClient(serverUrl); await client.patchPlaybookRun(playbookRunId, {name: newName}); - await localRenamePlaybookRun(serverUrl, playbookRunId, newName); - return {data: true}; + // Update local database + const result = await localRenamePlaybookRun(serverUrl, playbookRunId, newName); + return result.error ? result : {data: true}; } catch (error) { logDebug('error on renamePlaybookRun', getFullErrorMessage(error)); forceLogoutIfNecessary(serverUrl, error); diff --git a/app/products/playbooks/constants/screens.ts b/app/products/playbooks/constants/screens.ts index 7e69705cf..b33567a68 100644 --- a/app/products/playbooks/constants/screens.ts +++ b/app/products/playbooks/constants/screens.ts @@ -13,6 +13,7 @@ export const PLAYBOOK_SELECT_USER = 'PlaybookSelectUser'; export const PLAYBOOKS_SELECT_DATE = 'PlaybooksSelectDate'; export const PLAYBOOKS_SELECT_PLAYBOOK = 'PlaybooksSelectPlaybook'; export const PLAYBOOKS_START_A_RUN = 'PlaybooksStartARun'; +export const PLAYBOOKS_CREATE_QUICK_CHECKLIST = 'PlaybooksCreateQuickChecklist'; export default { PLAYBOOKS_RUNS, @@ -27,4 +28,5 @@ export default { PLAYBOOKS_SELECT_DATE, PLAYBOOKS_SELECT_PLAYBOOK, PLAYBOOKS_START_A_RUN, + PLAYBOOKS_CREATE_QUICK_CHECKLIST, } as const; diff --git a/app/products/playbooks/screens/create_quick_checklist/create_quick_checklist.tsx b/app/products/playbooks/screens/create_quick_checklist/create_quick_checklist.tsx new file mode 100644 index 000000000..97c1f369c --- /dev/null +++ b/app/products/playbooks/screens/create_quick_checklist/create_quick_checklist.tsx @@ -0,0 +1,182 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React, {useState, useCallback, useEffect} from 'react'; +import {useIntl, type IntlShape} from 'react-intl'; +import {Keyboard, ScrollView} from 'react-native'; +import {SafeAreaView} from 'react-native-safe-area-context'; + +import CompassIcon from '@components/compass_icon'; +import FloatingTextInput from '@components/floating_input/floating_text_input_label'; +import {useTheme} from '@context/theme'; +import useAndroidHardwareBackHandler from '@hooks/android_back_handler'; +import useNavButtonPressed from '@hooks/navigation_button_pressed'; +import {usePreventDoubleTap} from '@hooks/utils'; +import {createPlaybookRun, fetchPlaybookRunsForChannel} from '@playbooks/actions/remote/runs'; +import {goToPlaybookRun} from '@playbooks/screens/navigation'; +import {popTopScreen, setButtons} from '@screens/navigation'; +import {getFullErrorMessage} from '@utils/errors'; +import {logError} from '@utils/log'; +import {showPlaybookErrorSnackbar} from '@utils/snack_bar'; +import {makeStyleSheetFromTheme} from '@utils/theme'; + +import type {AvailableScreens} from '@typings/screens/navigation'; +import type {OptionsTopBarButton} from 'react-native-navigation'; + +type Props = { + componentId: AvailableScreens; + channelId: string; + channelName: string; + currentUserId: string; + currentTeamId: string; + serverUrl: string; +} + +const getStyleSheet = makeStyleSheetFromTheme((theme: Theme) => { + return { + container: { + flex: 1, + backgroundColor: theme.centerChannelBg, + }, + content: { + flex: 1, + paddingHorizontal: 20, + paddingVertical: 24, + }, + contentContainer: { + gap: 16, + }, + }; +}); + +const CLOSE_BUTTON_ID = 'close-create-quick-checklist'; +const CREATE_BUTTON_ID = 'create-quick-checklist'; + +async function makeLeftButton(theme: Theme): Promise { + return { + id: CLOSE_BUTTON_ID, + icon: await CompassIcon.getImageSource('close', 24, theme.sidebarHeaderTextColor), + testID: 'create_quick_checklist.close.button', + }; +} + +function makeRightButton(theme: Theme, intl: IntlShape, enabled: boolean): OptionsTopBarButton { + return { + color: theme.sidebarHeaderTextColor, + id: CREATE_BUTTON_ID, + text: intl.formatMessage({id: 'mobile.create_channel', defaultMessage: 'Create'}), + showAsAction: 'always', + testID: 'create_quick_checklist.create.button', + enabled, + }; +} + +function CreateQuickChecklist({ + componentId, + channelId, + channelName, + currentUserId, + currentTeamId, + serverUrl, +}: Props) { + const theme = useTheme(); + const intl = useIntl(); + const styles = getStyleSheet(theme); + + const [checklistName, setChecklistName] = useState(`${channelName} Checklist`); + const [description, setDescription] = useState(''); + const canSave = Boolean(checklistName.trim()); + + const handleCreate = usePreventDoubleTap(useCallback(async () => { + const res = await createPlaybookRun( + serverUrl, + '', // empty playbook_id for standalone checklist + currentUserId, + currentTeamId, + checklistName.trim(), + description.trim(), + channelId, + ); + + if (res.error || !res.data) { + logError('error on createPlaybookRun', getFullErrorMessage(res.error)); + showPlaybookErrorSnackbar(); + return; + } + + // Pop the create screen first, then navigate to the run + // This ensures the back button goes back to the channel, not the create screen + await popTopScreen(componentId); + await fetchPlaybookRunsForChannel(serverUrl, channelId); + await goToPlaybookRun(intl, res.data.id); + }, [serverUrl, currentUserId, currentTeamId, checklistName, description, channelId, componentId, intl])); + + useEffect(() => { + async function asyncWrapper() { + const leftButton = await makeLeftButton(theme); + const rightButton = makeRightButton(theme, intl, canSave); + + setButtons( + componentId, + { + leftButtons: [leftButton], + rightButtons: [rightButton], + }, + ); + } + + asyncWrapper(); + }, [componentId, theme, intl, canSave]); + + const close = useCallback(() => { + Keyboard.dismiss(); + popTopScreen(componentId); + }, [componentId]); + + useNavButtonPressed(CREATE_BUTTON_ID, componentId, handleCreate, [handleCreate]); + useNavButtonPressed(CLOSE_BUTTON_ID, componentId, close, [close]); + useAndroidHardwareBackHandler(componentId, close); + + return ( + + + + + + + ); +} + +export default CreateQuickChecklist; + diff --git a/app/products/playbooks/screens/create_quick_checklist/index.ts b/app/products/playbooks/screens/create_quick_checklist/index.ts new file mode 100644 index 000000000..ebe23149d --- /dev/null +++ b/app/products/playbooks/screens/create_quick_checklist/index.ts @@ -0,0 +1,7 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import CreateQuickChecklist from './create_quick_checklist'; + +export default CreateQuickChecklist; + diff --git a/app/products/playbooks/screens/index.test.tsx b/app/products/playbooks/screens/index.test.tsx index 28345c703..1a76d331d 100644 --- a/app/products/playbooks/screens/index.test.tsx +++ b/app/products/playbooks/screens/index.test.tsx @@ -7,6 +7,7 @@ import {withServerDatabase} from '@database/components'; import Screens from '@playbooks/constants/screens'; import {render} from '@test/intl-test-helper'; +import CreateQuickChecklist from './create_quick_checklist'; import EditCommand from './edit_command'; import ParticipantPlaybooks from './participant_playbooks'; import PlaybookRun from './playbook_run'; @@ -65,6 +66,12 @@ jest.mock('@playbooks/screens/start_a_run', () => ({ })); jest.mocked(StartARun).mockImplementation((props) => {Screens.PLAYBOOKS_START_A_RUN}); +jest.mock('@playbooks/screens/create_quick_checklist', () => ({ + __esModule: true, + default: jest.fn(), +})); +jest.mocked(CreateQuickChecklist).mockImplementation((props) => {Screens.PLAYBOOKS_CREATE_QUICK_CHECKLIST}); + jest.mock('@playbooks/screens/select_playbook', () => ({ __esModule: true, default: jest.fn(), diff --git a/app/products/playbooks/screens/index.tsx b/app/products/playbooks/screens/index.tsx index 21975c146..aeb24e164 100644 --- a/app/products/playbooks/screens/index.tsx +++ b/app/products/playbooks/screens/index.tsx @@ -30,6 +30,8 @@ export function loadPlaybooksScreen(screenName: string | number) { return withServerDatabase(require('@playbooks/screens/select_playbook').default); case Screens.PLAYBOOKS_START_A_RUN: return withServerDatabase(require('@playbooks/screens/start_a_run').default); + case Screens.PLAYBOOKS_CREATE_QUICK_CHECKLIST: + return withServerDatabase(require('@playbooks/screens/create_quick_checklist').default); default: return undefined; } diff --git a/app/products/playbooks/screens/navigation.ts b/app/products/playbooks/screens/navigation.ts index d6e3c2c45..7c778d46d 100644 --- a/app/products/playbooks/screens/navigation.ts +++ b/app/products/playbooks/screens/navigation.ts @@ -187,3 +187,23 @@ export async function goToStartARun(intl: IntlShape, theme: Theme, playbook: Pla }, }); } + +export function goToCreateQuickChecklist( + intl: IntlShape, + channelId: string, + channelName: string, + currentUserId: string, + currentTeamId: string, + serverUrl: string, +) { + const screen = Screens.PLAYBOOKS_CREATE_QUICK_CHECKLIST; + const title = intl.formatMessage({id: 'mobile.playbook.create_checklist', defaultMessage: 'Create Checklist'}); + + goToScreen(screen, title, { + channelId, + channelName, + currentUserId, + currentTeamId, + serverUrl, + }, {}); // Empty options, no subtitle needed +} diff --git a/app/products/playbooks/screens/playbook_run/checklist/add_checklist_item_bottom_sheet.tsx b/app/products/playbooks/screens/playbook_run/checklist/add_checklist_item_bottom_sheet.tsx index 8a8c4bae2..53e17e533 100644 --- a/app/products/playbooks/screens/playbook_run/checklist/add_checklist_item_bottom_sheet.tsx +++ b/app/products/playbooks/screens/playbook_run/checklist/add_checklist_item_bottom_sheet.tsx @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import React, {useCallback, useEffect, useState} from 'react'; +import React, {useCallback, useEffect, useMemo, useState} from 'react'; import {useIntl} from 'react-intl'; import {Keyboard, StyleSheet, View} from 'react-native'; @@ -43,7 +43,10 @@ const AddChecklistItemBottomSheet = ({ const theme = useTheme(); const [title, setTitle] = useState(''); - const [canSave, setCanSave] = useState(false); + + const canSave = useMemo(() => { + return Boolean(title.trim().length); + }, [title]); const rightButton = React.useMemo(() => { const base = buildNavigationButton( @@ -63,20 +66,16 @@ const AddChecklistItemBottomSheet = ({ }); }, [rightButton, componentId]); - useEffect(() => { - setCanSave(title.trim().length > 0); - }, [title]); - const handleClose = useCallback(() => { close(componentId); }, [componentId]); const handleSave = useCallback(() => { - if (title.trim().length > 0) { + if (canSave) { onSave(title.trim()); close(componentId); } - }, [title, componentId, onSave]); + }, [canSave, title, componentId, onSave]); useNavButtonPressed(SAVE_BUTTON_ID, componentId, handleSave, [handleSave]); useAndroidHardwareBackHandler(componentId, handleClose); diff --git a/app/products/playbooks/screens/playbook_run/checklist/checklist.test.tsx b/app/products/playbooks/screens/playbook_run/checklist/checklist.test.tsx index 13147020f..e5d509741 100644 --- a/app/products/playbooks/screens/playbook_run/checklist/checklist.test.tsx +++ b/app/products/playbooks/screens/playbook_run/checklist/checklist.test.tsx @@ -4,7 +4,9 @@ import {act, fireEvent, waitFor, within} from '@testing-library/react-native'; import React, {type ComponentProps} from 'react'; +import Button from '@components/button'; import ProgressBar from '@playbooks/components/progress_bar'; +import {goToAddChecklistItem} from '@playbooks/screens/navigation'; import {renderWithIntl} from '@test/intl-test-helper'; import TestHelper from '@test/test_helper'; @@ -21,6 +23,19 @@ jest.mocked(ProgressBar).mockImplementation( (props) => React.createElement('ProgressBar', {testID: 'progress-bar-component', ...props}), ); +jest.mock('@components/button', () => ({ + __esModule: true, + default: jest.fn(), +})); +jest.mocked(Button).mockImplementation( + (props) => React.createElement('Button', {testID: props.testID, ...props}), +); + +jest.mock('@playbooks/screens/navigation', () => ({ + goToRenameChecklist: jest.fn(), + goToAddChecklistItem: jest.fn(), +})); + describe('Checklist', () => { const mockChecklist = TestHelper.fakePlaybookChecklistModel({ id: 'checklist-1', @@ -228,4 +243,51 @@ describe('Checklist', () => { expect(progressBar.props.progress).toBe(100); expect(progressBar.props.isActive).toBe(false); }); + + it('renders add button when not finished and is participant', () => { + const props = getBaseProps(); + props.isFinished = false; + props.isParticipant = true; + + const {getByTestId} = renderWithIntl(); + + const addButton = getByTestId('add-checklist-item-button'); + expect(addButton).toBeTruthy(); + expect(addButton.props.text).toBe('New'); + expect(addButton.props.iconName).toBe('plus'); + }); + + it('does not render add button when finished', () => { + const props = getBaseProps(); + props.isFinished = true; + props.isParticipant = true; + + const {queryByTestId} = renderWithIntl(); + + expect(queryByTestId('add-checklist-item-button')).toBeNull(); + }); + + it('does not render add button when not participant', () => { + const props = getBaseProps(); + props.isFinished = false; + props.isParticipant = false; + + const {queryByTestId} = renderWithIntl(); + + expect(queryByTestId('add-checklist-item-button')).toBeNull(); + }); + + it('calls goToAddChecklistItem when add button is pressed', () => { + jest.mocked(goToAddChecklistItem).mockClear(); + const props = getBaseProps(); + props.isFinished = false; + props.isParticipant = true; + + const {getByTestId} = renderWithIntl(); + + const addButton = getByTestId('add-checklist-item-button'); + fireEvent.press(addButton); + + expect(goToAddChecklistItem).toHaveBeenCalled(); + }); }); diff --git a/app/products/playbooks/screens/playbook_run/checklist/checklist.tsx b/app/products/playbooks/screens/playbook_run/checklist/checklist.tsx index 19d26a32f..c4ffef71a 100644 --- a/app/products/playbooks/screens/playbook_run/checklist/checklist.tsx +++ b/app/products/playbooks/screens/playbook_run/checklist/checklist.tsx @@ -3,9 +3,10 @@ import React, {useCallback, useMemo, useState} from 'react'; import {useIntl} from 'react-intl'; -import {View, Text, TouchableOpacity, type LayoutChangeEvent, useWindowDimensions, StyleSheet} from 'react-native'; +import {View, Text, TouchableOpacity, type GestureResponderEvent, type LayoutChangeEvent, useWindowDimensions, StyleSheet} from 'react-native'; import Animated, {useAnimatedStyle, useSharedValue, withTiming} from 'react-native-reanimated'; +import Button from '@components/button'; import CompassIcon from '@components/compass_icon'; import {useServerUrl} from '@context/server'; import {useTheme} from '@context/theme'; @@ -13,6 +14,9 @@ import {renameChecklist, addChecklistItem} from '@playbooks/actions/remote/check import ProgressBar from '@playbooks/components/progress_bar'; import {goToRenameChecklist, goToAddChecklistItem} from '@playbooks/screens/navigation'; import {getChecklistProgress} from '@playbooks/utils/progress'; +import {getFullErrorMessage} from '@utils/errors'; +import {logError} from '@utils/log'; +import {showPlaybookErrorSnackbar} from '@utils/snack_bar'; import {makeStyleSheetFromTheme, changeOpacity} from '@utils/theme'; import {typography} from '@utils/typography'; @@ -140,17 +144,25 @@ const Checklist = ({ setExpanded((prev) => !prev); }, []); - const handleRename = useCallback((newTitle: string) => { - renameChecklist(serverUrl, playbookRunId, checklistNumber, checklist.id, newTitle); + const handleRename = useCallback(async (newTitle: string) => { + const res = await renameChecklist(serverUrl, playbookRunId, checklistNumber, checklist.id, newTitle); + if ('error' in res && res.error) { + showPlaybookErrorSnackbar(); + logError('error on renameChecklist', getFullErrorMessage(res.error)); + } }, [serverUrl, playbookRunId, checklist.id, checklistNumber]); - const handleEditPress = useCallback((e: any) => { + const handleEditPress = useCallback((e: GestureResponderEvent) => { e.stopPropagation(); goToRenameChecklist(intl, theme, playbookRunName, checklist.title, handleRename); }, [intl, theme, playbookRunName, checklist.title, handleRename]); - const handleAddItem = useCallback((title: string) => { - addChecklistItem(serverUrl, playbookRunId, checklistNumber, title); + const handleAddItem = useCallback(async (title: string) => { + const res = await addChecklistItem(serverUrl, playbookRunId, checklistNumber, title); + if ('error' in res && res.error) { + showPlaybookErrorSnackbar(); + logError('error on addChecklistItem', getFullErrorMessage(res.error)); + } }, [serverUrl, playbookRunId, checklistNumber]); const handleAddPress = useCallback(() => { @@ -231,19 +243,15 @@ const Checklist = ({ /> ))} {!isFinished && isParticipant && ( - - - - {intl.formatMessage({id: 'playbooks.checklist_item.add.button', defaultMessage: 'New'})} - - + /> )} {/* This is a hack to get the height of the checklist items */} 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 84e742130..f90bcda10 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 @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import React, {useCallback, useEffect, useState} from 'react'; +import React, {useCallback, useEffect, useMemo, useState} from 'react'; import {useIntl} from 'react-intl'; import {Keyboard, StyleSheet, View} from 'react-native'; @@ -45,7 +45,10 @@ const RenamePlaybookRunBottomSheet = ({ const theme = useTheme(); const [title, setTitle] = useState(currentTitle); - const [canSave, setCanSave] = useState(false); + + const canSave = useMemo(() => { + return title.trim().length > 0 && title !== currentTitle; + }, [title, currentTitle]); const rightButton = React.useMemo(() => { const base = buildNavigationButton( @@ -65,20 +68,16 @@ const RenamePlaybookRunBottomSheet = ({ }); }, [rightButton, componentId]); - useEffect(() => { - setCanSave(title.trim().length > 0 && title !== currentTitle); - }, [title, currentTitle]); - const handleClose = useCallback(() => { close(componentId); }, [componentId]); const handleSave = useCallback(() => { - if (title.trim().length > 0) { + if (canSave) { onSave(title.trim()); close(componentId); } - }, [title, componentId, onSave]); + }, [canSave, title, componentId, onSave]); useNavButtonPressed(SAVE_BUTTON_ID, componentId, handleSave, [handleSave]); useAndroidHardwareBackHandler(componentId, handleClose); diff --git a/app/products/playbooks/screens/playbook_run/status_update_indicator.test.tsx b/app/products/playbooks/screens/playbook_run/status_update_indicator.test.tsx index f083588f3..50ab2439a 100644 --- a/app/products/playbooks/screens/playbook_run/status_update_indicator.test.tsx +++ b/app/products/playbooks/screens/playbook_run/status_update_indicator.test.tsx @@ -61,7 +61,7 @@ describe('StatusUpdateIndicator', () => { />, ); - const text = getByText(/pdate overdue/); + const text = getByText(/Update overdue/); expect(text).toHaveStyle({color: Preferences.THEMES.denim.dndIndicator}); const icon = getByTestId('compass-icon'); diff --git a/app/screens/channel/header/header.test.tsx b/app/screens/channel/header/header.test.tsx index 70ca9538b..c0a884d70 100644 --- a/app/screens/channel/header/header.test.tsx +++ b/app/screens/channel/header/header.test.tsx @@ -6,8 +6,8 @@ import React, {type ComponentProps} from 'react'; import NavigationHeader from '@components/navigation_header'; import {General} from '@constants'; import {useServerUrl} from '@context/server'; -import {createPlaybookRun, fetchPlaybookRunsForChannel} from '@playbooks/actions/remote/runs'; -import {goToPlaybookRun, goToPlaybookRuns} from '@playbooks/screens/navigation'; +import {fetchPlaybookRunsForChannel} from '@playbooks/actions/remote/runs'; +import {goToCreateQuickChecklist, goToPlaybookRun, goToPlaybookRuns} from '@playbooks/screens/navigation'; import EphemeralStore from '@store/ephemeral_store'; import {renderWithIntl, waitFor} from '@test/intl-test-helper'; @@ -172,15 +172,7 @@ describe('ChannelHeader', () => { expect(goToPlaybookRun).not.toHaveBeenCalled(); }); - it('creates a new playbook run when clicking + button with no active runs', async () => { - const mockCreatedRun = { - id: 'new-run-id', - channel_id: 'channel-id', - name: 'Test Channel Checklist', - } as any; - jest.mocked(createPlaybookRun).mockResolvedValue({data: mockCreatedRun}); - jest.mocked(fetchPlaybookRunsForChannel).mockResolvedValue({runs: [mockCreatedRun]}); - + it('navigates to create quick checklist screen when clicking + button with no active runs', () => { const props = getBaseProps(); props.playbooksActiveRuns = 0; props.hasPlaybookRuns = false; @@ -195,19 +187,16 @@ describe('ChannelHeader', () => { playbookButton?.onPress(); - await waitFor(() => { - expect(createPlaybookRun).toHaveBeenCalledWith( - serverUrl, - '', // empty playbook_id - 'current-user-id', - 'team-id', - 'Test Channel Checklist', - '', // empty description - 'channel-id', - ); - expect(fetchPlaybookRunsForChannel).toHaveBeenCalledWith(serverUrl, 'channel-id'); - expect(goToPlaybookRun).toHaveBeenCalledWith(expect.anything(), 'new-run-id'); - }); + expect(goToCreateQuickChecklist).toHaveBeenCalledWith( + expect.anything(), // intl + 'channel-id', + 'Test Channel', + 'current-user-id', + 'team-id', + serverUrl, + ); + expect(goToPlaybookRun).not.toHaveBeenCalled(); + expect(goToPlaybookRuns).not.toHaveBeenCalled(); }); it('should not fetch runs when playbooks are disabled', async () => { diff --git a/app/screens/channel/header/header.tsx b/app/screens/channel/header/header.tsx index c2de03f48..7e0752019 100644 --- a/app/screens/channel/header/header.tsx +++ b/app/screens/channel/header/header.tsx @@ -19,17 +19,14 @@ import {useTheme} from '@context/theme'; import {useIsTablet} from '@hooks/device'; import {useDefaultHeaderHeight} from '@hooks/header'; import {usePreventDoubleTap} from '@hooks/utils'; -import {createPlaybookRun, fetchPlaybookRunsForChannel} from '@playbooks/actions/remote/runs'; -import {goToPlaybookRun, goToPlaybookRuns} from '@playbooks/screens/navigation'; +import {fetchPlaybookRunsForChannel} from '@playbooks/actions/remote/runs'; +import {goToCreateQuickChecklist, goToPlaybookRun, goToPlaybookRuns} from '@playbooks/screens/navigation'; import {BOTTOM_SHEET_ANDROID_OFFSET} from '@screens/bottom_sheet'; import ChannelBanner from '@screens/channel/header/channel_banner'; import {bottomSheet, popTopScreen, showModal} from '@screens/navigation'; import EphemeralStore from '@store/ephemeral_store'; import {isTypeDMorGM} from '@utils/channel'; -import {getFullErrorMessage} from '@utils/errors'; import {bottomSheetSnapPoint} from '@utils/helpers'; -import {logDebug} from '@utils/log'; -import {showPlaybookErrorSnackbar} from '@utils/snack_bar'; import {changeOpacity, makeStyleSheetFromTheme} from '@utils/theme'; import {typography} from '@utils/typography'; @@ -219,33 +216,17 @@ const ChannelHeader = ({ }); }, [isTablet, callsAvailable, isDMorGM, hasPlaybookRuns, theme, onTitlePress, channelId]); - const handleCreateQuickRun = useCallback(async () => { - const runName = `${displayName} Checklist`; - const res = await createPlaybookRun( - serverUrl, - '', // empty playbook_id - currentUserId, - teamId, - runName, - '', // empty description - channelId, - ); - - if (res.error || !res.data) { - logDebug('error on createPlaybookRun', getFullErrorMessage(res.error)); - showPlaybookErrorSnackbar(); - return; - } - - // Fetch updated runs and navigate to the new run - await fetchPlaybookRunsForChannel(serverUrl, channelId); - await goToPlaybookRun(intl, res.data.id); - }, [serverUrl, currentUserId, teamId, displayName, channelId, intl]); - const openPlaybooksRuns = useCallback(() => { // If no active runs, create a new one instead if (playbooksActiveRuns === 0) { - handleCreateQuickRun(); + goToCreateQuickChecklist( + intl, + channelId, + displayName, + currentUserId, + teamId, + serverUrl, + ); return; } @@ -254,7 +235,7 @@ const ChannelHeader = ({ return; } goToPlaybookRuns(intl, channelId, displayName); - }, [playbooksActiveRuns, handleCreateQuickRun, activeRunId, channelId, displayName, intl]); + }, [playbooksActiveRuns, activeRunId, channelId, displayName, intl, currentUserId, teamId, serverUrl]); const rightButtons = useMemo(() => { const buttons: HeaderRightButton[] = []; diff --git a/assets/base/i18n/en.json b/assets/base/i18n/en.json index 527a87c7b..fa2526f29 100644 --- a/assets/base/i18n/en.json +++ b/assets/base/i18n/en.json @@ -776,6 +776,7 @@ "mobile.permalink_preview.originally_posted": "Originally posted in ", "mobile.permission_denied_dismiss": "Don't Allow", "mobile.permission_denied_retry": "Settings", + "mobile.playbook.create_checklist": "Create Checklist", "mobile.post_info.add_reaction": "Add Reaction", "mobile.post_info.copy_text": "Copy Text", "mobile.post_info.mark_unread": "Mark as Unread", @@ -1097,6 +1098,7 @@ "playbooks.select_user.not_participants": "NOT PARTICIPATING", "playbooks.select_user.participants": "PARTICIPANTS", "playbooks.start_a_run.title": "New", + "playbooks.start_checklist.description_label": "Description (optional)", "playbooks.start_run.channel_label": "Channel", "playbooks.start_run.create_new_channel": "Create a new channel", "playbooks.start_run.create_new_channel.private_channel": "Private channel",