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.

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 new filter ep_skip_autosave_sync and enable autosave sync for protected_content feature #2861

Merged
merged 7 commits into from
Aug 5, 2022
Merged

Add new filter ep_skip_autosave_sync and enable autosave sync for protected_content feature #2861

merged 7 commits into from
Aug 5, 2022

Conversation

rebeccahum
Copy link
Contributor

@rebeccahum rebeccahum commented Jun 24, 2022

Description of the Change

Problem: When a user has the protected_content feature enabled, and they work on a draft but they never hit "Save draft" and instead, rely on the WordPress autosave, it won't show up in the Draft post listing screen (/wp-admin/edit.php?post_status=draft&post_type=post):

Screen Shot 2022-06-24 at 1 43 00 PM

This is because it never gets synced, as we disable it here:

if ( defined( 'DOING_AUTOSAVE' ) && DOING_AUTOSAVE ) {
// Bypass saving if doing autosave
// @codeCoverageIgnoreStart
return;
// @codeCoverageIgnoreEnd
}

This PR does two things:

  • Introduces the ep_skip_autosave_sync filter
  • Returns false for the aforementioned filter when the protected_content feature is enabled

Alternate Designs

I'm not sure if it would be beneficial to pass in the __FUNCTION__ for the second parameter during apply_filters() if one wanted to customize disabling the autosave sync on function-basis, since we have action_set_object_terms, action_sync_on_update, etc. to make it more granular:

		if ( apply_filters( 'ep_disable_autosave_sync', true, __FUNCTION__ ) ) {
			if ( defined( 'DOING_AUTOSAVE' ) && DOING_AUTOSAVE ) {
				// Bypass saving if doing autosave
				// @codeCoverageIgnoreStart
				return;
				// @codeCoverageIgnoreEnd
			}
		}

Curious on your thoughts @felipeelia for that.

Possible Drawbacks

N/A.

Verification Process

  1. Enable the protected_content feature
  2. Start drafting a new post and wait for WordPress to autosave
  3. Go to the Draft post listing screen at /wp-admin/edit.php?post_status=draft&post_type=post and expect to see the autosaved draft

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Changelog Entry

Fixed: autosaved drafts not showing up in draft post listing when using protected_content

Credits

Props @rebeccahum

@rebeccahum rebeccahum changed the title Add new filter ep_disable_autosave_sync and enable autosave sync for protected_content feature Add new filter ep_skip_autosave_sync and enable autosave sync for protected_content feature Jun 24, 2022
@felipeelia felipeelia added this to the 4.3.0 milestone Jul 10, 2022
@felipeelia felipeelia self-assigned this Jul 10, 2022
@felipeelia felipeelia modified the milestones: 4.3.0, 4.2.2 Jul 12, 2022
@felipeelia
Copy link
Member

Hey @rebeccahum, I'm finally getting to this one. I like the idea of adding __FUNCTION__ as a parameter so people can opt out of some scenarios, can you please add that too?

In addition to that, do you think it'd be possible to add some tests? End-to-end tests checking if that works as expected and also does not affect published posts is what I have in mind. Thanks!

@rebeccahum
Copy link
Contributor Author

@felipeelia Done and done!

tests/cypress/support/commands.js Outdated Show resolved Hide resolved
@rebeccahum
Copy link
Contributor Author

@felipeelia Thanks for the feedback, I think I got it working now!

@felipeelia felipeelia merged commit e74e84a into 10up:develop Aug 5, 2022
rebeccahum added a commit to Automattic/ElasticPress that referenced this pull request Aug 5, 2022
@rebeccahum rebeccahum deleted the add/ep_disable_sync_update-filter branch August 5, 2022 16:47
rebeccahum added a commit to Automattic/ElasticPress that referenced this pull request Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants