Dependencies API tweaks#1
Conversation
|
Size Change: 0 B Total Size: 1.21 MB ℹ️ View Unchanged
|
|
@spacedmonkey I think some of these tests fail as they are executed out of main repo - All PHP stuff tests pass on my local without any issues. |
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
…ey/gutenberg into feature/dependency-api-tweaks
| 'role' => 'author', | ||
| ) | ||
| ); | ||
| self::$subscriber_id = $factory->user->create( |
There was a problem hiding this comment.
We should use this user to test auth. Use a get_item / get_items call as a subscriber
There was a problem hiding this comment.
This has been added. Subscriber cannot view them according to tests.
| 'role' => 'administrator', | ||
| ) | ||
| ); | ||
| self::$editor_id = $factory->user->create( |
There was a problem hiding this comment.
It's been used for tests. Unfortunately editors can't read scripts/styles. The same applies to authors.
| 'role' => 'editor', | ||
| ) | ||
| ); | ||
| self::$author_id = $factory->user->create( |
There was a problem hiding this comment.
Can we test to see you can see block assets and their dependencies as an author.
There was a problem hiding this comment.
It's been used for tests. Unfortunately authors can't read scripts/styles. I think this should be addressed right?
There was a problem hiding this comment.
So tests are executed against not core assets and mentioned roles (authors, editors) can't see it. That's not a case for core assets. Shall we address this to ensure that authors, editors can also read scripts / styles? That doesn't seem to be risky at all.
| $this->assertEquals( home_url( '/test.css' ), $data['src'] ); | ||
| $this->assertEquals( home_url( '/test.css' ), $data['url'] ); | ||
| } | ||
|
|
There was a problem hiding this comment.
Add tests for no user / subscriber / author to see that permissions checks work.
| * Test single block script. | ||
| */ | ||
| public function test_get_item_block_script() { | ||
| foreach ( array( 0, self::$subscriber_id, self::$editor_id, self::$author_id, self::$admin_id, self::$superadmin_id ) as $user_id ) { |
There was a problem hiding this comment.
I don't personally like this style of loop. Can we just use a dataprovider here and other places that this format is used? Using dataprovider really helps in debugging.
There was a problem hiding this comment.
@spacedmonkey I tried to use data providers and they were not delivering the right data - if they return primitives like integers then they work fine. When I tried to return class props they were always set to null. Any idea what I did incorrectly?
|
Barring this fix https://github.com/spacedmonkey/gutenberg/pull/1/files#r514452318, I think we are ready to merge @lukaspawlik 🎉 |
| * Test single style. | ||
| */ | ||
| public function test_get_item() { | ||
| foreach ( array( self::$admin_id, self::$superadmin_id ) as $user_id ) { |
| * Test multiple styles. | ||
| */ | ||
| public function test_get_items() { | ||
| foreach ( array( self::$admin_id, self::$superadmin_id ) as $user_id ) { |
| self::delete_user( self::$superadmin_id ); | ||
| self::delete_user( self::$admin_id ); | ||
| self::delete_user( self::$editor_id ); | ||
| self::delete_user( self::$author_id ); | ||
| self::delete_user( self::$subscriber_id ); |
There was a problem hiding this comment.
| self::delete_user( self::$superadmin_id ); | |
| self::delete_user( self::$admin_id ); | |
| self::delete_user( self::$editor_id ); | |
| self::delete_user( self::$author_id ); | |
| self::delete_user( self::$subscriber_id ); | |
| foreach( self::$users_map as $key => $user_id ){ | |
| self::delete_user( $user_id ); | |
| } |
| /** | ||
| * @var int | ||
| */ | ||
| protected static $admin_id; |
There was a problem hiding this comment.
We can get rid of these class properties as well.
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
…ey/gutenberg into feature/dependency-api-tweaks
|
@spacedmonkey ready for your review (another one 😄 ) |
@spacedmonkey this is still WIP however, it addresses most meaningful feedback found on WordPress#21244