feat(schedule-actions): add delete confirmation action

[Assem-Uber]

Summary

Adds the Delete schedule action to the schedule-actions menu as a confirmation-only modal with a warning banner. On success, the UI navigates to the schedules list and invalidates listSchedules.

Changes

• schedule-actions.config.ts — delete action config with warning banner, DELETE on the schedule resource, and navigate-on-success handler
• schedule-actions-modal-content/ — optional httpMethod per action; send request body for non-GET methods
• schedule-actions.types.ts — ScheduleActionHttpMethod type (POST | PUT | DELETE)
• schedule-actions-enabled — enable delete in dynamic config and resolver schema
• Tests and fixtures for delete action in menu, modal, and orchestrator

Testing

• npm run test:unit:browser -- schedule-actions — 25 tests passed

Screen.Recording.2026-07-02.at.02.04.47.mov

Stack

• Depends on: feat(schedule-actions): wire actions popover on schedule page #1427 (merged) — schedule-actions mount on schedule page
• API: delete schedule handler merged in feat(api): delete schedule route handler #1420
• Part of: schedule-actions stack — SLICE-4 (delete UI)

Made with Cursor

[gitar-bot]

Code Review ⚠️ Changes requested 2 resolved / 3 findings

Integrates the delete schedule action with a confirmation banner, but the request logic triggers a redundant refetch of the just-deleted schedule. Reverting the describeSchedule invalidation logic is required to resolve this inconsistency.

⚠️ Edge Case: Deleting a schedule refetches the just-deleted describeSchedule

📄 src/views/schedule-actions/schedule-actions-modal-content/schedule-actions-modal-content.tsx:71-76 📄 src/views/schedule-actions/config/schedule-actions.config.ts:82-92

This commit removed the if (action.id !== 'delete') guard so onSuccess now unconditionally invalidates ['describeSchedule', params] (schedule-actions-modal-content.tsx:72-74) before running action.onSuccess, which does the router.push to the schedules list (schedule-actions.config.ts:82-92).

queryClient.invalidateQueries synchronously triggers a refetch of any active observer. The describeSchedule query on the schedule page uses the exact same key (['describeSchedule', params], see get-describe-schedule-query-options.ts) and is still mounted at that moment, because router.push navigation is asynchronous. As a result, immediately after a successful delete the app re-fetches the describe query for a schedule that no longer exists, producing a 404/error before the component unmounts.

This also directly contradicts the PR's stated design ("navigates to the schedules list and invalidates listSchedules without refetching the deleted schedule's describe query"). The delete onSuccess already invalidates listSchedules, which is what's needed. Consider skipping the describeSchedule invalidation for delete, or letting the action opt out of it, rather than always invalidating.

Let an action opt out of describeSchedule invalidation (delete sets it to false), avoiding a refetch of the deleted schedule.

onSuccess: (result, mutationParams) => {
  if (action.invalidateDescribeSchedule !== false) {
    queryClient.invalidateQueries({
      queryKey: ['describeSchedule', params],
    });
  }

  action.onSuccess?.({ queryClient, params, router });
  // ...
}

✅ 2 resolved

✅ Quality: Generic modal hardcodes 'delete' action id for invalidation skip

📄 src/views/schedule-actions/schedule-actions-modal-content/schedule-actions-modal-content.tsx:73-78
In schedule-actions-modal-content.tsx the generic, reusable modal component decides whether to invalidate the describeSchedule query by checking if (action.id !== 'delete'). This couples a component that is otherwise fully generic over its action config to one specific action id. If another action is added that should also skip (or must) invalidate the describe query, this string check has to be edited, and it is easy to miss. A cleaner approach is to drive the behavior from the action config itself (e.g. an explicit skipDescribeInvalidationOnSuccess?: boolean flag, or rely on the action's own onSuccess handler to manage cache invalidation) rather than branching on a magic string in the generic boundary.

✅ Quality: Body sent for all HTTP methods will break future GET actions

📄 src/views/schedule-actions/schedule-actions-modal-content/schedule-actions-modal-content.tsx:64-69 📄 src/views/schedule-actions/schedule-actions.types.ts:89
The mutationFn now always sends body: JSON.stringify(submissionData ?? {}) for every method (schedule-actions-modal-content.tsx:66-69), whereas previously the body was only attached for POST. At the same time httpMethod was widened to ScheduleActionHttpMethod = 'GET' | 'POST' | 'PUT' | 'DELETE' (schedule-actions.types.ts:89).

No current action uses GET, so this is not a live bug. However, request uses fetch, and fetch throws a TypeError when a GET (or HEAD) request is given a body. If a future action is configured with httpMethod: 'GET', the mutation will crash. Since the type now explicitly permits GET, it would be safer to omit the body for GET/HEAD requests.

🤖 Prompt for agents

Code Review: Integrates the delete schedule action with a confirmation banner, but the request logic triggers a redundant refetch of the just-deleted schedule. Reverting the `describeSchedule` invalidation logic is required to resolve this inconsistency.

1. ⚠️ Edge Case: Deleting a schedule refetches the just-deleted describeSchedule
   Files: src/views/schedule-actions/schedule-actions-modal-content/schedule-actions-modal-content.tsx:71-76, src/views/schedule-actions/config/schedule-actions.config.ts:82-92

   This commit removed the `if (action.id !== 'delete')` guard so `onSuccess` now unconditionally invalidates `['describeSchedule', params]` (schedule-actions-modal-content.tsx:72-74) before running `action.onSuccess`, which does the `router.push` to the schedules list (schedule-actions.config.ts:82-92).
   
   `queryClient.invalidateQueries` synchronously triggers a refetch of any active observer. The `describeSchedule` query on the schedule page uses the exact same key (`['describeSchedule', params]`, see get-describe-schedule-query-options.ts) and is still mounted at that moment, because `router.push` navigation is asynchronous. As a result, immediately after a successful delete the app re-fetches the describe query for a schedule that no longer exists, producing a 404/error before the component unmounts.
   
   This also directly contradicts the PR's stated design ("navigates to the schedules list and invalidates listSchedules without refetching the deleted schedule's describe query"). The delete `onSuccess` already invalidates `listSchedules`, which is what's needed. Consider skipping the describeSchedule invalidation for delete, or letting the action opt out of it, rather than always invalidating.

   Fix (Let an action opt out of describeSchedule invalidation (delete sets it to false), avoiding a refetch of the deleted schedule.):
   onSuccess: (result, mutationParams) => {
     if (action.invalidateDescribeSchedule !== false) {
       queryClient.invalidateQueries({
         queryKey: ['describeSchedule', params],
       });
     }
   
     action.onSuccess?.({ queryClient, params, router });
     // ...
   }

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[gitar-bot]

Code Review ✅ Approved 3 resolved / 3 findings

Integrates the delete schedule action with a warning banner, dynamic HTTP method support, and refined success navigation. Addressed issues regarding incorrect invalidation triggers, unnecessary re-fetches, and request body handling for GET methods.

✅ 3 resolved

✅ Quality: Generic modal hardcodes 'delete' action id for invalidation skip

📄 src/views/schedule-actions/schedule-actions-modal-content/schedule-actions-modal-content.tsx:73-78
In schedule-actions-modal-content.tsx the generic, reusable modal component decides whether to invalidate the describeSchedule query by checking if (action.id !== 'delete'). This couples a component that is otherwise fully generic over its action config to one specific action id. If another action is added that should also skip (or must) invalidate the describe query, this string check has to be edited, and it is easy to miss. A cleaner approach is to drive the behavior from the action config itself (e.g. an explicit skipDescribeInvalidationOnSuccess?: boolean flag, or rely on the action's own onSuccess handler to manage cache invalidation) rather than branching on a magic string in the generic boundary.

✅ Quality: Body sent for all HTTP methods will break future GET actions

📄 src/views/schedule-actions/schedule-actions-modal-content/schedule-actions-modal-content.tsx:64-69 📄 src/views/schedule-actions/schedule-actions.types.ts:89
The mutationFn now always sends body: JSON.stringify(submissionData ?? {}) for every method (schedule-actions-modal-content.tsx:66-69), whereas previously the body was only attached for POST. At the same time httpMethod was widened to ScheduleActionHttpMethod = 'GET' | 'POST' | 'PUT' | 'DELETE' (schedule-actions.types.ts:89).

No current action uses GET, so this is not a live bug. However, request uses fetch, and fetch throws a TypeError when a GET (or HEAD) request is given a body. If a future action is configured with httpMethod: 'GET', the mutation will crash. Since the type now explicitly permits GET, it would be safer to omit the body for GET/HEAD requests.

✅ Edge Case: Deleting a schedule refetches the just-deleted describeSchedule

📄 src/views/schedule-actions/schedule-actions-modal-content/schedule-actions-modal-content.tsx:71-76 📄 src/views/schedule-actions/config/schedule-actions.config.ts:82-92
This commit removed the if (action.id !== 'delete') guard so onSuccess now unconditionally invalidates ['describeSchedule', params] (schedule-actions-modal-content.tsx:72-74) before running action.onSuccess, which does the router.push to the schedules list (schedule-actions.config.ts:82-92).

queryClient.invalidateQueries synchronously triggers a refetch of any active observer. The describeSchedule query on the schedule page uses the exact same key (['describeSchedule', params], see get-describe-schedule-query-options.ts) and is still mounted at that moment, because router.push navigation is asynchronous. As a result, immediately after a successful delete the app re-fetches the describe query for a schedule that no longer exists, producing a 404/error before the component unmounts.

This also directly contradicts the PR's stated design ("navigates to the schedules list and invalidates listSchedules without refetching the deleted schedule's describe query"). The delete onSuccess already invalidates listSchedules, which is what's needed. Consider skipping the describeSchedule invalidation for delete, or letting the action opt out of it, rather than always invalidating.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[gitar-bot]

Code Review ✅ Approved 3 resolved / 3 findings

Implements the delete schedule confirmation modal with a warning banner and conditional navigation. Resolved issues regarding hardcoded action IDs, improper request body inclusion, and redundant refetching.

✅ 3 resolved

✅ Quality: Generic modal hardcodes 'delete' action id for invalidation skip

📄 src/views/schedule-actions/schedule-actions-modal-content/schedule-actions-modal-content.tsx:73-78
In schedule-actions-modal-content.tsx the generic, reusable modal component decides whether to invalidate the describeSchedule query by checking if (action.id !== 'delete'). This couples a component that is otherwise fully generic over its action config to one specific action id. If another action is added that should also skip (or must) invalidate the describe query, this string check has to be edited, and it is easy to miss. A cleaner approach is to drive the behavior from the action config itself (e.g. an explicit skipDescribeInvalidationOnSuccess?: boolean flag, or rely on the action's own onSuccess handler to manage cache invalidation) rather than branching on a magic string in the generic boundary.

✅ Quality: Body sent for all HTTP methods will break future GET actions

📄 src/views/schedule-actions/schedule-actions-modal-content/schedule-actions-modal-content.tsx:64-69 📄 src/views/schedule-actions/schedule-actions.types.ts:89
The mutationFn now always sends body: JSON.stringify(submissionData ?? {}) for every method (schedule-actions-modal-content.tsx:66-69), whereas previously the body was only attached for POST. At the same time httpMethod was widened to ScheduleActionHttpMethod = 'GET' | 'POST' | 'PUT' | 'DELETE' (schedule-actions.types.ts:89).

No current action uses GET, so this is not a live bug. However, request uses fetch, and fetch throws a TypeError when a GET (or HEAD) request is given a body. If a future action is configured with httpMethod: 'GET', the mutation will crash. Since the type now explicitly permits GET, it would be safer to omit the body for GET/HEAD requests.

✅ Edge Case: Deleting a schedule refetches the just-deleted describeSchedule

📄 src/views/schedule-actions/schedule-actions-modal-content/schedule-actions-modal-content.tsx:71-76 📄 src/views/schedule-actions/config/schedule-actions.config.ts:82-92
This commit removed the if (action.id !== 'delete') guard so onSuccess now unconditionally invalidates ['describeSchedule', params] (schedule-actions-modal-content.tsx:72-74) before running action.onSuccess, which does the router.push to the schedules list (schedule-actions.config.ts:82-92).

queryClient.invalidateQueries synchronously triggers a refetch of any active observer. The describeSchedule query on the schedule page uses the exact same key (['describeSchedule', params], see get-describe-schedule-query-options.ts) and is still mounted at that moment, because router.push navigation is asynchronous. As a result, immediately after a successful delete the app re-fetches the describe query for a schedule that no longer exists, producing a 404/error before the component unmounts.

This also directly contradicts the PR's stated design ("navigates to the schedules list and invalidates listSchedules without refetching the deleted schedule's describe query"). The delete onSuccess already invalidates listSchedules, which is what's needed. Consider skipping the describeSchedule invalidation for delete, or letting the action opt out of it, rather than always invalidating.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}