-
Notifications
You must be signed in to change notification settings - Fork 384
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
Update validation model with auto-accepted (instead of forcibly sanitized) and statuses for new-accepted/new-rejected #1429
Merged
Merged
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
fe33e95
Rename force-sanitization to auto-accept-sanitization
westonruter aed7380
Split new status into new-accepted and new-rejected
westonruter 8f6a26e
Update options related to auto-sanitization
westonruter 58d03c9
Update term list table styling for validation errors
westonruter de7b04b
Eliminate forced sanitization with special with_option case; incorpor…
westonruter a3d442d
Add support for querying multiple term statuses at a time; use for New
westonruter c2193fb
Merge branch 'develop' of https://github.com/Automattic/amp-wp into u…
westonruter 4375a40
Update display of edit post screen notices for AMP validation errors
westonruter 5bd1ea7
Merge branch 'develop' of https://github.com/Automattic/amp-wp into u…
westonruter 1ed50f2
Partially revert de7b04b to restore omission of forcibly-sanitized er…
westonruter b057e46
Merge branch 'develop' of https://github.com/Automattic/amp-wp into u…
westonruter e8e550e
WIP
westonruter 06e3d4d
Introduce AMP_Validation_Error_Taxonomy::sanitize_term_status() to de…
westonruter c1e0658
Refactor duplicated SQL prepare for IN condition
westonruter e5274ac
Remove amp attribute from html element on non-AMP documents
westonruter d334f2a
Show preview button on amp_invalid_url screen in native mode
westonruter 0d6781e
Add link to validation errors screen for bulk acceptance
westonruter 9493858
Fix PHP 5.2 error with call to protected method in closure
westonruter 70e9cbf
Add wp amp reset-site-validation command to purge site of validation …
westonruter 85d1641
Eliminate trashing for invalid URL posts
westonruter 3ea0df5
Add AMP_Validation_Error_Taxonomy::get_term() method to reduce code d…
westonruter b5fc798
Discontinue hiding validation errors that have no URL counts
westonruter 827503b
Add ability to clear empty validation error terms
westonruter f8f7db0
Merge branch 'develop' of https://github.com/Automattic/amp-wp into u…
westonruter e8c4e8a
Update status constants which were missed in merge conflict resolution
westonruter 273a00b
Make single-error-detail open by default
westonruter a272ce6
Merge branch 'develop' of https://github.com/Automattic/amp-wp into u…
westonruter 945e763
Show accepted/rejected in new status option and disable it
westonruter 276c5a5
Make the 'Status' column wider, to fit the icon and <select>
kienstra cd20106
Remove a (trivial) artifiact from a merge conflict
kienstra File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -116,5 +116,5 @@ table.striped > tbody > tr.even { | |
} | ||
|
||
.wp-list-table th.column-status { | ||
width: 15%; | ||
width: 20%; | ||
} |
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -263,6 +263,69 @@ public function validate_site( $args, $assoc_args ) { | |
WP_CLI::line( sprintf( __( 'For more details, please see: %s', 'amp' ), $url_more_details ) ); | ||
} | ||
|
||
/** | ||
* Reset all validation data on a site. | ||
* | ||
* This deletes all amp_invalid_url posts and all amp_validation_error terms. | ||
* | ||
* ## OPTIONS | ||
* | ||
* [--yes] | ||
* : Proceed to empty the site validation data without a confirmation prompt. | ||
* | ||
* ## EXAMPLES | ||
* | ||
* wp amp reset-site-validation --yes | ||
* | ||
* @subcommand reset-site-validation | ||
* @param array $args Positional args. Unused. | ||
* @param array $assoc_args Associative args. | ||
* @throws Exception If an error happens. | ||
*/ | ||
public function reset_site_validation( $args, $assoc_args ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great idea, this is something I've had to do manually with longer WP-CLI commands to delete the error posts and terms. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
unset( $args ); | ||
global $wpdb; | ||
WP_CLI::confirm( 'Are you sure you want to empty all amp_invalid_url posts and amp_validation_error taxonomy terms?', $assoc_args ); | ||
|
||
// Delete all posts. | ||
$count = $wpdb->get_var( $wpdb->prepare( "SELECT COUNT(*) FROM $wpdb->posts WHERE post_type = %s", AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG ) ); | ||
$query = $wpdb->prepare( "SELECT ID FROM $wpdb->posts WHERE post_type = %s", AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG ); | ||
$posts = new WP_CLI\Iterators\Query( $query, 10000 ); | ||
|
||
$progress = WP_CLI\Utils\make_progress_bar( | ||
/* translators: %d is the number of posts */ | ||
sprintf( __( 'Deleting %d amp_invalid_url posts...', 'amp' ), $count ), | ||
$count | ||
); | ||
while ( $posts->valid() ) { | ||
$post_id = $posts->current()->ID; | ||
$posts->next(); | ||
wp_delete_post( $post_id, true ); | ||
$progress->tick(); | ||
} | ||
$progress->finish(); | ||
|
||
// Delete all terms. Note that many terms should get deleted when their post counts go to zero above. | ||
$count = $wpdb->get_var( $wpdb->prepare( "SELECT COUNT( * ) FROM $wpdb->term_taxonomy WHERE taxonomy = %s", AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG ) ); | ||
$query = $wpdb->prepare( "SELECT term_id FROM $wpdb->term_taxonomy WHERE taxonomy = %s", AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG ); | ||
$terms = new WP_CLI\Iterators\Query( $query, 10000 ); | ||
|
||
$progress = WP_CLI\Utils\make_progress_bar( | ||
/* translators: %d is the number of terms */ | ||
sprintf( __( 'Deleting %d amp_taxonomy_error terms...', 'amp' ), $count ), | ||
$count | ||
); | ||
while ( $terms->valid() ) { | ||
$term_id = $terms->current()->term_id; | ||
$terms->next(); | ||
wp_delete_term( $term_id, AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG ); | ||
$progress->tick(); | ||
} | ||
$progress->finish(); | ||
|
||
WP_CLI::success( 'All AMP validation data has been removed.' ); | ||
} | ||
|
||
/** | ||
* Gets the total number of URLs to validate. | ||
* | ||
|
@@ -586,7 +649,7 @@ public static function validate_and_store_url( $url, $type ) { | |
$validation_errors, | ||
function( $error ) { | ||
$validation_status = AMP_Validation_Error_Taxonomy::get_validation_error_sanitization( $error ); | ||
return AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS !== $validation_status['term_status']; | ||
return AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACK_ACCEPTED_STATUS !== $validation_status['term_status']; | ||
} | ||
) ); | ||
|
||
|
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
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
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
Oops, something went wrong.
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.
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.
In a different CSS file, what do you think about changing this width to
20%
:https://github.com/Automattic/amp-wp/blob/e67234d86375cbc3454416e84365219eab1d12df/assets/css/amp-validation-single-error-url.css#L118-L120
The 'status' icon sometimes appears above the
<select>
when the<select>
is wider. Though this happens for all of them at slightly more narrow screen widths.This
width: 20%
seems to work well in my testing so far: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.
This styling issue isn't caused by this PR, it's just more apparent when the
<select>
has longer text, like'New Rejected'
instead of'Rejected'
.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.
Applied in 276c5a5