Skip to content

Fix/control calls #18

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 24, 2025
Merged

Fix/control calls #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
42c548b
🐛 policy calls
GautierDele Apr 24, 2025
b69e9c9
Apply fixes from StyleCI
StyleCIBot Apr 24, 2025
da7a0e7
Update PoliciesTest.php
GautierDele Apr 24, 2025
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
2 changes: 1 addition & 1 deletion src/AccessServiceProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ protected function registerCommands(array $commands)
protected function registerStubs()
{
Event::listen(function (PublishingStubs $event) {
$event->add(realpath(__DIR__.'/Console/stubs/control.stub'), 'controller.stub');
$event->add(realpath(__DIR__.'/Console/stubs/control.stub'), 'control.stub');
$event->add(realpath(__DIR__.'/Console/stubs/perimeter.plain.stub'), 'perimeter.plain.stub');
$event->add(realpath(__DIR__.'/Console/stubs/perimeter.overlay.stub'), 'perimeter.overlay.stub');
});
Expand Down
2 changes: 0 additions & 2 deletions src/Console/ControlMakeCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ protected function getDefaultNamespace($rootNamespace)
/**
* Build the class with the given name.
*
* Remove the base controller import if we are already in the base namespace.
*
* @param string $name
*
* @return string
Expand Down
2 changes: 0 additions & 2 deletions src/Console/PerimeterMakeCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,6 @@ protected function getDefaultNamespace($rootNamespace)
/**
* Build the class with the given name.
*
* Remove the base controller import if we are already in the base namespace.
*
* @param string $name
*
* @return string
Expand Down
1 change: 0 additions & 1 deletion src/Console/stubs/control.stub
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace {{ namespace }};

use {{ rootNamespace }}Http\Controllers\Controller;
use Illuminate\Http\Request;
use Lomkit\Access\Controls\Control;
use Illuminate\Database\Eloquent\Model;
Expand Down
1 change: 0 additions & 1 deletion src/Console/stubs/perimeter.overlay.stub
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace {{ namespace }};

use {{ rootNamespace }}Http\Controllers\Controller;
use Lomkit\Access\Perimeters\OverlayPerimeter;

class {{ class }} extends OverlayPerimeter
Expand Down
1 change: 0 additions & 1 deletion src/Console/stubs/perimeter.plain.stub
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace {{ namespace }};

use {{ rootNamespace }}Http\Controllers\Controller;
use Lomkit\Access\Perimeters\Perimeter;

class {{ class }} extends Perimeter
Expand Down
7 changes: 7 additions & 0 deletions src/Controls/Control.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,13 @@ public static function new(): self
*/
public static function resolveControlName(string $modelName): string
{
// @TODO: The auto guess here is strange, we specify the models / controls everywhere, is there a better way of doing this ? (In policies guess the model as Laravel is doing ?)
// @TODO: Discussed with Lucas G

if (method_exists($modelName, 'control')) {
return $modelName::control()::class;
}
Comment on lines +237 to +242
Copy link
Contributor

@coderabbitai coderabbitai bot Apr 24, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Enhanced control resolution by checking for static control method

Good enhancement! The resolveControlName method now checks if the model class has a static control method and uses it if available, before falling back to the conventional naming resolution.

However, the TODO comments suggest there are concerns about the auto-guessing approach. Consider addressing these TODOs before finalizing this PR or creating follow-up tickets to track these concerns.

🏁 Script executed:

#!/bin/bash
# Check if any models in the codebase implement the static control method
echo "Looking for models that implement static control method..."
rg -l "public static function control\(" --type php

Length of output: 208

🏁 Script executed:

#!/bin/bash
# Inspect the HasControl trait definition
echo "Inspecting HasControl trait:"
rg -Hn "trait HasControl" --type php

# List all references to HasControl to see where it's used
echo "Listing all references to HasControl:"
rg -Hn "HasControl" --type php

Length of output: 677

🏁 Script executed:

#!/bin/bash
# Check for any control() method definitions in HasControl trait
echo "Inspecting HasControl trait for control() methods:"
rg -Hn "function control" -C3 src/Controls/HasControl.php

Length of output: 589

Resolve TODOs for auto-guess behavior in resolveControlName

We’ve confirmed that no application models override the static control() method (only the HasControl trait provides it), so the fallback “auto guess” path remains lightly tested and the TODO comments at lines 237–242 still stand:

// @TODO: The auto guess here is strange, we specify the models / controls everywhere, is there a better way of doing this ? (In policies guess the model as Laravel is doing ?)
// @TODO: Discussed with Lucas G

if (method_exists($modelName, 'control')) {
    return $modelName::control()::class;
}

Please address these by one of the following:

  • Implement a more robust naming strategy (e.g., Laravel‐style policy guessing).
  • Remove or resolve the lingering TODOs.
  • If it’s out of scope for this PR, convert them into tracked tickets for follow-up.


$resolver = static::$controlNameResolver ?? function (string $modelName) {
$appNamespace = static::appNamespace();

Expand Down
14 changes: 7 additions & 7 deletions src/Policies/ControlledPolicy.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ protected function getControl(): Control
*/
public function viewAny(Model $user)
{
return $this->getControl()->should($user, __FUNCTION__, new ($this->getModel()));
return $this->getControl()->applies($user, __FUNCTION__, new ($this->getModel()));
}

/**
Expand All @@ -58,7 +58,7 @@ public function viewAny(Model $user)
*/
public function view(Model $user, Model $model)
{
return $this->getControl()->should($user, __FUNCTION__, $model);
return $this->getControl()->applies($user, __FUNCTION__, $model);
}

/**
Expand All @@ -70,7 +70,7 @@ public function view(Model $user, Model $model)
*/
public function create(Model $user)
{
return $this->getControl()->should($user, __FUNCTION__, new ($this->getModel()));
return $this->getControl()->applies($user, __FUNCTION__, new ($this->getModel()));
}

/**
Expand All @@ -83,7 +83,7 @@ public function create(Model $user)
*/
public function update(Model $user, Model $model)
{
return $this->getControl()->should($user, __FUNCTION__, $model);
return $this->getControl()->applies($user, __FUNCTION__, $model);
}

/**
Expand All @@ -96,7 +96,7 @@ public function update(Model $user, Model $model)
*/
public function delete(Model $user, Model $model)
{
return $this->getControl()->should($user, __FUNCTION__, $model);
return $this->getControl()->applies($user, __FUNCTION__, $model);
}

/**
Expand All @@ -109,7 +109,7 @@ public function delete(Model $user, Model $model)
*/
public function restore(Model $user, Model $model)
{
return $this->getControl()->should($user, __FUNCTION__, $model);
return $this->getControl()->applies($user, __FUNCTION__, $model);
}

/**
Expand All @@ -122,6 +122,6 @@ public function restore(Model $user, Model $model)
*/
public function forceDelete(Model $user, Model $model)
{
return $this->getControl()->should($user, __FUNCTION__, $model);
return $this->getControl()->applies($user, __FUNCTION__, $model);
}
}
104 changes: 104 additions & 0 deletions tests/Feature/PoliciesTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
<?php

namespace Lomkit\Access\Tests\Feature;

use Illuminate\Support\Facades\Gate;
use Lomkit\Access\Tests\Support\Models\Model;
use Lomkit\Access\Tests\Support\Models\User;

class PoliciesTest extends \Lomkit\Access\Tests\Unit\TestCase
{
public function test_policies_calls_view_method_properly(): void
{
Gate::define('view global models', function (User $user) {
return true;
});
$model = Model::factory()
->createOne();
$user = \Illuminate\Support\Facades\Auth::user();

$policy = new \Lomkit\Access\Tests\Support\Policies\ModelPolicy();

$this->assertTrue($policy->view($user, $model));
}

public function test_policies_calls_view_any_method_properly(): void
{
Gate::define('viewAny global models', function (User $user) {
return true;
});
$user = \Illuminate\Support\Facades\Auth::user();

$policy = new \Lomkit\Access\Tests\Support\Policies\ModelPolicy();

$this->assertTrue($policy->viewAny($user));
}

public function test_policies_calls_create_method_properly(): void
{
Gate::define('create global models', function (User $user) {
return true;
});
$user = \Illuminate\Support\Facades\Auth::user();

$policy = new \Lomkit\Access\Tests\Support\Policies\ModelPolicy();

$this->assertTrue($policy->create($user));
}

public function test_policies_calls_update_method_properly(): void
{
Gate::define('update global models', function (User $user) {
return true;
});
$model = Model::factory()
->createOne();
$user = \Illuminate\Support\Facades\Auth::user();

$policy = new \Lomkit\Access\Tests\Support\Policies\ModelPolicy();

$this->assertTrue($policy->update($user, $model));
}

public function test_policies_calls_delete_method_properly(): void
{
Gate::define('delete global models', function (User $user) {
return true;
});
$model = Model::factory()
->createOne();
$user = \Illuminate\Support\Facades\Auth::user();

$policy = new \Lomkit\Access\Tests\Support\Policies\ModelPolicy();

$this->assertTrue($policy->delete($user, $model));
}

public function test_policies_calls_restore_method_properly(): void
{
Gate::define('restore global models', function (User $user) {
return true;
});
$model = Model::factory()
->createOne();
$user = \Illuminate\Support\Facades\Auth::user();

$policy = new \Lomkit\Access\Tests\Support\Policies\ModelPolicy();

$this->assertTrue($policy->restore($user, $model));
}

public function test_policies_calls_force_delete_method_properly(): void
{
Gate::define('forceDelete global models', function (User $user) {
return true;
});
$model = Model::factory()
->createOne();
$user = \Illuminate\Support\Facades\Auth::user();

$policy = new \Lomkit\Access\Tests\Support\Policies\ModelPolicy();

$this->assertTrue($policy->forceDelete($user, $model));
}
}
3 changes: 3 additions & 0 deletions tests/Support/Models/Model.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Illuminate\Database\Eloquent\Model as BaseModel;
use Laravel\Scout\Searchable;
use Lomkit\Access\Controls\HasControl;
use Lomkit\Access\Tests\Support\Access\Controls\ModelControl;
use Lomkit\Access\Tests\Support\Database\Factories\ModelFactory;

class Model extends BaseModel
Expand All @@ -14,6 +15,8 @@ class Model extends BaseModel
use HasControl;
use Searchable;

public static $control = ModelControl::class;

protected static function newFactory()
{
return new ModelFactory();
Expand Down
2 changes: 1 addition & 1 deletion tests/Support/Policies/ModelPolicy.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

namespace Lomkit\Access\Tests\Support\Policies;

use Illuminate\Database\Eloquent\Model;
use Lomkit\Access\Policies\ControlledPolicy;
use Lomkit\Access\Tests\Support\Models\Model;

class ModelPolicy extends ControlledPolicy
{
Expand Down
2 changes: 1 addition & 1 deletion tests/Unit/StubsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public function test_stubs_correctly_registered(): void

$this->assertEquals(
[
realpath(__DIR__.'/../../src/Console/stubs/control.stub') => 'controller.stub',
realpath(__DIR__.'/../../src/Console/stubs/control.stub') => 'control.stub',
realpath(__DIR__.'/../../src/Console/stubs/perimeter.plain.stub') => 'perimeter.plain.stub',
realpath(__DIR__.'/../../src/Console/stubs/perimeter.overlay.stub') => 'perimeter.overlay.stub',
],
Expand Down