WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#35276 closed defect (bug) (fixed)

Allow `comment_author_IP` and `comment_agent` keys to be updated via `wp_update_comment()`

Reported by: rachelbaker Owned by: boonebgorges
Milestone: 4.5 Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

Comments created via wp_insert_comment() can pass in values for the comment_author_IP and comment_agent keys, but those keys cannot be updated via wp_update_comment().

Use case:
Sally uses a REST API endpoint to create a comment. When Sally receives the successful response with her new Comment object from the REST API, Sally realizes she made a mistake. She has typos in the values for: comment_author_IP and comment_agent. Sally tries to send a PUT request to correct her typos, and receives a mean error message saying those properties are not able to be updated. Sally is sad.

Attachments (3)

35276.diff (949 bytes) - added by adamsilverstein 5 years ago.
don't be sad sally
35276.2.diff (2.4 KB) - added by welcher 5 years ago.
Adding unit tests so Sally can be sure she won't be sad about something else.
35276.3.diff (2.4 KB) - added by adamsilverstein 5 years ago.

Download all attachments as: .zip

Change History (14)

@adamsilverstein
5 years ago

don't be sad sally

#1 @adamsilverstein
5 years ago

I think all we need is to add these keys ( 35276.diff )? worth adding a unit test to show it works.

Related: #14601

#2 @rachelbaker
5 years ago

  • Keywords has-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to 4.5

#3 @dd32
5 years ago

although there's definately use-cases where you want these to be modifiable by a client, these are also internal values at present which cannot be modified through any UI.
They feel like they should be set by the server based on it's called environment and never be updated, which is probably why the code is like it is.

For example, would you want any regular authenticated user who can comment to be able to modify the IP or user agent that their comments are stored as? (Note: I don't know if the REST API currently plans on limiting it somehow)

#4 @dd32
5 years ago

Previously: #14601

#5 @rachelbaker
5 years ago

@dd32 If a value can be added/set programmatically then why not prevent it from being programmatically modified? I always thought it was weird that in wp_filter_comment() the pre_comment_user_ip and pre_comment_user_agent filters trigger but are useless when a comment is being updated.

The comment functions were built around the idea that a comment would ONLY be created from a comment form displayed on the front-end by a theme. Values like the comment author's IP and user/agent could be inferred from $_SERVER vars. With comments being created from a API client, I would argue that not only is it important to allow the internal values of comment_author_IP and comment_user_agent to be set (which wp_insert_comment() always allowed), but to be corrected *programmatically* if needed.

For example, would you want any regular authenticated user who can comment to be able to modify the IP or user agent that their comments are stored as? (Note: I don't know if the REST API currently plans on limiting it somehow)

Editing any of the comment properties in the REST API requires both the moderate_comments and edit_comment capabilities.

@welcher
5 years ago

Adding unit tests so Sally can be sure she won't be sad about something else.

#6 @welcher
5 years ago

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

#7 @adamsilverstein
5 years ago

Thanks @welcher!

35276.3.diff adds a doc-block header for the test so we can run it directly and also to link the test back to the original ticket

#8 @danielbachhuber
5 years ago

Something I've been thinking about recently with regards to whether or not fields should be editable: import / export. If someone were to build a import / export workflow based on the REST API, which seems totally reasonable to me, it seems like a reasonable assumption they should be able to update most (?) any fields for Resources (Posts, Comments, Terms, Users).

I don't have a great suggestion for it at the moment, but maybe we need some paradigm or abstraction for a mode in which any field can be edited (including modified date, too).

#9 @joehoyle
5 years ago

I'm strongly in favour of adding support for updating all fields via wp_update_comment, that's the internal API, not about UI so I don't see any reason why this data is arbitrarily immutable. I think there's even case that a comment update could/should actually trigger changes on these fields (at least potentially). Tying the data APIs of WordPress to the front-end website use-case IMO is not a great idea, therefore it seems that within the context of the underlaying "WordPress framework" of a comment object, all fields should be modifiable programmatically.

#10 @boonebgorges
5 years ago

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

+1 for the change. We would likely never build a core UI that allows users to modify these values, but that's a decision about the design of a *client*, and need not be enforced by the API. wp_update_comment() is the API function and should be flexible for all use cases.

#11 @boonebgorges
5 years ago

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

In 36215:

Allow comment agent and author IP to be set via wp_update_comment().

Props adamsilverstein, welcher.
Fixes #35276.

Note: See TracTickets for help on using tickets.