Limit validation to AMP theme support #1132
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks, Will Review Soon Hi @westonruter, |
This was referenced May 7, 2018
kienstra
approved these changes
May 8, 2018
There was a problem hiding this comment.
Approved
Hi @westonruter,
Thanks, this pull request looks good, and works as expected. There are no validation errors when using Legacy Templating. Only in Native AMP or Paired Mode.
It's nice that this also simplifies the repo, using less if blocks.
There's a question here, but it's not a blocker.
| $validation_errors = array(); | ||
| // Skip if the post type is not viewable on the frontend, since we need a permalink to validate. | ||
| if ( ! is_post_type_viewable( $post->post_type ) ) { | ||
| return; |
There was a problem hiding this comment.
Good idea to exit if the post type isn't viewable.
| // Make sure original post is restored after applying shortcodes which could change it. | ||
| $GLOBALS['post'] = $post; // WPCS: override ok. | ||
| setup_postdata( $post ); | ||
| if ( $validation_status_post ) { |
There was a problem hiding this comment.
Could this be an else block?
if ( ! $validation_status_post ) {
return;
} else {
$validation_errors = json_decode( $validation_status_post->post_content, true );
}There was a problem hiding this comment.
Actually, there's no need for the if at all here. Good point.
* Fixes problems related to the_content filters being applied in the admin. * Eliminates validation errors from being displayed when the_content filters apply when creating new posts. * Reduces influx of support topics related to warning messages; the debug link is not helpful unless theme support is present.
westonruter
force-pushed
the
update/warning-notice
branch
from
871bf4f
to
801b9cf
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
the_contentfilters being applied in the admin. See amp plugin causing inability to view posts #1130.the_contentfilters apply when creating new posts.To test:
git clone --recursive https://github.com/Automattic/amp-wp.git amp cd amp git checkout update/warning-notice npm install composer install npm run buildThis will create an
amp.zipfile that you can install on your site. Note you'll have to first deactivate and uninstall the v0.7.0 AMP plugin before you can upload and activate this ZIP on the plugins admin screen. Ideally you could test this on a staging site.Support tickets: