WordPress.org

Make WordPress Core

Opened 9 months ago

Closed 9 months ago

Last modified 9 months ago

#38857 closed defect (bug) (fixed)

REST API: Merge similar translation strings

Reported by: ramiy Owned by: SergeyBiryukov
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.7
Component: I18N Keywords: has-patch commit
Focuses: rest-api Cc:

Description

In the new Rest API, there are 3 places where we check if the current_user_can() edit posts/terms/users.

We have two permission error messages:

  • Sorry, you are not allowed to edit resource.
  • Sorry, you are not allowed to manage this resource.

Thy can be merged into one strings:

  • Sorry, you are not allowed to edit this resource.

Related: #38808

Attachments (3)

38857.patch (2.2 KB) - added by ramiy 9 months ago.
38857.2.patch (6.4 KB) - added by SergeyBiryukov 9 months ago.
38857.3.patch (5.3 KB) - added by SergeyBiryukov 9 months ago.
Refreshed

Download all attachments as: .zip

Change History (16)

@ramiy
9 months ago

#1 @ramiy
9 months ago

  • Keywords has-patch added

#2 @SergeyBiryukov
9 months ago

  • Milestone changed from Awaiting Review to 4.7

#3 @SergeyBiryukov
9 months ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 39304:

REST API: Merge two similar permission error strings.

Props ramiy.
Fixes #38857.

#4 @SergeyBiryukov
9 months ago

In 39305:

REST API: Merge two similar permission error strings in class-wp-rest-comments-controller.php.

We're checking if current_user_can( 'moderate_comments' ) here, not the specific comment permissions.

See #38857.

#5 @ramiy
9 months ago

I think the last commit affects my patch on #38859.

#6 follow-up: @SergeyBiryukov
9 months ago

  • Keywords commit added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Going to reopen, as there are still some inconsistencies in REST API strings.

Some messages refer to specific resource types, like posts or comments, others refer to a generic "resource". Here's a rest_forbidden_context string from each endpoint:

  • class-wp-rest-comments-controller.php:
    • "Sorry, you are not allowed to view comments with edit context."
  • class-wp-rest-post-types-controller.php:
    • "Sorry, you are not allowed to edit this resource."
  • class-wp-rest-posts-controller.php:
    • "Sorry, you are not allowed to edit posts in this post type."
    • "Sorry, you are not allowed to edit this post."
  • class-wp-rest-taxonomies-controller.php:
    • "Sorry, you are not allowed to edit this resource."
  • class-wp-rest-terms-controller.php:
    • "Sorry, you are not allowed to view this resource with edit context."
  • class-wp-rest-users-controller.php:
    • "Sorry, you are not allowed to view this resource with edit context."

38857.2.patch brings some consistency. Note: it uses two strings that could probably be merged, but I've separated them for clarity, as they refer to different capabilities (edit_terms and manage_terms, respectively):

  • "Sorry, you are not allowed to edit terms in this taxonomy."
  • "Sorry, you are not allowed to manage terms in this taxonomy."

(Not sure what's the difference between those two capabilities though, they appear to be used somewhat interchangeably in core.)

On a related note, some of item schema description strings look weird in terms of translation, for example:

  • "First name for the resource."
  • "The nickname for the resource."

Is there a reason to a use a generic "resource" here (and in other classes) instead of "user"?

#7 @SergeyBiryukov
9 months ago

#38859 was marked as a duplicate.

#8 in reply to: ↑ 6 @ramiy
9 months ago

Replying to SergeyBiryukov:

Some messages refer to specific resource types, like posts or comments, others refer to a generic "resource".

I noticed this too. Not sure if it is intentional. Any way, I like your patch, it clarifies the error messages.

#9 @ramiy
9 months ago

Although I think that your patch needs to be on a ticket dedicated to clarifying error messages (#38859) and not on the current ticket that merges strings.

#10 @SergeyBiryukov
9 months ago

In 39308:

Text Changes: Merge and clarify some permission error strings in the admin.

See #38857.

@SergeyBiryukov
9 months ago

Refreshed

#11 @SergeyBiryukov
9 months ago

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

In 39309:

REST API: Merge and clarify some permission error strings.

Fixes #38857.

#12 @SergeyBiryukov
9 months ago

In 39312:

Text Changes: Merge strings referring to list_users capability.

See #38857.

#13 @SergeyBiryukov
9 months ago

In 39313:

REST API: Merge some more permission error strings missed in [39309].

See #38857.

Note: See TracTickets for help on using tickets.