Dynamic binding gates #158
Conversation
WalkthroughThe pull request refactors the authorization logic in two key areas. In the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Response
Caller->>Response: buildGatesForModel(gates)
Response->>Response: Map each gate using array_map & array_combine
Response-->>Caller: Return authorization array
sequenceDiagram
participant Client
participant SearchRules
participant GateFacade
Client->>SearchRules: validate(input)
SearchRules->>GateFacade: getPolicyFor(resource::$model)
GateFacade-->>SearchRules: Return policy object
SearchRules->>SearchRules: Filter policy methods (exclude before, after, __call, __construct)
SearchRules-->>Client: Return dynamic validation rules with valid gates
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Http/Response.php (1)
35-38: Cleaner implementation of gate binding with dynamic naming!The refactored implementation elegantly handles dynamic gate binding using array functions. This approach is more maintainable than the previous version and supports custom gate names via configuration.
Consider adding a null check in case
$gatesis empty to avoid potential errors:protected function buildGatesForModel(Model $model, Resource $resource, array $gates) { + if (empty($gates)) { + return []; + } return array_combine( array_map(fn($gate) => config("rest.gates.names.authorized_to_{$gate}", "authorized_to_{$gate}"), $gates), array_map(fn($gate) => $resource->authorizedTo($gate, $model), $gates) ); }src/Rules/SearchRules.php (1)
72-74: Great implementation of dynamic gate discovery!This approach dynamically fetches all available policy methods, making the authorization system more flexible and maintainable. New gates will be automatically recognized without code changes.
Consider extracting this logic to a helper method for reusability and improved readability:
+ /** + * Get valid gates from the resource policy. + * + * @return array + */ + protected function getValidGatesFromPolicy(): array + { + $policy = Gate::getPolicyFor($this->resource::$model); + if (!$policy) { + return []; + } + + return array_filter( + get_class_methods($policy), + fn($method) => !in_array($method, ['before', 'after', '__call', '__construct']) + ); + } public function validate(string $attribute, mixed $value, Closure $fail): void { if ($attribute !== '') { $attribute .= '.'; } - $policy = Gate::getPolicyFor($this->resource::$model); - $validGates = $policy ? get_class_methods($policy) : []; - $validGates = array_filter($validGates, fn($method) => !in_array($method, ['before', 'after', '__call', '__construct'])); + $validGates = $this->getValidGatesFromPolicy();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Http/Response.php(1 hunks)src/Rules/SearchRules.php(3 hunks)
🔇 Additional comments (2)
src/Rules/SearchRules.php (2)
12-12: New Gate facade import supports dynamic policy discovery.This import is necessary for the dynamic policy and gate retrieval implemented below.
97-97:✅ Verification successful
Dynamic validation rule for gates enhances flexibility.
Using the dynamically generated
$validGatesarray instead of a hardcoded list makes the system more flexible and future-proof. New gates will be automatically validated without requiring code changes.Let's check for any potentially missed gates by comparing this dynamic approach with the previous hardcoded approach:
🏁 Script executed:
#!/bin/bash # Find any references to hardcoded gate lists in the codebase echo "Looking for hardcoded gate lists that might need updating..." rg -A 2 -B 2 "viewAny|forceDelete" --type php | grep -v "/vendor/"Length of output: 14615
Dynamic validation rule verified – no additional issues detected
The dynamic approach using
$validGateshas been confirmed to properly encompass the gate values referenced throughout the codebase. The search for hardcoded gate lists (particularly those containing names like"forceDelete","restore","view", etc.) did not reveal any overlooked, hardcoded values. This confirms that new gates will be automatically supported without further code changes.
|
The idea seems really nice but the tests aren't passing 😄 |
closes #156
The purpose of this PR is to automatically bind all the gates of a model (before, only the view, viewAny, delete, restore... methods were available).
It will therefore be possible to add a new authorization (e.g. whether or not the user has the right to download the resource).
What's more, if the gate doesn't have a name in the configuration file, I've made sure to generate one automatically.
It would be a good idea to update the documentation to explain this.
I look forward to your feedback on this feature :)
Summary by CodeRabbit