[wp-trac] [WordPress Trac] #42069: Saving metadata fails (randomly) if equal value already exists

WordPress Trac noreply at wordpress.org
Thu Oct 11 19:48:15 UTC 2018


#42069: Saving metadata fails (randomly) if equal value already exists
-------------------------------------------------+-------------------------
 Reporter:  JVel                                 |       Owner:  (none)
     Type:  defect (bug)                         |      Status:  new
 Priority:  normal                               |   Milestone:  5.0
Component:  REST API                             |     Version:  4.8.2
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch needs-testing has-unit-    |     Focuses:  rest-api
  tests                                          |
-------------------------------------------------+-------------------------

Comment (by boonebgorges):

 I don't think this is ready to commit yet. The `test_boolean_update` tests
 are failing for me:

 ```
 There were 2 failures:

 1)
 WP_Test_REST_Post_Meta_Fields::test_update_value_return_success_with_same_value
 with data set #4 ('test_boolean_update', '')
 Failed asserting that 400 matches expected 200.

 /home/bgorges/sites/wpdev/tests/phpunit/tests/rest-api/rest-post-meta-
 fields.php:1209

 2)
 WP_Test_REST_Post_Meta_Fields::test_update_value_return_success_with_similar_value
 with data set #0 ('test_boolean_update', 0, '')
 Failed asserting that 400 matches expected 200.

 /home/bgorges/sites/wpdev/tests/phpunit/tests/rest-api/rest-post-meta-
 fields.php:1247

 ```

 The problem seems to be that the `test_boolean_update` meta key is
 registered with `type=boolean` (and `sanitize_callback=absint`, which
 doesn't seem like a proper match). As such, passing values like `1` or
 `"1"` is causing this check to fail
 https://core.trac.wordpress.org/browser/tags/4.9.8/src/wp-includes/rest-
 api/fields/class-wp-rest-meta-fields.php#L149  (@dcavins alludes to this
 above https://core.trac.wordpress.org/ticket/42069#comment:17)

 If I understand the design correctly, the strict checks that are coupled
 with proper use of `register_meta()` mean that we should not be testing
 for loosely-equal values in this way. In other words, meta values are not
 exposed via the REST endpoints unless they're registered with a strict
 `type`, and thus we don't support non-strict updates. If that's correct,
 then I believe the proper patch is [attachment:"42069.6.diff"].

 This could use a review from @dcavins (as I may be misunderstanding the
 idea behind this part of his patches) as well as a member of the REST API
 team.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/42069#comment:24>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list