diff --git a/CLAUDE.md b/CLAUDE.md index eaa9fc125..93c96b595 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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 diff --git a/app/products/playbooks/actions/local/checklist.test.ts b/app/products/playbooks/actions/local/checklist.test.ts index a89bb6f3a..a398dd72d 100644 --- a/app/products/playbooks/actions/local/checklist.test.ts +++ b/app/products/playbooks/actions/local/checklist.test.ts @@ -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); + }); +}); diff --git a/app/products/playbooks/actions/local/checklist.ts b/app/products/playbooks/actions/local/checklist.ts index 3af9edf94..0d697fba7 100644 --- a/app/products/playbooks/actions/local/checklist.ts +++ b/app/products/playbooks/actions/local/checklist.ts @@ -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}; + } +} diff --git a/app/products/playbooks/actions/remote/checklist.test.ts b/app/products/playbooks/actions/remote/checklist.test.ts index ab75f2891..432ca0939 100644 --- a/app/products/playbooks/actions/remote/checklist.test.ts +++ b/app/products/playbooks/actions/remote/checklist.test.ts @@ -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); + }); + }); }); diff --git a/app/products/playbooks/actions/remote/checklist.ts b/app/products/playbooks/actions/remote/checklist.ts index 3b5bc6176..a61177fac 100644 --- a/app/products/playbooks/actions/remote/checklist.ts +++ b/app/products/playbooks/actions/remote/checklist.ts @@ -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}; + } +}; diff --git a/app/products/playbooks/client/rest.ts b/app/products/playbooks/client/rest.ts index 884dd93fe..64f7f7442 100644 --- a/app/products/playbooks/client/rest.ts +++ b/app/products/playbooks/client/rest.ts @@ -29,6 +29,7 @@ export interface ClientPlaybooksMix { setAssignee: (playbookRunId: string, checklistNum: number, itemNum: number, assigneeId?: string) => Promise; setDueDate: (playbookRunId: string, checklistNum: number, itemNum: number, date?: number) => Promise; addChecklistItem: (playbookRunId: string, checklistNum: number, title: string) => Promise; + deleteChecklistItem: (playbookRunId: string, checklistNum: number, itemNum: number) => Promise; renameChecklist: (playbookRunId: string, checklistNumber: number, newName: string) => Promise; @@ -216,6 +217,13 @@ const ClientPlaybooks = >(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( diff --git a/app/products/playbooks/screens/playbook_run/checklist/checklist_item/checklist_item_bottom_sheet/checklist_item_bottom_sheet.test.tsx b/app/products/playbooks/screens/playbook_run/checklist/checklist_item/checklist_item_bottom_sheet/checklist_item_bottom_sheet.test.tsx index c8c9ad66f..94051c613 100644 --- a/app/products/playbooks/screens/playbook_run/checklist/checklist_item/checklist_item_bottom_sheet/checklist_item_bottom_sheet.test.tsx +++ b/app/products/playbooks/screens/playbook_run/checklist/checklist_item/checklist_item_bottom_sheet/checklist_item_bottom_sheet.test.tsx @@ -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(); + + 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(); + + 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(); + + 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(); + + 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(); + + 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(); + + 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(); + }); + }); + }); }); diff --git a/app/products/playbooks/screens/playbook_run/checklist/checklist_item/checklist_item_bottom_sheet/checklist_item_bottom_sheet.tsx b/app/products/playbooks/screens/playbook_run/checklist/checklist_item/checklist_item_bottom_sheet/checklist_item_bottom_sheet.tsx index 8395d7e06..795f2de53 100644 --- a/app/products/playbooks/screens/playbook_run/checklist/checklist_item/checklist_item_bottom_sheet/checklist_item_bottom_sheet.tsx +++ b/app/products/playbooks/screens/playbook_run/checklist/checklist_item/checklist_item_bottom_sheet/checklist_item_bottom_sheet.tsx @@ -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 ( + + [styles.deleteButton, pressed && styles.deleteButtonPressed]} + testID='checklist_item_bottom_sheet.delete_button' + > + + {intl.formatMessage(messages.deleteTask)} + + + + )} ); }; diff --git a/assets/base/i18n/en.json b/assets/base/i18n/en.json index 96d5c89b6..0f7381442 100644 --- a/assets/base/i18n/en.json +++ b/assets/base/i18n/en.json @@ -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",