WordPress.org

Make WordPress Core

Opened 7 months ago

Closed 7 months ago

Last modified 7 months ago

#38811 closed defect (bug) (fixed)

REST API: ambiguous strings

Reported by: Presskopp Owned by: joehoyle
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.7
Component: REST API Keywords: has-patch commit
Focuses: docs Cc:

Description

1) Sorry, you are not allowed to give resource that role.
2) Sorry, you are not allowed to delete resource.
3) Sorry, you are not allowed to update resource.
4) Sorry, you are not allowed to create new resource.
5) Sorry, you are not allowed to filter by role.

Seems there are articles missing or at least would be better.

Example:
https://translate.wordpress.org/projects/wp/dev/en-gb/default?filters%5Boriginal_id%5D=3647187

all in 4.7 dev to find

or see them here while untranslated:
https://translate.wordpress.org/projects/wp/dev/de/default

Attachments (2)

38811.patch (41.5 KB) - added by ramiy 7 months ago.
38811.2.patch (43.1 KB) - added by ramiy 7 months ago.

Download all attachments as: .zip

Change History (19)

#1 @rmccue
7 months ago

  • Focuses rest-api added
  • Owner set to rmccue
  • Status changed from new to assigned

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


7 months ago

#3 @helen
7 months ago

  • Component changed from Text Changes to REST API
  • Focuses docs added; rest-api removed
  • Milestone changed from Awaiting Review to 4.7
  • Summary changed from ambiguous strings to REST API: ambiguous strings

#4 @jnylen0
7 months ago

I think we should also change these (and any other API strings) to say something other than "resource". (Post, user, comment, etc.)

Technically a page is a post, so "post" is probably fine here too.

This ticket was mentioned in Slack in #core-i18n by ramiy. View the logs.


7 months ago

#6 @ramiy
7 months ago

We have many strings in the new Rest API that use "resource" to describe posts / pages / comments / taxonomies / terms / settings / users / etc...

I think we should not use the term "resource". Although it will increase the total strings amount, but on the other hand we will remove the unnecessary confusion.

#8 @jbpaul17
7 months ago

Summarizing the Related chat on Slack from above: those chiming in on #core-restapi were either in support of something more specific than "resource" or were not against it.

@rmccue I can help generate the variations on those strings, just not sure if there's somewhere those should go besides in a comment here in Trac. So, let me know how I can help... thanks!

@ramiy
7 months ago

@ramiy
7 months ago

#9 @ramiy
7 months ago

  • Keywords has-patch added

#10 @joehoyle
7 months ago

This looks good to me, I'm +1 for merging this.

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


7 months ago

#12 follow-up: @joehoyle
7 months ago

This looks good, apart from I don't think we should be using the post word in class-wp-rest-post-types-controller.php.

Technically a page is a post, so "post" is probably fine here too.

This might be true, however via the API, CPTs are not called "posts" anywhere, so this language would confuse things I think.

I think we should either use the post type labels are part of these strings, or still with "resource" in the case of posts.

#13 @rachelbaker
7 months ago

  • Keywords commit added
  • Owner changed from rmccue to joehoyle

@joehoyle I am +1 on 38811.2.patch as well.

#14 in reply to: ↑ 12 @SergeyBiryukov
7 months ago

Replying to joehoyle:

via the API, CPTs are not called "posts" anywhere, so this language would confuse things I think.

I see a lot of "post type" references in the docs in class-wp-rest-post-types-controller.php:

  • Core class to access post types via the REST API.
  • Retrieves all public post types.
  • Retrieves a specific post type.
  • Prepares a post type object for serialization.
  • @param stdClass $post_type Post type data.
  • Filters a post type returned from the API.
  • Allows modification of the post type data right before it is returned.
  • @param object $item The original post type object.
  • Retrieves the post type's schema, conforming to JSON Schema.

So I think 38811.2.patch makes the strings more consistent rather than confusing.

Last edited 7 months ago by SergeyBiryukov (previous) (diff)

#15 @joehoyle
7 months ago

Ugh sorry I put the wrong file in my comment, I meant class-wp-rest-posts-controller.php, however - given we call types "Post Types" then I guess it's ok we refer to "pages" as a post in the error messages; at least maybe it's better than "resource". I'll commit this, and maybe we can look at using post type labels in the future in the posts controller.

#16 @joehoyle
7 months ago

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

In 39342:

REST API: Update “resource” strings to use the appropriate nouns.

Props ramiy.
Fixes #38811.

#17 @danielbachhuber
7 months ago

Just for the record, these changes have made the descriptions more inconsistent than they were before. I used "resource" originally for consistency. Now, because of class inheritance, any given controller is going to have a mix of language.

Context: https://github.com/WP-API/WP-API/issues/2056

Note: See TracTickets for help on using tickets.