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

fix is_wp_version_compatible() to accommodate incorrect format x.x.0 #5677

Closed
wants to merge 41 commits into from
Closed
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
f70cd67
fix is_wp_version_compatible() to accommodate incorrect format x.x.0
afragen Nov 16, 2023
a012763
wpcs fix
afragen Nov 16, 2023
e41c513
update test to return wp_trigger_error
afragen Nov 16, 2023
3e6a7bd
check for string before throwing error
afragen Nov 16, 2023
603dcf9
use E_USER_NOTICE, update test
afragen Nov 16, 2023
e08ac43
refactor check, update $version before wp_trigger_error
afragen Nov 16, 2023
4eb9190
add more tests
afragen Nov 16, 2023
54e7518
fix substr_count
afragen Nov 16, 2023
3cad191
use error_log so function doesn't exit
afragen Nov 16, 2023
ea216d5
I think this now works with wp_trigger_error
afragen Nov 16, 2023
93b0126
Revert "I think this now works with wp_trigger_error"
afragen Nov 16, 2023
fc88bb5
more tests
afragen Nov 16, 2023
7098415
use wp_trigger_error
afragen Jan 25, 2024
1d731cb
Merge branch 'trunk' into 59448
afragen Jan 25, 2024
d54c5d6
remover error handler for test that returns wp_trigger_error and reset
afragen Jan 25, 2024
e45ddce
improve version number corrector
afragen Jan 25, 2024
68b7f48
improve warning message
afragen Jan 25, 2024
3781e42
update related tests to use proper version number
afragen Jan 25, 2024
85845cd
use same exclusion as wpUniquePrefixedId.php:158
afragen Jan 26, 2024
bcbff68
revert notice and changes to other REST test
afragen Jan 29, 2024
1f236e3
add new test for silent fix
afragen Jan 29, 2024
c5d38ce
cleanup dataset
afragen Jan 29, 2024
014d45d
keep fixed test data for rest-plugins-controller
afragen Jan 29, 2024
27a5a64
re-format
afragen Jan 29, 2024
c7d07f0
more cleanup
afragen Jan 29, 2024
0482621
Add notice for incorrect version numbering.
Jan 29, 2024
4e7367b
Move valid version numbering to the main test.
Jan 29, 2024
3ff62db
Implement a notice check.
Jan 29, 2024
e4f52ae
Adjust and add datasets to the notice test.
Jan 29, 2024
e1429fe
F: notice check
Jan 29, 2024
bd5a45d
Merge pull request #9 from costdev/is_wp_version_compatible/add_notic…
afragen Jan 29, 2024
0cfcdeb
update translator comment
afragen Jan 29, 2024
bccd14f
refactor function for trailing whitespace add more datasets
afragen Jan 29, 2024
5549256
Remove notice from function.
peterwilsoncc Feb 21, 2024
fdda8f0
Update tests for unofficial version numbers to remove notice expectat…
peterwilsoncc Feb 21, 2024
4dcf147
Remove unused variable following 55492561b4d9dbd690a198dd941cb8bbd54e…
peterwilsoncc Feb 21, 2024
b6a1f8f
Remove trailing value with substr rather than regex.
peterwilsoncc Feb 21, 2024
4edb63d
Merge branch 'trunk' into 59448
peterwilsoncc Feb 21, 2024
0d27643
Remove tearDown following 55492561b4d9dbd690a198dd941cb8bbd54e3706
peterwilsoncc Feb 21, 2024
d11dbe3
Fix unit test description following removal of notice.
peterwilsoncc Feb 25, 2024
c5b3a67
Merge branch 'trunk' into 59448
peterwilsoncc Feb 25, 2024
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
10 changes: 10 additions & 0 deletions src/wp-includes/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -8716,6 +8716,16 @@ function is_wp_version_compatible( $required ) {
// Strip off any -alpha, -RC, -beta, -src suffixes.
list( $version ) = explode( '-', $wp_version );

if ( is_string( $required ) && substr_count( $required, '.' ) > 1 && str_ends_with( $required, '.0' ) ) {
error_log(
Copy link
Member

Choose a reason for hiding this comment

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

This should use wp_trigger_error

Copy link
Member Author

Choose a reason for hiding this comment

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

I was initially having issues with tests not passing due to the wp_trigger_error() message being returned but found a solution by removing the error handler before the test and resetting afterwards.

__FUNCTION__ . '(): ' .
/* translators: s: version string sent to function */
sprintf( __( '`%s` Not a valid WordPress version string.' ), $required )
Copy link
Member

Choose a reason for hiding this comment

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

It would be helpful to present both what was passed in and what the result was. What do you think of this instead?

"Invalid version 6.0.0 passed to is_wp_version_compatible, 6.0 is assumed."

);

$required = rtrim( $required, '.0' );
}

return empty( $required ) || version_compare( $version, $required, '>=' );
}

Expand Down
26 changes: 26 additions & 0 deletions tests/phpunit/tests/functions/isWpVersionCompatible.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,32 @@ public function data_is_wp_version_compatible() {
'required' => array(),
'expected' => true,
),

// Improper WP version.
'improper trailing x.x.0' => array(
'required' => '5.2.0',
'expected' => true,
),
'correct version' => array(
'required' => '5.2',
'expected' => true,
),
'incorrect trailing x.0.0' => array(
'required' => '5.0.0',
'expected' => true,
),
'correct version x.0' => array(
'required' => '5.0',
'expected' => true,
),
'correct version x.0.1' => array(
'required' => '5.0.1',
'expected' => true,
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth adding a test to ensure the version is converted correctly? It might require some code shuffling.

Copy link
Contributor

Choose a reason for hiding this comment

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

Provided there are tests covering all the bases, testing the output with each input should be enough, and the respective lines can be checked for coverage in a coverage report.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added this test to ensure a .0 in the middle of a version number wouldn’t be changed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will add some new tests for that. I'll have it fixed by tomorrow.

'correct version x.1.1' => array(
'required' => '5.1.1',
'expected' => true,
),
);
}

Expand Down