Right check for wp_update_comment (WP_REST_Comments_Controller::update_item)

[enrico.sorcinelli]

The WP_REST_Comments_Controller::update_item method assumes that the wp_update_comment function returns also false value in order to check if the update fails.
This is never true since wp_update_comment currently only returns 1 or 0 (and, if #39732 will be accepted, it could return also a WP_Errorobject)

The attached patch fixes the check, even if the downside is that if no DB rows are updated, the method above return a WP_Error.

Maybe have we to work into wp_update_comment allowing it to return also false value?

[kadamwhite]

Per @jnylen0 in slack the current behavior is unintuitive, but correct; tagging him to clarify further

[jnylen0]

Hi @enrico.sorcinelli - the current behavior is correct, though the code could definitely be improved.

There are several possible return paths from wp_update_comment:

• 0 - invalid comment ID. Handled here in the API code.
• 0 - invalid comment post ID. Handled here in the API code.
• 0 - no DB rows updated. This is not an error and should not be treated as one.
• 1 - DB row updated successfully.
• false - the low-level DB update failed, for example here or here. This indicates a problem with the server, so it should be a HTTP 500 error as it is currently.

The REST API should accept an update that does not actually change anything - this is not an error condition. It's unfortunate that wp_update_comment uses 0 as the return value for both an error and a success. We work around this in the REST API by detecting the possible error conditions first. Then we know that 0 actually represents a success.

See #38700 for more details and previous changes here.

We can improve this code by first changing wp_update_comment to have more sensible return values, as you've proposed in #39732. Let's move the discussion over there.

Finally, if there are missing test cases for any of the conditions I outlined above, I'd like to get them added.

[SergeyBiryukov]

In 48235:

Comments: Make wp_update_comment() return false instead of 0 for an invalid comment or post ID.

This addresses an inconsistency where 0 could mean one of the three scenarios:

• Invalid comment ID.
• Invalid comment post ID.
• No DB rows updated. This is not an error and should not be treated as one.

With this change, wp_update_comment() always returns either false or a WP_Error object on failure, depending on the value of the $wp_error parameter.

Follow-up to [48154], [48215], [48216], [48218], [48230].

Props dd32, jnylen0, enrico.sorcinelli.
Fixes #39732. See #38700, #39735.