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

Fix #628 by enabling opt-in to nullable scopes #644

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 48 additions & 2 deletions src/Database/Scope/Scope.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ class Scope implements ScopeContract
*/
protected $scopeRoleAbilities = true;

/**
* Determines whether queries should check null scopes
* when there is a scope set.
*
* @var bool
*/
protected $allowNullScope = false;

/**
* Scope queries to the given tenant ID.
*
Expand Down Expand Up @@ -74,6 +82,18 @@ public function dontScopeRoleAbilities()
return $this;
}

/**
* Don't include null scope in queries.
*
* @return $this
*/
public function allowNullScope()
{
$this->allowNullScope = true;

return $this;
}

/**
* Append the tenant ID to the given cache key.
*
Expand All @@ -93,7 +113,7 @@ public function appendToCacheKey($key)
*/
public function applyToModel(Model $model)
{
if (! $this->onlyScopeRelations && ! is_null($this->scope)) {
if ($this->shouldApplyScopeToModel()) {
$model->scope = $this->scope;
}

Expand All @@ -109,7 +129,7 @@ public function applyToModel(Model $model)
*/
public function applyToModelQuery($query, $table = null)
{
if ($this->onlyScopeRelations) {
if ($this->shouldApplyScopeToModelQuery()) {
return $query;
}

Expand All @@ -129,6 +149,10 @@ public function applyToModelQuery($query, $table = null)
*/
public function applyToRelationQuery($query, $table)
{
if (is_null($this->scope) && $this->allowNullScope) {
return $query;
}

return $this->applyToQuery($query, $table);
}

Expand Down Expand Up @@ -250,4 +274,26 @@ protected function isRoleClass($className)
{
return Models::classname(Role::class) === $className;
}

/**
* Determine of the scope should be applied to the model.
*
* @return bool
*/
protected function shouldApplyScopeToModel()
{
return ! $this->onlyScopeRelations &&
($this->allowNullScope || ! is_null($this->scope));
}

/**
* Determine if the scope should be applied to the model query.
*
* @return bool
*/
public function shouldApplyScopeToModelQuery()
{
return $this->onlyScopeRelations ||
($this->allowNullScope && is_null($this->scope));
}
}
2 changes: 2 additions & 0 deletions tests/Concerns/TestsConsoleCommands.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ protected function laravel()
return new $class($arguments['input'], $arguments['output']);
});

$laravel->shouldReceive('runningUnitTests')->andReturn(false);

return $laravel;
}

Expand Down
45 changes: 45 additions & 0 deletions tests/MultiTenancyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,26 @@ function scoped_abilities_do_not_work_when_unscoped($provider)
$this->assertFalse($bouncer->can('read'));
}

/**
* @test
* @dataProvider bouncerProvider
*/
function scoped_abilities_work_when_unscoped_and_allowed($provider)
{
list($bouncer, $user) = $provider();

$bouncer->scope()->to(1)->allowNullScope();
$bouncer->allow($user)->to(['write', 'read']);

$this->assertTrue($bouncer->can('write'));
$this->assertTrue($bouncer->can('read'));
$this->assertEquals(2, $user->abilities()->count());

$bouncer->scope()->to(null);
$this->assertTrue($bouncer->can('write'));
$this->assertTrue($bouncer->can('read'));
}

/**
* @test
* @dataProvider bouncerProvider
Expand Down Expand Up @@ -351,6 +371,31 @@ function assigning_and_retracting_roles_scopes_them_properly($provider)
$this->assertFalse($bouncer->is($user)->an('admin'));
}

/**
* @test
* @dataProvider bouncerProvider
*/
function assigning_and_retracting_roles_scopes_them_properly_when_allowing_null_scopes($provider)
{
list($bouncer, $user) = $provider();

$bouncer->scope()->to(1)->onlyRelations()->allowNullScope();
$bouncer->assign('admin')->to($user);

$bouncer->scope()->to(2);
$bouncer->assign('admin')->to($user);
$bouncer->retract('admin')->from($user);

$bouncer->scope()->to(1);
$this->assertTrue($bouncer->is($user)->an('admin'));

$bouncer->scope()->to(2);
$this->assertFalse($bouncer->is($user)->an('admin'));

$bouncer->scope()->to(null);
$this->assertTrue($bouncer->is($user)->an('admin'));
}

/**
* @test
* @dataProvider bouncerProvider
Expand Down
Loading