Skip to content
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.

Sign up for GitHub

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

Add an async __unstablePreSavePost hook; resolving with false prevents saving #58022

Merged
merged 12 commits into from
Aug 2, 2024
Merged

Add an async __unstablePreSavePost hook; resolving with false prevents saving #58022

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
e684c39
Add an async `__unstablePreSavePost` hook; resolving with false preve…
adamsilverstein Jan 19, 2024
f2e65b8
Simplify preSave check
adamsilverstein Jan 19, 2024
101c0d1
Merge branch 'trunk' into add/preSaveHook
adamsilverstein May 8, 2024
bcae174
Filter edits & enable throwing Error in `__unstablePreSavePost` filter
adamsilverstein May 8, 2024
da8196e
Merge branch 'trunk' into add/preSaveHook
adamsilverstein Jul 16, 2024
ff7d6b4
Pass Promise.resolve( edits ) instead of edits
adamsilverstein Jul 16, 2024
4ead1ff
Move REQUEST_POST_UPDATE_START to before preSave async filter
adamsilverstein Jul 16, 2024
d5d7bfc
Update saveEntityRecord error handling to use try/catch
adamsilverstein Jul 16, 2024
a546cdd
Merge branch 'trunk' into add/preSaveHook
adamsilverstein Aug 1, 2024
7764b36
Use explicit false check for error
adamsilverstein Aug 1, 2024
2a0d9f2
reduce changes, test functionality, don’t pass data to filter
adamsilverstein Aug 1, 2024
05b4852
whitespace
adamsilverstein Aug 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/editor/src/store/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,15 @@ export const savePost =
dispatch.editPost( { content }, { undoIgnore: true } );
}

const preSaveOK = await applyFilters(
'editor.__unstablePreSavePost',
Promise.resolve( true ),
options
);
if ( ! preSaveOK ) {
return;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for starting to implement this! 🚀

Some further improvements would be nice:

  1. The hook should be able to return/throw an error. Like Error( 'ACF validation failed' ). The code further down the savePost function will then display a notice with that error.
  2. The hook should be able to inspect also the edits, and maybe even change them. In addition to the options that are already passed to it (in practice we inspect options.isAutosave).

The result would look like:

try {
  filteredEdits = await applyFilters(
    'editor.__unstablePreSavePost',
    Promise.resolve( edits ),
    options
  );
} catch ( err ) {
  error = err;
}

if ( ! error ) {
  // do save and the SavePost hook
}

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👋 Thanks a lot for working on this, Adam! I was pointed to this PR by Jarda, as I have a somewhat related use case:

I have an entity (a wp_navigation postType) that is loaded via useEntityRecords(). The entity has a post meta field registered (which is returned by the endpoint). When saving the entity (after it has had some edits), I’d like to include the post meta, even if the latter doesn't have any edits. (See the bottom of this comment for more context.)

It seems that an __unstablePreSavePost filter -- if Jarda's suggestion of allowing modifications to edits is applied -- could solve that problem (although I'd need it to work in the Site Editor rather than in the Post Editor).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

although I'd need it to work in the Site Editor rather than in the Post Editor

Do we need to add it there separately @ockham?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hook should be able to inspect also the edits, and maybe even change them. In addition to the options that are already passed to it (in practice we inspect options.isAutosave).

My only goal here is to prevent saving, I don't thing we should enable changing the edits at this point, what would the use case for that feature be?

I have an entity (a wp_navigation postType) that is loaded via useEntityRecords(). The entity has a post meta field registered (which is returned by the endpoint). When saving the entity (after it has had some edits), I’d like to include the post meta, even if the latter doesn't have any edits.

@ockham Ah, this does sound like a valid use case for being able to modify the edits before a save, have you already solved this elsewhere?

This would complicate the use of the proposed preSave hook as I intended - simply returning an error message to hook into the existing error handling logic to prevent saving.

Since this proposal changes the use case for the hook, I would prefer to consider it in a follow up issue, what do you think?


const previousRecord = select.getCurrentPost();
const edits = {
id: previousRecord.id,
Expand Down
Loading