Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#38821 closed defect (bug) (fixed)

REST API: Consider removing `karma` from Comment endpoint

Reported by: dd32's profile dd32 Owned by: rachelbaker's profile rachelbaker
Milestone: 4.7 Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch has-unit-tests
Focuses: rest-api Cc:

Description

The Comments API currently exposes the core comment_karma field, which WordPress hasn't internally used for at least 8 years now.

The comment_karma field can only be set by users with the moderate_comments cap - this is a new precedent for WordPress in that regard, as we don't set that field anywhere. It's not exactly possible (although it can be assumed quite easily) to know which users should (or shouldn't) be allowed to access the field.

I'd like to propose that we skip shipping this as part of the comments api (setting/viewing/querying) - it's a late change, but it's a vestige from WordPress's past (that even I couldn't find a ticket reference for) which has only been used by plugins (if anything) in the last decade. It should be up to those plugins to expose this field in whatever makes sense for them.

Attached is a patch that just removes it from the endpoint.

Attachments (2)

38821.diff (11.4 KB) - added by dd32 8 years ago.
38821.2.diff (9.4 KB) - added by danielbachhuber 8 years ago.
Remove all traces of karma

Download all attachments as: .zip

Change History (16)

@dd32
8 years ago

#1 @dd32
8 years ago

but it's a vestige from WordPress's past (that even I couldn't find a ticket reference for) which has only been used by plugins (if anything) in the last decade.

Correction; WordPress has never used it, it was inherited from b2/cafelog which also didn't yet use it.

#2 @swissspidy
8 years ago

If the field is removed, would adding it back be as easy as using register_rest_field()?.

This ticket was mentioned in Slack in #core by helen. View the logs.


8 years ago

#4 @jnylen0
8 years ago

  • Keywords needs-unit-tests added

It makes sense to me to remove this field.

38821.diff needs test suite fixes and no longer applies cleanly.

If the field is removed, would adding it back be as easy as using register_rest_field()?

Yes, here is the code needed to do that:

<?php
add_action( 'rest_api_init', function() {
    register_rest_field( 'comment', 'karma', array(
        'get_callback' => function( $comment_arr ) {
            $comment_obj = get_comment( $comment_arr['id'] ); 
            return (int) $comment_obj->comment_karma;
        },
        'update_callback' => function( $karma, $comment_obj ) {
            $ret = wp_update_comment( array(
                'comment_ID'    => $comment_obj->comment_ID,
                'comment_karma' => $karma
            ) );
            if ( false === $ret ) {
                return new WP_Error( 'rest_comment_karma_failed', __( 'Failed to update comment karma.' ), array( 'status' => 500 ) );
            }
            return true;
        },
        'schema' => array(
            'description' => __( 'Comment karma.' ),
            'type'        => 'integer'
        ),
    ) );
} );

Caveats:

  • permissions checks not included
  • false === $ret may not catch all wp_update_comment error cases; see also #38700
Last edited 8 years ago by jnylen0 (previous) (diff)

This ticket was mentioned in Slack in #core-restapi by rachelbaker. View the logs.


8 years ago

#6 @rachelbaker
8 years ago

  • Keywords 2nd-opinion removed

Comments are a mess. The karma property is part of the WP_Comment object, and also can be used as a query param in WP_Comment_Query. I understand the cap check here does not have an equal precedent.
I am fine removing this property from the endpoint in the interest of time.

#7 @rachelbaker
8 years ago

  • Keywords has-patch needs-refresh added; dev-feedback removed

@danielbachhuber
8 years ago

Remove all traces of karma

#8 @danielbachhuber
8 years ago

  • Keywords has-unit-tests added; needs-unit-tests needs-refresh removed

#9 @danielbachhuber
8 years ago

  • Owner set to joehoyle
  • Status changed from new to assigned

#10 @rachelbaker
8 years ago

  • Owner changed from joehoyle to rachelbaker

#11 @rachelbaker
8 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 39292:

REST API: Remove the karma property and query parameter from the Comments endpoints.

WordPress has not used the karma property internally for the past 8 years. There is no need to expose it in the REST API endpoints. Sites that use karma can include it using the register_rest_field() function.

Props dd32, danielbachhuber.
Fixes #38821.

This ticket was mentioned in Slack in #docs by jnylen. View the logs.


8 years ago

This ticket was mentioned in Slack in #core-restapi by jnylen. View the logs.


8 years ago

This ticket was mentioned in Slack in #core-comments by ispencer. View the logs.


7 years ago

Note: See TracTickets for help on using tickets.