[MM-67006] Delete task (#9381)

* delete button

* prevent deletion if we can't communicate with server
add tests

* I18n

* move to Pressable

* address review comments

* Use dedicated i18n IDs for delete task dialog

Replace reused generic translation IDs (mobile.post.cancel, post_info.del)
with playbooks-specific IDs for better maintainability and to avoid
unintended changes if source messages are modified.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Guillermo Vayá 2026-01-11 22:31:42 +01:00 committed by GitHub
parent 1a563a7917
commit 184f272965
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 351 additions and 5 deletions

View file

@ -296,8 +296,10 @@ Located at `libraries/@mattermost/`:
### Localization (i18n)
- **CRITICAL**: Only update `en.json` - never modify other language files or Weblate gets corrupted
- **Adding new strings**: Define the message ID and defaultMessage in code using `defineMessages()`, then run `npm run i18n-extract` to automatically add them to `en.json`
- Default messages in code must match JSON translations exactly, including newlines
- Translation IDs should be descriptive enough for translators to understand context
- Don't reuse translation IDs
- Translate user-facing strings, not debug/error messages
### Markdown Component Usage

View file

@ -6,7 +6,7 @@ import {getPlaybookChecklistById} from '@playbooks/database/queries/checklist';
import {getPlaybookChecklistItemById} from '@playbooks/database/queries/item';
import TestHelper from '@test/test_helper';
import {updateChecklistItem, setChecklistItemCommand, setAssignee, setDueDate, renameChecklist} from './checklist';
import {updateChecklistItem, setChecklistItemCommand, setAssignee, setDueDate, renameChecklist, deleteChecklistItem} from './checklist';
import type ServerDataOperator from '@database/operator/server_data_operator';
@ -394,3 +394,69 @@ describe('renameChecklist', () => {
expect(updated!.title).toBe('Updated Checklist Title');
});
});
describe('deleteChecklistItem', () => {
it('should handle not found database', async () => {
const {error} = await deleteChecklistItem('foo', 'itemid');
expect(error).toBeTruthy();
expect((error as Error).message).toContain('foo database not found');
});
it('should handle item not found', async () => {
const {error} = await deleteChecklistItem(serverUrl, 'nonexistent');
expect(error).toBe('Item not found: nonexistent');
});
it('should handle database write errors', async () => {
const checklistId = 'checklistid';
const item = TestHelper.createPlaybookItem(checklistId, 0);
await operator.handlePlaybookChecklistItem({items: [{...item, checklist_id: checklistId}], prepareRecordsOnly: false});
const originalWrite = operator.database.write;
operator.database.write = jest.fn().mockRejectedValue(new Error('Database write failed'));
const {error} = await deleteChecklistItem(serverUrl, item.id);
expect(error).toBeTruthy();
operator.database.write = originalWrite;
});
it('should delete checklist item successfully', async () => {
const checklistId = 'checklistid';
const item = TestHelper.createPlaybookItem(checklistId, 0);
await operator.handlePlaybookChecklistItem({items: [{...item, checklist_id: checklistId}], prepareRecordsOnly: false});
// Verify item exists before deletion
const beforeDelete = await getPlaybookChecklistItemById(operator.database, item.id);
expect(beforeDelete).toBeDefined();
expect(beforeDelete!.id).toBe(item.id);
const {data, error} = await deleteChecklistItem(serverUrl, item.id);
expect(error).toBeUndefined();
expect(data).toBe(true);
// Verify item is deleted
const afterDelete = await getPlaybookChecklistItemById(operator.database, item.id);
expect(afterDelete).toBeUndefined();
});
it('should delete checklist item with different states', async () => {
const checklistId = 'checklistid';
const states: ChecklistItemState[] = ['', 'in_progress', 'closed', 'skipped'];
const testPromises = states.map(async (state) => {
const item = TestHelper.createPlaybookItem(checklistId, 0);
item.state = state;
await operator.handlePlaybookChecklistItem({items: [{...item, checklist_id: checklistId}], prepareRecordsOnly: false});
const {data, error} = await deleteChecklistItem(serverUrl, item.id);
expect(error).toBeUndefined();
expect(data).toBe(true);
const deleted = await getPlaybookChecklistItemById(operator.database, item.id);
expect(deleted).toBeUndefined();
});
await Promise.all(testPromises);
});
});

View file

@ -115,3 +115,22 @@ export async function renameChecklist(serverUrl: string, checklistId: string, ti
return {error};
}
}
export async function deleteChecklistItem(serverUrl: string, itemId: string) {
try {
const {database} = DatabaseManager.getServerDatabaseAndOperator(serverUrl);
const item = await getPlaybookChecklistItemById(database, itemId);
if (!item) {
return {error: `Item not found: ${itemId}`};
}
await database.write(async () => {
await item.destroyPermanently();
});
return {data: true};
} catch (error) {
logError('failed to delete checklist item', error);
return {error};
}
}

View file

@ -10,6 +10,7 @@ import {
setAssignee as localSetAssignee,
setDueDate as localSetDueDate,
renameChecklist as localRenameChecklist,
deleteChecklistItem as localDeleteChecklistItem,
} from '@playbooks/actions/local/checklist';
import {handlePlaybookRuns} from '@playbooks/actions/local/run';
@ -23,6 +24,7 @@ import {
setDueDate,
renameChecklist,
addChecklistItem,
deleteChecklistItem,
} from './checklist';
const serverUrl = 'baseHandler.test.com';
@ -43,6 +45,7 @@ const mockClient = {
setDueDate: jest.fn(),
renameChecklist: jest.fn(),
addChecklistItem: jest.fn(),
deleteChecklistItem: jest.fn(),
fetchPlaybookRun: jest.fn(),
};
@ -442,4 +445,45 @@ describe('checklist', () => {
expect(handlePlaybookRuns).toHaveBeenCalledWith(serverUrl, [mockRun], false, true);
});
});
describe('deleteChecklistItem', () => {
it('should handle client error', async () => {
jest.spyOn(NetworkManager, 'getClient').mockImplementationOnce(throwFunc);
const result = await deleteChecklistItem(serverUrl, playbookRunId, itemId, checklistNumber, itemNumber);
expect(result.error).toBeDefined();
expect(mockClient.deleteChecklistItem).not.toHaveBeenCalled();
expect(localDeleteChecklistItem).not.toHaveBeenCalled();
});
it('should handle API exception - server deletion fails', async () => {
mockClient.deleteChecklistItem.mockImplementationOnce(throwFunc);
const result = await deleteChecklistItem(serverUrl, playbookRunId, itemId, checklistNumber, itemNumber);
expect(result.error).toBeDefined();
expect(mockClient.deleteChecklistItem).toHaveBeenCalledWith(playbookRunId, checklistNumber, itemNumber);
expect(localDeleteChecklistItem).not.toHaveBeenCalled();
});
it('should handle local deletion error - server succeeds but local fails', async () => {
mockClient.deleteChecklistItem.mockResolvedValueOnce(undefined);
jest.mocked(localDeleteChecklistItem).mockResolvedValueOnce({error: 'Local DB error'});
const result = await deleteChecklistItem(serverUrl, playbookRunId, itemId, checklistNumber, itemNumber);
expect(result.error).toBe('Local DB error');
expect(mockClient.deleteChecklistItem).toHaveBeenCalledWith(playbookRunId, checklistNumber, itemNumber);
expect(localDeleteChecklistItem).toHaveBeenCalledWith(serverUrl, itemId);
});
it('should delete checklist item successfully', async () => {
mockClient.deleteChecklistItem.mockResolvedValueOnce(undefined);
jest.mocked(localDeleteChecklistItem).mockResolvedValueOnce({data: true});
const result = await deleteChecklistItem(serverUrl, playbookRunId, itemId, checklistNumber, itemNumber);
expect(result.error).toBeUndefined();
expect(result.data).toBe(true);
expect(mockClient.deleteChecklistItem).toHaveBeenCalledWith(playbookRunId, checklistNumber, itemNumber);
expect(localDeleteChecklistItem).toHaveBeenCalledWith(serverUrl, itemId);
});
});
});

View file

@ -10,6 +10,7 @@ import {
setAssignee as localSetAssignee,
setDueDate as localSetDueDate,
renameChecklist as localRenameChecklist,
deleteChecklistItem as localDeleteChecklistItem,
} from '@playbooks/actions/local/checklist';
import {handlePlaybookRuns} from '@playbooks/actions/local/run';
import {getFullErrorMessage} from '@utils/errors';
@ -202,3 +203,29 @@ export const addChecklistItem = async (
return {error};
}
};
export const deleteChecklistItem = async (
serverUrl: string,
playbookRunId: string,
itemId: string,
checklistNumber: number,
itemNumber: number,
) => {
try {
const client = NetworkManager.getClient(serverUrl);
await client.deleteChecklistItem(playbookRunId, checklistNumber, itemNumber);
// Only delete from local database if server operation succeeded
const localResult = await localDeleteChecklistItem(serverUrl, itemId);
if (localResult.error) {
logDebug('error on deleteChecklistItem local deletion', localResult.error);
return {error: localResult.error};
}
return {data: true};
} catch (error) {
logDebug('error on deleteChecklistItem', getFullErrorMessage(error));
forceLogoutIfNecessary(serverUrl, error);
return {error};
}
};

View file

@ -29,6 +29,7 @@ export interface ClientPlaybooksMix {
setAssignee: (playbookRunId: string, checklistNum: number, itemNum: number, assigneeId?: string) => Promise<void>;
setDueDate: (playbookRunId: string, checklistNum: number, itemNum: number, date?: number) => Promise<void>;
addChecklistItem: (playbookRunId: string, checklistNum: number, title: string) => Promise<void>;
deleteChecklistItem: (playbookRunId: string, checklistNum: number, itemNum: number) => Promise<void>;
renameChecklist: (playbookRunId: string, checklistNumber: number, newName: string) => Promise<void>;
@ -216,6 +217,13 @@ const ClientPlaybooks = <TBase extends Constructor<ClientBase>>(superclass: TBas
);
};
deleteChecklistItem = async (playbookRunId: string, checklistNum: number, itemNum: number) => {
await this.doFetch(
`${this.getPlaybookRunRoute(playbookRunId)}/checklists/${checklistNum}/item/${itemNum}`,
{method: 'delete'},
);
};
// Slash Commands
runChecklistItemSlashCommand = async (playbookRunId: string, checklistNumber: number, itemNumber: number) => {
const data = await this.doFetch(

View file

@ -5,13 +5,13 @@
import {BottomSheetScrollView} from '@gorhom/bottom-sheet';
import {act, fireEvent, waitFor} from '@testing-library/react-native';
import React, {type ComponentProps} from 'react';
import {ScrollView} from 'react-native';
import {Alert, ScrollView} from 'react-native';
import OptionBox from '@components/option_box';
import OptionItem from '@components/option_item';
import {Preferences} from '@constants';
import {useIsTablet} from '@hooks/device';
import {setAssignee, setChecklistItemCommand, setDueDate} from '@playbooks/actions/remote/checklist';
import {setAssignee, setChecklistItemCommand, setDueDate, deleteChecklistItem} from '@playbooks/actions/remote/checklist';
import {goToEditCommand, goToSelectDate, goToSelectUser} from '@playbooks/screens/navigation';
import {dismissBottomSheet, openUserProfileModal} from '@screens/navigation';
import {renderWithIntl} from '@test/intl-test-helper';
@ -731,4 +731,110 @@ describe('ChecklistItemBottomSheet', () => {
expect(icon.props.size).toBe(24);
});
});
describe('delete button', () => {
beforeEach(() => {
jest.spyOn(Alert, 'alert');
});
it('should render delete button when isDisabled is false', () => {
const props = getBaseProps();
props.isDisabled = false;
const {getByTestId} = renderWithIntl(<ChecklistItemBottomSheet {...props}/>);
expect(getByTestId('checklist_item_bottom_sheet.delete_button')).toBeVisible();
});
it('should not render delete button when isDisabled is true', () => {
const props = getBaseProps();
props.isDisabled = true;
const {queryByTestId} = renderWithIntl(<ChecklistItemBottomSheet {...props}/>);
expect(queryByTestId('checklist_item_bottom_sheet.delete_button')).toBeNull();
});
it('should show confirmation alert when delete button is pressed', () => {
const props = getBaseProps();
const {getByTestId} = renderWithIntl(<ChecklistItemBottomSheet {...props}/>);
const deleteButton = getByTestId('checklist_item_bottom_sheet.delete_button');
fireEvent.press(deleteButton);
expect(Alert.alert).toHaveBeenCalledWith(
'Delete task',
'Are you sure you want to delete this task? This action cannot be undone.',
expect.arrayContaining([
expect.objectContaining({text: 'Cancel', style: 'cancel'}),
expect.objectContaining({text: 'Delete', style: 'destructive'}),
]),
);
});
it('should call deleteChecklistItem and dismissBottomSheet when confirmed', async () => {
jest.mocked(deleteChecklistItem).mockResolvedValue({data: true});
const props = getBaseProps();
props.checklistNumber = 2;
props.itemNumber = 3;
const {getByTestId} = renderWithIntl(<ChecklistItemBottomSheet {...props}/>);
const deleteButton = getByTestId('checklist_item_bottom_sheet.delete_button');
fireEvent.press(deleteButton);
// Get the onPress callback from the Delete button in the alert
const alertCall = jest.mocked(Alert.alert).mock.calls[0];
const buttons = alertCall[2] as Array<{text: string; onPress?: () => void}>;
const deleteButtonConfig = buttons.find((b) => b.text === 'Delete');
await act(async () => {
deleteButtonConfig?.onPress?.();
});
expect(dismissBottomSheet).toHaveBeenCalled();
expect(deleteChecklistItem).toHaveBeenCalledWith(
'server-url',
'run-1',
'item-1',
2,
3,
);
});
it('should not call deleteChecklistItem when cancelled', () => {
const props = getBaseProps();
const {getByTestId} = renderWithIntl(<ChecklistItemBottomSheet {...props}/>);
const deleteButton = getByTestId('checklist_item_bottom_sheet.delete_button');
fireEvent.press(deleteButton);
// Get the onPress callback from the Cancel button in the alert
const alertCall = jest.mocked(Alert.alert).mock.calls[0];
const buttons = alertCall[2] as Array<{text: string; onPress?: () => void}>;
const cancelButtonConfig = buttons.find((b) => b.text === 'Cancel');
// Cancel button doesn't have onPress, it just closes the alert
expect(cancelButtonConfig?.onPress).toBeUndefined();
expect(deleteChecklistItem).not.toHaveBeenCalled();
});
it('should show error snackbar when deleteChecklistItem fails', async () => {
jest.mocked(deleteChecklistItem).mockResolvedValue({error: 'Delete failed'});
const props = getBaseProps();
const {getByTestId} = renderWithIntl(<ChecklistItemBottomSheet {...props}/>);
const deleteButton = getByTestId('checklist_item_bottom_sheet.delete_button');
fireEvent.press(deleteButton);
const alertCall = jest.mocked(Alert.alert).mock.calls[0];
const buttons = alertCall[2] as Array<{text: string; onPress?: () => void}>;
const deleteButtonConfig = buttons.find((b) => b.text === 'Delete');
await act(async () => {
deleteButtonConfig?.onPress?.();
});
await waitFor(() => {
expect(showPlaybookErrorSnackbar).toHaveBeenCalled();
});
});
});
});

View file

@ -3,7 +3,7 @@
import React, {useCallback, useMemo, type ComponentProps} from 'react';
import {defineMessages, useIntl} from 'react-intl';
import {View, Text, Platform} from 'react-native';
import {View, Text, Platform, Pressable, Alert} from 'react-native';
import CompassIcon from '@components/compass_icon';
import MenuDivider from '@components/menu_divider';
@ -11,7 +11,7 @@ import OptionBox from '@components/option_box';
import OptionItem, {ITEM_HEIGHT} from '@components/option_item';
import {useServerUrl} from '@context/server';
import {useTheme} from '@context/theme';
import {setAssignee, setChecklistItemCommand, setDueDate} from '@playbooks/actions/remote/checklist';
import {setAssignee, setChecklistItemCommand, setDueDate, deleteChecklistItem} from '@playbooks/actions/remote/checklist';
import {goToEditCommand, goToSelectDate, goToSelectUser} from '@playbooks/screens/navigation';
import {getDueDateString} from '@playbooks/utils/time';
import {dismissBottomSheet, openUserProfileModal} from '@screens/navigation';
@ -74,6 +74,26 @@ const messages = defineMessages({
id: 'playbooks.checklist_item.task_rendered_conditionally_explanation',
defaultMessage: 'This task was rendered conditionally based on',
},
deleteTask: {
id: 'playbooks.checklist_item.delete_task',
defaultMessage: 'Delete task',
},
deleteTaskTitle: {
id: 'playbooks.checklist_item.delete_task_title',
defaultMessage: 'Delete task',
},
deleteTaskConfirmation: {
id: 'playbooks.checklist_item.delete_task_confirmation',
defaultMessage: 'Are you sure you want to delete this task? This action cannot be undone.',
},
cancel: {
id: 'playbooks.checklist_item.delete_task_cancel',
defaultMessage: 'Cancel',
},
delete: {
id: 'playbooks.checklist_item.delete_task_confirm',
defaultMessage: 'Delete',
},
});
const ACTION_BUTTON_HEIGHT = 62;
@ -144,6 +164,17 @@ const getStyleSheet = makeStyleSheetFromTheme((theme) => ({
conditionIcon: {
transform: [{rotate: '90deg'}],
},
deleteButton: {
marginTop: 8,
alignItems: 'center',
},
deleteButtonText: {
...typography('Body', 200, 'SemiBold'),
color: theme.dndIndicator,
},
deleteButtonPressed: {
opacity: 0.2,
},
}));
type Props = {
@ -367,6 +398,30 @@ const ChecklistItemBottomSheet = ({
</>
);
const handleDelete = useCallback(() => {
Alert.alert(
intl.formatMessage(messages.deleteTaskTitle),
intl.formatMessage(messages.deleteTaskConfirmation),
[
{
text: intl.formatMessage(messages.cancel),
style: 'cancel',
},
{
text: intl.formatMessage(messages.delete),
style: 'destructive',
onPress: async () => {
await dismissBottomSheet();
const res = await deleteChecklistItem(serverUrl, runId, item.id, checklistNumber, itemNumber);
if (res.error) {
showPlaybookErrorSnackbar();
}
},
},
],
);
}, [intl, serverUrl, runId, item.id, checklistNumber, itemNumber]);
return (
<View
style={styles.container}
@ -391,6 +446,20 @@ const ChecklistItemBottomSheet = ({
{!isDisabled && renderActionButtons()}
{renderTaskDetails()}
{showConditionIcon && renderConditionSection()}
{!isDisabled && (
<>
<MenuDivider/>
<Pressable
onPress={handleDelete}
style={({pressed}) => [styles.deleteButton, pressed && styles.deleteButtonPressed]}
testID='checklist_item_bottom_sheet.delete_button'
>
<Text style={styles.deleteButtonText}>
{intl.formatMessage(messages.deleteTask)}
</Text>
</Pressable>
</>
)}
</View>
);
};

View file

@ -1040,6 +1040,11 @@
"playbooks.checklist_item.check": "Check",
"playbooks.checklist_item.checked": "Checked",
"playbooks.checklist_item.command": "Command",
"playbooks.checklist_item.delete_task": "Delete task",
"playbooks.checklist_item.delete_task_cancel": "Cancel",
"playbooks.checklist_item.delete_task_confirm": "Delete",
"playbooks.checklist_item.delete_task_confirmation": "Are you sure you want to delete this task? This action cannot be undone.",
"playbooks.checklist_item.delete_task_title": "Delete task",
"playbooks.checklist_item.due_date": "Due date",
"playbooks.checklist_item.none": "None",
"playbooks.checklist_item.rerun_command": "Rerun command",