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

Use all eligible post types when all_templates_supported is selected. #1338

Merged
merged 1 commit into from Aug 14, 2018

Conversation

@hellofromtonya
Copy link
Contributor

@hellofromtonya hellofromtonya commented Aug 14, 2018

Per issue 1302, post type notices are being blasted when:

  1. There are no options saved in the database AND
  2. You switch from Classic to either Paired or Native.

The root cause for this problem is that the method AMP_Options_Manager::check_supported_post_type_update_errors() is only considering the selected post types when the all_templates_supported option is selected.

This PR fixes the problem by using all of the eligible post types (and not just the ones that are individually selected) when all_templates_supported option is selected.

Fixes #1302.

Fixes #1302.

In the `check_supported_post_type_update_errors()` method, when `all_templates_supported` option is selected, we want to use all of the eligible post types and not just the ones that are individually selected.  In doing so, this commit fixes #1302 where it blasts erroneous notices about unable to deactivate supported post types.
@hellofromtonya hellofromtonya added this to the v1.0 milestone Aug 14, 2018
@hellofromtonya hellofromtonya requested a review from westonruter Aug 14, 2018
Copy link
Member

@westonruter westonruter left a comment

Looks great!

@westonruter westonruter merged commit 6a7fd5e into develop Aug 14, 2018
2 checks passed
@westonruter westonruter deleted the fix/erroneous-post-type-notices branch Aug 14, 2018
@postphotos postphotos added this to Definition in v1.0 Aug 15, 2018
@westonruter westonruter moved this from Definition to Ready for review in v1.0 Aug 15, 2018
@westonruter westonruter moved this from Ready for review to Ready for QA in v1.0 Aug 15, 2018
@kienstra
Copy link
Contributor

@kienstra kienstra commented Sep 18, 2018

Moving To 'Ready For Merging'

If it's alright, I'm moving this to 'Ready For Merging.' It looks like reproducing this on the staging site will be complex, though we could create a new site for this if you think it's important.

@kienstra kienstra moved this from Ready for QA to Ready for Merging in v1.0 Sep 18, 2018
@kienstra kienstra moved this from Ready for Merging to In Production in v1.0 Dec 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
v1.0
In Production
Linked issues

Successfully merging this pull request may close these issues.

3 participants