-
Notifications
You must be signed in to change notification settings - Fork 849
Forms: add form delete action #46648
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
|
|
||
| return queryParams; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extracted this to re-usable method since we now import and use it beyond this hook.
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a delete/trash action to the Forms management dashboard, allowing users to move forms to trash with a confirmation dialog and success notifications.
Changes:
- Add a new
useDeleteFormhook to manage the delete flow including state, API calls, and cache invalidation - Extract query building logic into
getFormsListQueryfunction for reuse - Integrate trash action into the Forms list UI with confirmation dialog and notifications
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| projects/packages/forms/src/dashboard/hooks/use-forms-data.ts | Refactored query building logic into a reusable exported function |
| projects/packages/forms/src/dashboard/hooks/use-delete-form.ts | New hook implementing the complete delete/trash workflow with confirmation, API calls, error handling, and cache invalidation |
| projects/packages/forms/src/dashboard/forms/index.tsx | Integrated trash action into the Forms list with ConfirmDialog UI component |
| projects/packages/forms/changelog/add-delete-form-post | Added changelog entry for the new feature |
| Significance: minor | ||
| Type: added | ||
|
|
||
| Forms: add form delete action. |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changelog entry should not use a "Forms:" prefix since this entry is already within the forms package. According to the project guidelines, the package name should not be used as a prefix for entries in that same package. Consider changing "Forms: add form delete action." to "Add form delete action."
| export default function useDeleteForm( { | ||
| view, | ||
| setView, | ||
| recordsLength, | ||
| }: UseDeleteFormArgs ): UseDeleteFormReturn { | ||
| const [ formPendingDelete, setFormPendingDelete ] = useState< FormListItem | null >( null ); | ||
| const [ isDeleteConfirmDialogOpen, setIsDeleteConfirmDialogOpen ] = useState( false ); | ||
| const [ isDeleting, setIsDeleting ] = useState( false ); | ||
|
|
||
| const { deleteEntityRecord, invalidateResolution } = useDispatch( 'core' ) as CoreDispatch; | ||
| const { createSuccessNotice, createErrorNotice } = useDispatch( noticesStore ); | ||
|
|
||
| const page = view.page ?? 1; | ||
| const perPage = view.perPage ?? 20; | ||
| const search = view.search ?? ''; | ||
|
|
||
| const currentQuery = useMemo( | ||
| () => getFormsListQuery( page, perPage, search ), | ||
| [ page, perPage, search ] | ||
| ); | ||
|
|
||
| const openDeleteConfirmDialog = useCallback( ( item: FormListItem ) => { | ||
| setFormPendingDelete( item ); | ||
| setIsDeleteConfirmDialogOpen( true ); | ||
| }, [] ); | ||
|
|
||
| const closeDeleteConfirmDialog = useCallback( () => { | ||
| setIsDeleteConfirmDialogOpen( false ); | ||
| setFormPendingDelete( null ); | ||
| }, [] ); | ||
|
|
||
| const onConfirmDelete = useCallback( async () => { | ||
| if ( ! formPendingDelete || isDeleting ) { | ||
| return; | ||
| } | ||
|
|
||
| setIsDeleteConfirmDialogOpen( false ); | ||
| setIsDeleting( true ); | ||
|
|
||
| const shouldNavigateToPreviousPage = page > 1 && recordsLength === 1; | ||
|
|
||
| try { | ||
| await deleteEntityRecord( | ||
| 'postType', | ||
| 'jetpack_form', | ||
| formPendingDelete.id, | ||
| { force: false }, | ||
| { throwOnError: true } | ||
| ); | ||
|
|
||
| createSuccessNotice( __( 'Form moved to trash.', 'jetpack-forms' ), { | ||
| type: 'snackbar', | ||
| id: 'delete-form', | ||
| } ); | ||
|
|
||
| if ( shouldNavigateToPreviousPage ) { | ||
| setView( { ...view, page: page - 1 } ); | ||
| } | ||
| } catch { | ||
| createErrorNotice( __( 'Could not move form to trash.', 'jetpack-forms' ), { | ||
| type: 'snackbar', | ||
| id: 'delete-form-error', | ||
| } ); | ||
| } finally { | ||
| setIsDeleting( false ); | ||
| setFormPendingDelete( null ); | ||
|
|
||
| // Invalidate the list query so the trashed form disappears from the table and totals refresh. | ||
| invalidateResolution( 'getEntityRecords', [ 'postType', 'jetpack_form', currentQuery ] ); | ||
| invalidateResolution( 'getEntityRecords', [ | ||
| 'postType', | ||
| 'jetpack_form', | ||
| { ...currentQuery, per_page: 1, _fields: 'id' }, | ||
| ] ); | ||
| } | ||
| }, [ | ||
| createErrorNotice, | ||
| createSuccessNotice, | ||
| currentQuery, | ||
| deleteEntityRecord, | ||
| formPendingDelete, | ||
| invalidateResolution, | ||
| isDeleting, | ||
| page, | ||
| recordsLength, | ||
| setView, | ||
| view, | ||
| ] ); | ||
|
|
||
| return { | ||
| isDeleteConfirmDialogOpen, | ||
| isDeleting, | ||
| openDeleteConfirmDialog, | ||
| closeDeleteConfirmDialog, | ||
| onConfirmDelete, | ||
| }; | ||
| } |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new useDeleteForm hook lacks test coverage. Similar hooks in this directory, such as use-response-navigation, have comprehensive test coverage. Consider adding tests to verify the delete flow, including confirmation dialog state management, success/error handling, cache invalidation, and pagination edge cases.
Code Coverage SummaryCoverage changed in 2 files.
1 file is newly checked for coverage.
Full summary · PHP report · JS report If appropriate, add one of these labels to override the failing coverage check:
Covered by non-unit tests
|
02e251e to
c80d29a
Compare
Proposed changes:
We recently added a new forms tab and forms list to the dashboard behind our central form management flag.
This PR adds a Trash action that allows us to move Forms to trash. Specifically:
Screenshot: Trash Action
Screenshot: Confirmation Popup
Screenshot: Confirmation

Other information:
Jetpack product discussion
See Central Forms Management project on Linear.
Does this pull request change what data or activity we track or use?
No.
Testing instructions:
Add this feature flag to your test site.
Test for no regressions: