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

Jump to bottom

Add AMP icon if AMP is enabled on single error page #1473

Merged
merged 11 commits into from Sep 27, 2018
Merged

Add AMP icon if AMP is enabled on single error page #1473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
cfdb689
Add AMP icon on single URL page if AMP enabled
jacobschweitzer Sep 27, 2018
1ff63e5
Check for post ID
jacobschweitzer Sep 27, 2018
5b69c62
Fix dropdown style
jacobschweitzer Sep 27, 2018
7f49ccc
Fix test by creating post earlier in method
jacobschweitzer Sep 27, 2018
25acc88
Merge remote-tracking branch 'origin/develop' into add/amp-icon-if-en…
jacobschweitzer Sep 27, 2018
6ecb3fe
Improvements from code review
jacobschweitzer Sep 27, 2018
0c4c24e
Simplify get_post call and remove post ID check
jacobschweitzer Sep 27, 2018
ab53e0e
Remove un-needed parameters
jacobschweitzer Sep 27, 2018
d3b56ed
Update test to use global $post
jacobschweitzer Sep 27, 2018
02dd7aa
Test fixes
jacobschweitzer Sep 27, 2018
cf399ef
Test fix
jacobschweitzer Sep 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions assets/css/amp-validation-single-error-url.css
View file
Open in desktop
Expand Up @@ -149,3 +149,7 @@ table.striped > tbody > tr.even {
height: 30px;
border-right: 1px solid #a0a5aa;
}
.amp-validation-error-status {
width: auto;
float: none;
}
16 changes: 15 additions & 1 deletion assets/js/amp-invalid-url-post-edit-screen.js
View file
Open in desktop
Expand Up @@ -7,7 +7,8 @@ const ampInvalidUrlPostEditScreen = ( function() { // eslint-disable-line no-unu
unsaved_changes: '',
showing_number_errors: '',
page_heading: '',
show_all: ''
show_all: '',
amp_enabled: false
}
}
};
Expand Down Expand Up @@ -41,6 +42,7 @@ const ampInvalidUrlPostEditScreen = ( function() { // eslint-disable-line no-unu
component.handleBulkActions();
component.changeHeading();
component.watchForUnsavedChanges();
component.showAMPIconIfEnabled();
};

/**
Expand Down Expand Up @@ -391,5 +393,17 @@ const ampInvalidUrlPostEditScreen = ( function() { // eslint-disable-line no-unu
}
};

/**
* Adds the AMP icon to the page heading if AMP is enabled on this URL.
*/
component.showAMPIconIfEnabled = function() {
const heading = document.querySelector( 'h1.wp-heading-inline' );
if ( heading && true === component.data.l10n.amp_enabled ) {
const ampIcon = document.createElement( 'span' );
ampIcon.classList.add( 'status-text', 'sanitized' );
heading.appendChild( ampIcon );
}
};

return component;
}() );
82 changes: 56 additions & 26 deletions includes/validation/class-amp-invalid-url-post-type.php
View file
Open in desktop
Expand Up @@ -396,34 +396,14 @@ public static function get_invalid_url_validation_errors( $url, $args = array()
* }
*/
public static function display_invalid_url_validation_error_counts_summary( $post, $args = array() ) {
$args = array_merge(
$args = array_merge(
array(
'display_enabled_status' => false,
),
$args
);
$counts = array_fill_keys(
array( 'new_accepted', 'ack_accepted', 'new_rejected', 'ack_rejected' ),
0
);

$validation_errors = self::get_invalid_url_validation_errors( $post );
foreach ( $validation_errors as $error ) {
switch ( $error['term']->term_group ) {
case AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_REJECTED_STATUS:
$counts['new_rejected']++;
break;
case AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_ACCEPTED_STATUS:
$counts['new_accepted']++;
break;
case AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACK_ACCEPTED_STATUS:
$counts['ack_accepted']++;
break;
case AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACK_REJECTED_STATUS:
$counts['ack_rejected']++;
break;
}
}
$counts = self::count_invalid_url_validation_errors( $validation_errors );

$result = array();
if ( $counts['new_rejected'] ) {
Expand Down Expand Up @@ -460,9 +440,8 @@ public static function display_invalid_url_validation_error_counts_summary( $pos
}

if ( $args['display_enabled_status'] ) {
$are_there_unaccepted_errors = ( $counts['new_rejected'] || $counts['ack_rejected'] );
$is_amp_enabled = ! $are_there_unaccepted_errors;
$class = $is_amp_enabled ? 'sanitized' : 'new';
$is_amp_enabled = self::is_amp_enabled_on_post( $post );
$class = $is_amp_enabled ? 'sanitized' : 'new';
?>
<span id="amp-enabled-icon" class="status-text <?php echo esc_attr( $class ); ?>">
<?php
Expand Down Expand Up @@ -1543,11 +1522,13 @@ public static function add_edit_post_inline_script() {
return;
}

$post = get_post();
$data = array(
'l10n' => array(
'unsaved_changes' => __( 'You have unsaved changes. Are you sure you want to leave?', 'amp' ),
'page_heading' => self::get_single_url_page_heading(),
'show_all' => __( 'Show all', 'amp' ),
'amp_enabled' => self::is_amp_enabled_on_post( $post ),
),
);

Expand Down Expand Up @@ -1993,7 +1974,7 @@ public static function get_single_url_page_heading() {
}

// Mainly uses the same conditionals as print_status_meta_box().
$post = get_post( intval( $_GET['post'] ) ); // WPCS: CSRF OK.
$post = get_post();
$queried_object = get_post_meta( $post->ID, '_amp_queried_object', true );
$name = __( 'Single URL', 'amp' ); // Default.
if ( isset( $queried_object['type'] ) && isset( $queried_object['id'] ) ) {
Expand Down Expand Up @@ -2130,4 +2111,53 @@ public static function filter_bulk_post_updated_messages( $messages, $bulk_count

return $messages;
}

/**
* Is AMP Enabled on Post
*
* @param WP_Post $post Post object to check.
*
* @return bool|void
*/
public static function is_amp_enabled_on_post( $post ) {
if ( empty( $post ) ) {
return;
}

$validation_errors = self::get_invalid_url_validation_errors( $post );
$counts = self::count_invalid_url_validation_errors( $validation_errors );
$are_there_unaccepted_errors = ( $counts['new_rejected'] || $counts['ack_rejected'] );
return ! $are_there_unaccepted_errors;
}

/**
* Count Invalid URL Validation Errors
*
* @param array $validation_errors Validation errors.
*
* @return array
*/
protected static function count_invalid_url_validation_errors( $validation_errors ) {
$counts = array_fill_keys(
array( 'new_accepted', 'ack_accepted', 'new_rejected', 'ack_rejected' ),
0
);
foreach ( $validation_errors as $error ) {
switch ( $error['term']->term_group ) {
case AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_REJECTED_STATUS:
$counts['new_rejected']++;
break;
case AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_ACCEPTED_STATUS:
$counts['new_accepted']++;
break;
case AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACK_ACCEPTED_STATUS:
$counts['ack_accepted']++;
break;
case AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACK_REJECTED_STATUS:
$counts['ack_rejected']++;
break;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

This foreach loop is duplicated with display_invalid_url_validation_error_counts_summary. I suggest adding a protected method for count_invalid_url_validation_errors() which you pass the $validation_errors array both here and in display_invalid_url_validation_error_counts_summary.

return $counts;
}
}
29 changes: 17 additions & 12 deletions tests/validation/test-class-amp-invalid-url-post-type.php
View file
Open in desktop
Expand Up @@ -1059,9 +1059,14 @@ public function test_render_link_to_error_index_screen() {
* @covers \AMP_Invalid_URL_Post_Type::add_edit_post_inline_script()
*/
public function test_add_edit_post_inline_script() {
global $pagenow;
global $pagenow, $post;

$pagenow = 'post.php';
$pagenow = 'post.php';
$amp_invalid_url_post = $this->factory()->post->create_and_get( array( 'post_type' => AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG ) );
$test_post = $this->factory()->post->create_and_get();

$post = $amp_invalid_url_post;
$_GET['post'] = $amp_invalid_url_post->ID;
set_current_screen( AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG );
AMP_Invalid_URL_Post_Type::enqueue_edit_post_screen_scripts();
AMP_Invalid_URL_Post_Type::add_edit_post_inline_script();
Expand All @@ -1081,23 +1086,20 @@ public function test_add_edit_post_inline_script() {
$this->assertContains( strval( $total_errors ), $inline_script );

// The 'page_heading' value should be present in the inline script.
$amp_invalid_url_post = $this->factory()->post->create_and_get( array( 'post_type' => AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG ) );
$post = $this->factory()->post->create_and_get();
$_GET['post'] = $amp_invalid_url_post->ID;
$_GET['action'] = 'edit';
$_GET['action'] = 'edit';
update_post_meta(
$amp_invalid_url_post->ID,
'_amp_queried_object',
array(
'type' => 'post',
'id' => $post->ID,
'id' => $test_post->ID,
)
);
AMP_Invalid_URL_Post_Type::add_edit_post_inline_script();
$after_script = wp_scripts()->registered[ AMP_Invalid_URL_Post_Type::EDIT_POST_SCRIPT_HANDLE ]->extra['after'];
$inline_script = end( $after_script );
$this->assertContains(
sprintf( 'Errors for: %s', $post->post_title ),
sprintf( 'Errors for: %s', $test_post->post_title ),
$inline_script
);
$this->assertContains( 'Show all', $inline_script );
Expand Down Expand Up @@ -1425,8 +1427,9 @@ public function test_filter_dashboard_glance_items() {
* @covers \AMP_Invalid_URL_Post_Type::get_single_url_page_heading()
*/
public function test_get_single_url_page_heading() {
global $post;
$meta_key = '_amp_queried_object';
$post = $this->factory()->post->create_and_get();
$test_post = $this->factory()->post->create_and_get();
$amp_invalid_url_post = $this->factory()->post->create_and_get( array( 'post_type' => AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG ) );

// If $pagenow is not post.php, this should not filter the labels.
Expand All @@ -1438,21 +1441,23 @@ public function test_get_single_url_page_heading() {
$this->assertEmpty( AMP_Invalid_URL_Post_Type::get_single_url_page_heading() );

// Though $_GET['post'] and $_GET['action'] are now set, but the post type is 'post', so this should not filter the labels.
$_GET['post'] = $post->ID;
$post = $test_post;
$_GET['post'] = $test_post->ID;
$_GET['action'] = 'edit';
$this->assertEmpty( AMP_Invalid_URL_Post_Type::get_single_url_page_heading() );

$_GET['post'] = $amp_invalid_url_post->ID;
$post = $amp_invalid_url_post;
update_post_meta(
$amp_invalid_url_post->ID,
$meta_key,
array(
'type' => 'post',
'id' => $post->ID,
'id' => $test_post->ID,
)
);
$this->assertEquals(
sprintf( 'Errors for: %s', $post->post_title ),
sprintf( 'Errors for: %s', $test_post->post_title ),
AMP_Invalid_URL_Post_Type::get_single_url_page_heading()
);

Expand Down