WordPoints / wordpoints

Points plugin for WordPress
GNU General Public License v2.0
20 stars 15 forks source link

Fix new PHPCS errors #324

Closed JDGrimes closed 9 years ago

JDGrimes commented 9 years ago

Updates to WPCS mean we're seeing some new errors.

JDGrimes commented 9 years ago

Fixed for now.

JDGrimes commented 9 years ago

This has broken the Ajax tests:

.F..F...................................

Time: 6.31 seconds, Memory: 76.00Mb

There were 2 failures:

1) WordPoints_Points_Hooks_Order_AJAX_Test::test_as_admin

Failed asserting that two arrays are equal.

--- Expected

+++ Actual

@@ @@

 Array (

     'points' => Array (

-        0 => 'registration_points_hook-1'

-        1 => 'comments_points_hook-1'

+        0 => 'registration_points_hook-1wordpoints_comments_points_hook-1'

     )

 )

/home/travis/build/WordPoints/wordpoints/tests/phpunit/tests/points/ajax/hook-order.php:80

2) WordPoints_Points_Hooks_Order_AJAX_Test::test_network_as_super_admin

Failed asserting that two arrays are equal.

--- Expected

+++ Actual

@@ @@

 Array (

     'points' => Array (

-        0 => 'registration_points_hook-1'

-        1 => 'comments_points_hook-1'

+        0 => 'registration_points_hook-1wordpoints_comments_points_hook-1'

     )

 )

/home/travis/build/WordPoints/wordpoints/tests/phpunit/tests/points/ajax/hook-order.php:169

The problem is this:

        $points_types = array_map( 'sanitize_key', $_POST['points_types'] );

$_POST['points_types'] is an array of arrays, not a simple array. So we need to sanitize differently. I think we should pull the list of points types from the database, and use them to get each sub-array from the POSTed value. The way it works now, the list could contain points types that don't even exist.

JDGrimes commented 9 years ago

$_POST['points_types'] is an array of arrays, not a simple array.

Actually, that is incorrect. It is a simple array. The hooks are a comma-delimited string. The problem is that sanitize_key() is being used, because it strips out the commas. We need to use sanitize_text_field() instead.

I'll still refactor it to use the list of points types from the database.