Skip to content

Update schema information for template controller.#1870

Closed
spacedmonkey wants to merge 14 commits intoWordPress:trunkfrom
spacedmonkey:fix/field-templates
Closed

Update schema information for template controller.#1870
spacedmonkey wants to merge 14 commits intoWordPress:trunkfrom
spacedmonkey:fix/field-templates

Conversation

@spacedmonkey
Copy link
Member

Trac ticket: https://core.trac.wordpress.org/ticket/54422


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

}

if ( is_wp_error( $result ) ) {
if ( 'db_update_error' === $result->get_error_code() ) {
Copy link
Contributor

@adamziel adamziel Nov 15, 2021

Choose a reason for hiding this comment

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

Any chance this could come up because of an invalid input that e.g. violates some db constraint? An example would be trying to insert a duplicated value when a field is UNIQUE – it would appear to be a db error but the problem would actually be with the inputs. This specific one is unlikely here, but you get the idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

Either way, if were get a database error, we should return that.

Copy link
Member

Choose a reason for hiding this comment

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

@adamziel I'm not aware of any such cases, but if they do exist, we should probably update wp_insert_post to handle those errors to return something more descriptive.

@adamziel
Copy link
Contributor

adamziel commented Nov 15, 2021

I read the PR and it looks good and makes sense. I didn't have a chance to test it yet, though. It would be nice to run Gutenberg e2e tests with this PR applied.

@TimothyBJacobs
Copy link
Member

Thanks for putting this together @spacedmonkey. Let's rename the test classes and test files to follow the standards while we're at it.

@hellofromtonya
Copy link
Contributor

Let's rename the test classes and test files to follow the standards while we're at it.

I took care of renaming file and test classes in commit 27c3bb6.

@TimothyBJacobs
Copy link
Member

Thanks @hellofromtonya!

* @covers WP_REST_Templates_Controller::get_items
*/
public function test_get_items() {
function find_and_normalize_template_by_id( $templates, $id ) {
Copy link
Member

Choose a reason for hiding this comment

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

This creates a globally scoped function, we can make this an anonymous function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants