Skip to content
This repository was archived by the owner on Nov 7, 2025. It is now read-only.

Conversation

@oleghalin
Copy link

@oleghalin oleghalin commented Oct 30, 2025

Pull Request

Summary

Delete replicated event and fix test suite

Fixes #35

Type

  • Bug fix (non-breaking)
  • New feature (non-breaking)
  • Breaking change
  • Documentation update
  • Test improvement
  • Performance improvement

Implementation

Deleted replicated event this it doesnt exist in default laravel framework
Fixed types and implemented fake inline class

I didn't applied missed contract methods in this PR

Testing

Checklist

  • Passing static analysis (PHPStan/Psalm)
  • Documentation updated (if needed)
  • Follows PSR-12 coding standards
  • Commit messages follow conventional commits format

Summary by CodeRabbit

  • Refactor
    • Grant and revoke authorization operations now return success status for improved feedback and error handling
    • Testing utilities enhanced with extended method signatures for authorization operations
    • Removed automatic permission replication when duplicating models

@coderabbitai
Copy link

coderabbitai bot commented Oct 30, 2025

Walkthrough

Updates return types of grant/revoke methods from void to bool across testing interfaces, adds new method stubs to ManagerInterface implementation, expands parameter types to accept string|array, and removes a non-existent Laravel model event hook from the HasAuthorization trait initialization.

Changes

Cohort / File(s) Summary
Testing Infrastructure Updates
src/Testing/FakesOpenFga.php, src/Testing/FakeOpenFga.php
Added ManagerInterface implementation and new method stubs (listRelations, write) to wrapper; updated grant/revoke to return bool instead of void; expanded first parameter to accept string|array; improved type-hinting in internal closures
Trait Event Hook Removal
src/Traits/HasAuthorization.php
Removed static::replicated(...) event handler from initialization that was attempting to hook into non-existent Laravel model lifecycle event

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • FakesOpenFga.php requires attention for interface contract compliance, new unimplemented method stubs (listRelations, write throwing RuntimeException), and parameter type widening from string to string|array
  • FakeOpenFga.php changes are straightforward return type additions but verify consistency with wrapper implementation
  • HasAuthorization.php is a simple removal but confirm the hook was not relied upon elsewhere and validates the issue resolution

Poem

🐰 Hoppy news, my friends so keen,
Grant and revoke now true they've been!
Bool returns where void once stood,
Replicated hooks removed for good. 🔨
The trait now hops without a care,
Laravel events beyond compare!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.77% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The title "Fix/test suite and eloquent events" references real components changed in the PR—the test infrastructure (FakeOpenFga, FakesOpenFga) and eloquent events (HasAuthorization trait)—which are accurate aspects of the changeset. However, the title is vague about the specific nature of these fixes; a reader scanning the commit history would not clearly understand that the primary change is removing the problematic replicated event or that grant/revoke now return bool. The use of "Fix/" as a prefix without clarifying what is being fixed makes the title generic and imprecise. Consider revising the title to be more specific, such as "Remove replicated event from HasAuthorization and update test infrastructure to implement ManagerInterface" or "Fix HasAuthorization by removing unsupported replicated event and align test fake with contract methods." This would clearly communicate the main change to future readers reviewing the commit history.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Out of Scope Changes Check ✅ Passed All changes in the PR align with the stated objectives: removing the replicated event (directly addresses issue #35), implementing ManagerInterface in the test fake (supporting the test suite fix), and adding return types to grant/revoke methods (aligning test infrastructure with expected contracts). The note in the PR description that "missed contract methods" were not applied in this PR suggests the author intentionally scoped the work. No changes appear unrelated to either the core issue fix or the test suite improvements mentioned in the PR objectives.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1ea8f4a and 5b2e2d6.

📒 Files selected for processing (3)
  • src/Testing/FakeOpenFga.php (3 hunks)
  • src/Testing/FakesOpenFga.php (4 hunks)
  • src/Traits/HasAuthorization.php (0 hunks)
💤 Files with no reviewable changes (1)
  • src/Traits/HasAuthorization.php
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.php: Use PHP 8.3+ with strict typing (declare(strict_types=1))
Use modern PHP features such as constructor property promotion, readonly properties, and #[Override] attributes
Always use named parameters whenever possible for clarity, rather than positional parameters
Apply comprehensive Rector rules for code modernization
Maintain PHPStan level 1 and Psalm level 1 for type safety
Maintain strict typing and add proper PHPDoc annotations

**/*.php: Use PHP 8.3+ with strict typing (declare(strict_types=1))
Use modern PHP features such as constructor property promotion, readonly properties, and #[Override] attributes
Always use named parameters whenever possible (rather than positional parameters) for clarity
Apply comprehensive Rector rules for code modernization
Maintain strict typing and add proper PHPDoc annotations for type safety

Files:

  • src/Testing/FakeOpenFga.php
  • src/Testing/FakesOpenFga.php
🧬 Code graph analysis (2)
src/Testing/FakeOpenFga.php (3)
src/Testing/FakesOpenFga.php (2)
  • grant (134-137)
  • revoke (139-142)
src/Traits/HasAuthorization.php (2)
  • grant (160-169)
  • revoke (258-267)
src/Contracts/ManagerInterface.php (2)
  • grant (58-63)
  • revoke (133-138)
src/Testing/FakesOpenFga.php (2)
src/Testing/FakeOpenFga.php (2)
  • grant (403-408)
  • revoke (555-560)
src/Contracts/ManagerInterface.php (4)
  • grant (58-63)
  • revoke (133-138)
  • listRelations (96-103)
  • write (147-151)

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • PSR-12: Entity not found: Issue - Could not find referenced Issue.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to initialize HasAuthorization trait

1 participant