Skip to content

Commit

Permalink
Better handle sitatuations where a user's role on a team is undefined (
Browse files Browse the repository at this point in the history
…#905)

* Better handle sitatuations where a user's role on a team is undefined

In some situations (especially when testing using database factories), the "role" column on the team_user table may be left NULL. As a result, attempting to check the user's permissions based on their role causes an error, as `Jetstream::findRole()` requires a string.

For example:

```php
use App\Models\Team;
use App\Models\User;

$team = Team::factory()
	->with(User::factory())
	->create();

$team->users->first()->hasTeamPermission($team, 'read');
```

This commit ensures that `Jetstream::findRole()` is only called if we have a corresponding role, preventing this error. Similarly, `teamPermissions()` will no longer cause a "ErrorException: Attempt to read property "permissions" on null" exception when the role is undefined.

* Be more explicit with the the autoload definitions to account for case-sensitive filesystems

Running locally on macOS, PHP can find `Database\Factories\TeamFactory` in `database/factories/TeamFactory.php`, but on case-sensitive systems (like Linux, used by the CI pipeline and hosts everywhere) PSR-4 becomes even more important.
  • Loading branch information
stevegrunwell authored Oct 28, 2021
1 parent eb9c161 commit ab149e8
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 6 deletions.
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@
"autoload-dev": {
"psr-4": {
"Laravel\\Jetstream\\Tests\\": "tests/",
"App\\": "stubs/app/"
"App\\": "stubs/app/",
"Database\\Factories\\": "database/factories/"
}
},
"extra": {
Expand Down
14 changes: 9 additions & 5 deletions src/HasTeams.php
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ public function belongsToTeam($team)
* Get the role that the user has on the team.
*
* @param mixed $team
* @return \Laravel\Jetstream\Role
* @return \Laravel\Jetstream\Role|null
*/
public function teamRole($team)
{
Expand All @@ -140,9 +140,13 @@ public function teamRole($team)
return;
}

return Jetstream::findRole($team->users->where(
'id', $this->id
)->first()->membership->role);
$role = $team->users
->where('id', $this->id)
->first()
->membership
->role;

return $role ? Jetstream::findRole($role) : null;
}

/**
Expand Down Expand Up @@ -179,7 +183,7 @@ public function teamPermissions($team)
return [];
}

return $this->teamRole($team)->permissions;
return (array) optional($this->teamRole($team))->permissions;
}

/**
Expand Down
112 changes: 112 additions & 0 deletions tests/HasTeamsTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
<?php

namespace Laravel\Jetstream\Tests;

use App\Actions\Jetstream\CreateTeam;
use App\Models\Team;
use App\Models\User;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Laravel\Jetstream\Jetstream;
use Laravel\Jetstream\OwnerRole;
use Laravel\Jetstream\Role;
use Laravel\Jetstream\Tests\Fixtures\User as UserFixture;

class HasTeamsTest extends OrchestraTestCase
{
use RefreshDatabase;

public function setUp(): void
{
parent::setUp();

Jetstream::$permissions = [];
Jetstream::$roles = [];

Jetstream::useUserModel(UserFixture::class);
}

public function test_teamRole_returns_an_OwnerRole_for_the_team_owner(): void
{
$team = Team::factory()->create();

$this->assertInstanceOf(OwnerRole::class, $team->owner->teamRole($team));
}

public function test_teamRole_returns_the_matching_role(): void
{
Jetstream::role('admin', 'Admin', [
'read',
'create',
])->description('Admin Description');

$team = Team::factory()
->hasAttached(User::factory(), [
'role' => 'admin',
])
->create();
$role = $team->users->first()->teamRole($team);

$this->assertInstanceOf(Role::class, $role);
$this->assertSame('admin', $role->key);
}

public function test_teamRole_returns_null_if_the_user_does_not_belong_to_the_team(): void
{
$team = Team::factory()->create();

$this->assertNull((new UserFixture())->teamRole($team));
}

public function test_teamRole_returns_null_if_the_user_does_not_have_a_role_on_the_site(): void
{
$team = Team::factory()
->has(User::factory())
->create();

$this->assertNull($team->users->first()->teamRole($team));
}

public function test_teamPermissions_returns_all_for_team_owners(): void
{
$team = Team::factory()->create();

$this->assertSame(['*'], $team->owner->teamPermissions($team));
}

public function test_teamPermissions_returns_empty_for_non_members(): void
{
$team = Team::factory()->create();

$this->assertSame([], (new UserFixture())->teamPermissions($team));
}

public function test_teamPermissions_returns_permissions_for_the_users_role(): void
{
Jetstream::role('admin', 'Admin', [
'read',
'create',
])->description('Admin Description');

$team = Team::factory()
->hasAttached(User::factory(), [
'role' => 'admin',
])
->create();

$this->assertSame(['read', 'create'], $team->users->first()->teamPermissions($team));
}

public function test_teamPermissions_returns_empty_permissions_for_members_without_a_defined_role(): void
{
Jetstream::role('admin', 'Admin', [
'read',
'create',
])->description('Admin Description');

$team = Team::factory()
->has(User::factory())
->create();

$this->assertSame([], $team->users->first()->teamPermissions($team));
}
}

0 comments on commit ab149e8

Please sign in to comment.