Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up image editor REST route. #23368

Merged

Conversation

TimothyBJacobs
Copy link
Member

Description

  • Changes the route to image-editor.
  • Returns proper translated errors with status codes.
  • On success, returns the new media item's full response.
  • Updates apiFetch call to use data instead of manually constructing the JSON body.

How has this been tested?

Manually tested editing an image.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

- Changes the route to `image-editor`.
- Returns proper translated errors with status codes.
- On success, returns the new media item's full response.
- Updates apiFetch call to use `data` instead of manually constructing the JSON body.
@TimothyBJacobs TimothyBJacobs self-assigned this Jun 22, 2020
@TimothyBJacobs TimothyBJacobs added [Block] Image Affects the Image Block REST API Interaction Related to REST API labels Jun 22, 2020
@TimothyBJacobs TimothyBJacobs mentioned this pull request Jun 22, 2020
12 tasks
@@ -44,22 +44,22 @@ public function register_routes() {
'permission_callback' => array( $this, 'permission_callback' ),
'args' => array(
'x' => array(
'type' => 'float',
'type' => 'number',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange, using number failed for me before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, it seems to work, but what happens then? Is it rounded? Ideally we accept float values here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A float is referred to as a number in JSON Schema, https://json-schema.org/draft/2019-09/json-schema-core.html#rfc.section.4.2.1.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More accurate link ( its the v4 spec which we use ) https://tools.ietf.org/html/draft-zyp-json-schema-04#section-3.5

@@ -44,22 +44,22 @@ public function register_routes() {
'permission_callback' => array( $this, 'permission_callback' ),
'args' => array(
'x' => array(
'type' => 'float',
'type' => 'number',
'minimum' => 0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it also possible to specify a maximum?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep we can use the maximum property.

Copy link
Contributor

@ajlende ajlende left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't mean to do some of the same changes in #23369. We'll get your stuff merged and I can adjust. Everything was working when I pulled this down and tried it out. 👍

@TimothyBJacobs TimothyBJacobs merged commit cc4aa89 into WordPress:master Jun 22, 2020
@github-actions github-actions bot added this to the Gutenberg 8.5 milestone Jun 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Image Affects the Image Block REST API Interaction Related to REST API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants