Skip to content

Fix deprecation warnings #7614

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

Merged
merged 11 commits into from
Sep 14, 2023
Merged

Fix deprecation warnings #7614

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1bd40ef
Add guard to avoid using add_query_arg() if URL does not exists
thelovekesh Sep 3, 2023
2c43d0d
Fix deprecation warnings due to AMP validation test cases
thelovekesh Sep 3, 2023
020eaaa
Update PHPUnit config to convert deprecations to error
thelovekesh Sep 3, 2023
6e9f2cf
Merge branch 'develop' into fix/deprecation-warning
thelovekesh Sep 12, 2023
308e17c
Update capability for amp_validation_error taxonomy
thelovekesh Sep 12, 2023
1e8de9a
Add title in OptionsMenu tests before rendering settings page
thelovekesh Sep 12, 2023
14b70d7
Add amp-onboarding-wizard submenu page on options.php menu
thelovekesh Sep 12, 2023
41b2ff6
Add options.php as parent slug in onboarding wizard submenu page test…
thelovekesh Sep 12, 2023
091fdfd
Revert "Update capability for amp_validation_error taxonomy"
thelovekesh Sep 14, 2023
6d8a722
Fix typo in inline comments
thelovekesh Sep 14, 2023
e8f73c5
Try increasing navigation timeout for e2e tests to 60000 ms
thelovekesh Sep 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions tests/php/validation/test-class-amp-validation-error-taxonomy.php
View file
Open in desktop
Copy link
Member

@westonruter westonruter Sep 14, 2023

Choose a reason for hiding this comment

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

Is this commit needed to work around the issue in tests which you're fixing in core?

Copy link
Collaborator Author

@thelovekesh thelovekesh Sep 14, 2023

Choose a reason for hiding this comment

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

Which commit? I am unable to see any diff with this comment.

Copy link
Member

@westonruter westonruter Sep 14, 2023

Choose a reason for hiding this comment

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

Sorry, I meant 2c43d0d

Copy link
Collaborator Author

@thelovekesh thelovekesh Sep 14, 2023

Choose a reason for hiding this comment

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

It's different from that one. handle_single_url_page_bulk_and_inline_actions() uses get_edit_post_link() which requires edit_post capability in general. In test cases we were not setting up the user hence it was resulting into deprecation warnings.

Copy link
Member

@westonruter westonruter Sep 14, 2023

Choose a reason for hiding this comment

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

What was the deprecation warning?

Copy link
Collaborator Author

@thelovekesh thelovekesh Sep 14, 2023

Choose a reason for hiding this comment

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

Those were the same as passing null to add_query_arg()

Deprecated: stripos(): Passing null to parameter #1 ($haystack) of type string is deprecated in /tmp/wordpress/src/wp-includes/functions.php on line 1157

Deprecated: str_contains(): Passing null to parameter #1 ($haystack) of type string is deprecated in /tmp/wordpress/src/wp-includes/functions.php on line 1164

Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use AmpProject\AmpWP\Services;
use AmpProject\AmpWP\Tests\Helpers\HandleValidation;
use AmpProject\AmpWP\Tests\TestCase;
use AmpProject\AmpWP\Tests\Helpers\MockAdminUser;

/**
* Tests for AMP_Validation_Error_Taxonomy class.
Expand All @@ -20,6 +21,7 @@
class Test_AMP_Validation_Error_Taxonomy extends TestCase {

use HandleValidation;
use MockAdminUser;

/**
* The tested class.
Expand Down Expand Up @@ -1429,27 +1431,41 @@ static function ( $redirect_url ) use ( &$redirected_url ) {
AMP_Validation_Error_Taxonomy::handle_single_url_page_bulk_and_inline_actions( $incorrect_post_type );
$this->assertEquals( get_term( $error_term->term_id )->term_group, $initial_accepted_status );

// Setup admin user which have edit_posts capability.
$this->mock_admin_user();

// Build the expected URL for the redirect.
$admin_post_url = admin_url( 'post.php' );
$redirect_query_args = [
'post' => $correct_post_type,
'action' => 'edit',
'amp_actioned' => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPT_ACTION,
'amp_actioned_count' => 1,
];

/*
* Although the post type is correct, this should not update the post accepted status to be 'accepted'.
* There should be a warning because wp_safe_redirect() should be called at the end of the tested method.
*/
AMP_Validation_Error_Taxonomy::handle_single_url_page_bulk_and_inline_actions( $correct_post_type );

$this->assertSame( '?action=edit&amp_actioned=amp_validation_error_accept&amp_actioned_count=1', $redirected_url );
$this->assertSame( add_query_arg( $redirect_query_args, $admin_post_url ), $redirected_url );
$this->assertEquals( get_term( $error_term->term_id )->term_group, AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_ACCEPTED_STATUS );

// When the action is to 'reject' the error, this should not update the status of the error to 'rejected'.
$_REQUEST['action'] = AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_REJECT_ACTION;
$_REQUEST['action'] = AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_REJECT_ACTION;
$redirect_query_args['amp_actioned'] = AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_REJECT_ACTION;
AMP_Validation_Error_Taxonomy::handle_single_url_page_bulk_and_inline_actions( $correct_post_type );

$this->assertSame( '?action=edit&amp_actioned=amp_validation_error_reject&amp_actioned_count=1', $redirected_url );
$this->assertSame( add_query_arg( $redirect_query_args, $admin_post_url ), $redirected_url );
$this->assertEquals( get_term( $error_term->term_id )->term_group, AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_ACCEPTED_STATUS );

// When the action is to 'delete' the error, this should delete the error.
$_REQUEST['action'] = 'delete';
$_REQUEST['action'] = 'delete';
$redirect_query_args['amp_actioned'] = 'delete';
AMP_Validation_Error_Taxonomy::handle_single_url_page_bulk_and_inline_actions( $correct_post_type );

$this->assertSame( '?action=edit&amp_actioned=delete&amp_actioned_count=1', $redirected_url );
$this->assertSame( add_query_arg( $redirect_query_args, $admin_post_url ), $redirected_url );
$this->assertEquals( null, get_term( $error_term->term_id ) );
}

Expand Down