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

[Pattern Directory]: Add categories endpoint #45749

Merged
merged 4 commits into from
Nov 16, 2022

Conversation

ntsekouras
Copy link
Contributor

What?

This PR adds a new endpoint in pattern directory controller for categories. Extracted from #45237.

Testing Instructions

Test the API endpoint in a REST client (/wp-json/wp/v2/pattern-directory/categories). It should match the pattern categories in pattern directory.

@ntsekouras ntsekouras added Core REST API Task Task for Core REST API efforts [Feature] Pattern Directory The Pattern Directory, a place to find patterns labels Nov 14, 2022
@ntsekouras ntsekouras self-assigned this Nov 14, 2022
@codesandbox
Copy link

codesandbox bot commented Nov 14, 2022

CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

@ntsekouras ntsekouras force-pushed the add/pattern-directory-categories-endpoint branch from a1beb4b to ae38a3f Compare November 15, 2022 10:23
Comment on lines +89 to +111
public function test_prepare_pattern_category_for_response() {
$raw_categories = array(
(object) array(
'id' => 3,
'name' => 'Columns',
'slug' => 'columns',
'description' => 'A description',
),
);

$prepared_category = static::$controller->prepare_response_for_collection(
static::$controller->prepare_pattern_category_for_response( $raw_categories[0], new WP_REST_Request() )
);

$this->assertSame(
array(
'id' => 3,
'name' => 'Columns',
'slug' => 'columns',
),
$prepared_category
);
}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should use assertions similar to the ::prepare_item_for_response test - https://github.com/WordPress/wordpress-develop/blob/master/tests/phpunit/tests/rest-api/rest-pattern-directory-controller.php#L324-L334. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it makes any difference to load from a json file, since we're only checking if it preserves the props we want to and we don't make any request. That's why I also didn't use assertArrayNotHasKey, because it makes sense to test what to expect and not if just a key isn't there.

Copy link
Member

Choose a reason for hiding this comment

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

I think that's okay. But core endpoints are probably expected to have the schema defined so we can add that assertion.

I don't have a strong opinion here; I'm just trying to make sure backports don't require many changes.

Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

The new endpoint works as expected.

Besides my two new suggestions, getting a second opinion on adding and using schema in unit tests would be nice.

@ntsekouras ntsekouras force-pushed the add/pattern-directory-categories-endpoint branch from 5c43edf to f4d8e09 Compare November 16, 2022 10:30
@ntsekouras ntsekouras merged commit 768e01a into trunk Nov 16, 2022
@ntsekouras ntsekouras deleted the add/pattern-directory-categories-endpoint branch November 16, 2022 11:18
@github-actions github-actions bot added this to the Gutenberg 14.7 milestone Nov 16, 2022
@DaisyOlsen DaisyOlsen added the [Type] Enhancement A suggestion for improvement. label Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core REST API Task Task for Core REST API efforts [Feature] Pattern Directory The Pattern Directory, a place to find patterns [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants