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

[Sidebars endpoint] Add permissions PHPUnit tests #24784

Merged
merged 10 commits into from
Aug 26, 2020

Conversation

adamziel
Copy link
Contributor

@adamziel adamziel commented Aug 25, 2020

Description

This PR adds coverage for when the user is logged out or has insufficient permissions as per #24290 (review).

How has this been tested?

Confirm all checks pass on this PR.

Types of changes

Non-breaking change

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.

@adamziel adamziel added [Feature] Widgets Screen The block-based screen that replaced widgets.php. [Package] Edit Widgets /packages/edit-widgets labels Aug 25, 2020
@adamziel adamziel self-assigned this Aug 25, 2020
@adamziel adamziel removed the [Package] Edit Widgets /packages/edit-widgets label Aug 25, 2020
@github-actions
Copy link

github-actions bot commented Aug 25, 2020

Size Change: +2.43 kB (0%)

Total Size: 1.16 MB

Filename Size Change
build/annotations/index.js 3.67 kB +1 B
build/autop/index.js 2.82 kB +1 B
build/block-directory/index.js 7.99 kB +20 B (0%)
build/block-editor/index.js 126 kB +271 B (0%)
build/block-library/editor-rtl.css 8.5 kB +4 B (0%)
build/block-library/editor.css 8.5 kB +3 B (0%)
build/block-library/index.js 135 kB +1.98 kB (1%)
build/block-library/style-rtl.css 7.45 kB +23 B (0%)
build/block-library/style.css 7.45 kB +21 B (0%)
build/blocks/index.js 47.7 kB -2 B (0%)
build/compose/index.js 9.67 kB -1 B
build/data-controls/index.js 1.29 kB +1 B
build/data/index.js 8.55 kB -2 B (0%)
build/date/index.js 5.38 kB +2 B (0%)
build/dom/index.js 4.48 kB +2 B (0%)
build/edit-navigation/index.js 11.6 kB +45 B (0%)
build/edit-post/index.js 304 kB -1 B
build/edit-widgets/index.js 11.8 kB +89 B (0%)
build/editor/index.js 45.3 kB -33 B (0%)
build/format-library/index.js 7.71 kB +3 B (0%)
build/is-shallow-equal/index.js 710 B -1 B
build/media-utils/index.js 5.32 kB -1 B
build/nux/index.js 3.4 kB -1 B
build/plugins/index.js 2.56 kB -1 B
build/shortcode/index.js 1.7 kB +1 B
build/url/index.js 4.06 kB +2 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/api-fetch/index.js 3.44 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-editor/style-rtl.css 10.7 kB 0 B
build/block-editor/style.css 10.7 kB 0 B
build/block-library/theme-rtl.css 729 B 0 B
build/block-library/theme.css 730 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/components/index.js 200 kB 0 B
build/components/style-rtl.css 15.7 kB 0 B
build/components/style.css 15.7 kB 0 B
build/core-data/index.js 12.3 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/edit-navigation/style-rtl.css 1.16 kB 0 B
build/edit-navigation/style.css 1.16 kB 0 B
build/edit-post/style-rtl.css 5.61 kB 0 B
build/edit-post/style.css 5.61 kB 0 B
build/edit-site/index.js 17 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/style-rtl.css 3.8 kB 0 B
build/editor/style.css 3.79 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/primitives/index.js 1.41 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.9 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

Copy link
Contributor

@draganescu draganescu left a comment

Choose a reason for hiding this comment

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

Everything is green, let's make it more green.

Copy link
Member

@TimothyBJacobs TimothyBJacobs left a comment

Choose a reason for hiding this comment

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

Thanks for adding these! I left some feedback.

@@ -6,6 +6,15 @@
* @subpackage REST_API
*/

require_once dirname( __FILE__ ) . '/../lib/class-wp-rest-sidebars-controller.php';
add_filter(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
add_filter(
add_action(

Copy link
Member

Choose a reason for hiding this comment

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

Also, why do we need to include this here at all? If it is because the endpoint is in an experiment, I think we should do the same thing we did as the block directory and enable the experiment in the tests

tellyworth@26363cf#diff-85d18a6eb126c0f8c8da280e74555a2c

In phpunit/bootstrap.php.

$GLOBALS['wp_tests_options'] = array(
 	'gutenberg-experiments' => array(
 		'gutenberg-block-directory' => '1',
 	),
 );

phpunit/class-rest-sidebars-controller-test.php Outdated Show resolved Hide resolved
phpunit/class-rest-sidebars-controller-test.php Outdated Show resolved Hide resolved
phpunit/class-rest-sidebars-controller-test.php Outdated Show resolved Hide resolved
@@ -407,4 +457,11 @@ public function test_get_item_schema() {
$this->assertArrayHasKey( 'status', $properties );
$this->assertArrayHasKey( 'widgets', $properties );
}

public function users_without_permissions() {
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this is working as expected? As far as I know, data providers are executed before setup before class, which means that the ids haven't been created yet. This is probably why the tests are passing when checking for a 401.

@adamziel
Copy link
Contributor Author

All good notes @TimothyBJacobs, I just addressed your comments, would you mind re-reviewing?

@TimothyBJacobs TimothyBJacobs self-requested a review August 26, 2020 17:39
Copy link
Member

@TimothyBJacobs TimothyBJacobs left a comment

Choose a reason for hiding this comment

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

Looks great to me!

@draganescu draganescu merged commit 6129e72 into master Aug 26, 2020
@draganescu draganescu deleted the update/phpunit-widget-screen branch August 26, 2020 17:45
@github-actions github-actions bot added this to the Gutenberg 8.9 milestone Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Widgets Screen The block-based screen that replaced widgets.php.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants