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

  • New Features

    • Grant and revoke operations now return a success boolean.
    • Added new public methods for listing relations and applying batch authorization writes (placeholders for pending implementation).
  • Behavior Changes

    • Permissions are no longer automatically copied when a model is duplicated; duplicates will not inherit original permissions.

@coderabbitai
Copy link

coderabbitai bot commented Oct 30, 2025

Walkthrough

Grant/revoke methods now return bool and accept wider user types in fakes; the fake manager implements ManagerInterface and adds stub listRelations/write methods; tuple callback type hints updated; the HasAuthorization trait no longer registers a replicated model event.

Changes

Cohort / File(s) Summary
FakeOpenFga implementation
src/Testing/FakeOpenFga.php
grant() and revoke() signatures changed from : void to : bool; both now return true after updating internal tuples. Inner array_filter callback for deletions types the tuple parameter.
Fake manager / wrapper
src/Testing/FakesOpenFga.php
Wrapper now implements ManagerInterface. grant() / revoke() accept `string
Authorization trait
src/Traits/HasAuthorization.php
Removed static::replicated event registration from initializeHasAuthorization(), disabling automatic permission replication on model duplication.

Sequence Diagram(s)

(Skipped — changes are signature/type adjustments and an event removal; no new control-flow interaction requiring a sequence diagram.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Inspect grant() / revoke() return-value usage across tests and callers to ensure callers don't rely on void semantics.
  • Verify the widened string|array $user handling in FakesOpenFga matches real manager behavior.
  • Review stub listRelations() and write() for expected future API surface (exceptions vs. concrete stubs).
  • Confirm tuple typing in array_filter callbacks maintains intended behavior and does not introduce strict type issues.
  • Check removal of the replicated callback doesn't reintroduce the original error or break expected replication workflows.

Poem

🐰
I hopped through tuples, typed and neat,
Grant returns true — a tidy feat.
Stubs await their scheduled play,
Replicated hops have hopped away.
A rabbit cheers the tidy suite! 🎉

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
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.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Delete replicated event and fix test suite" directly and specifically summarizes the primary changes in the pull request. It clearly identifies the main objective of removing the non-existent replicated event that was causing the reported error in issue #35, and references the related test suite improvements. The title is concise, meaningful, and gives reviewers an accurate understanding of what the changeset addresses without being vague or overly broad.
Linked Issues Check ✅ Passed The pull request directly addresses the primary coding requirement from issue #35 by removing the replicated event callback from the HasAuthorization trait that was causing the "Call to undefined method" error. The changes to FakeOpenFga.php and FakesOpenFga.php that update method return types from void to bool and implement proper interface compliance are part of the stated objective to fix the test suite. The PR documentation update request mentioned in the issue is acknowledged but is not a coding requirement for this fix. All critical coding objectives from the linked issue have been met.
Out of Scope Changes Check ✅ Passed All changes in the pull request are directly related to the stated objectives. The removal of the replicated event from HasAuthorization.php is the core fix for issue #35, while the method signature updates in FakeOpenFga.php and FakesOpenFga.php are explicitly described as part of fixing the test suite (reflected in the PR title). The PR author notes they intentionally did not apply additional contract methods, demonstrating controlled scope. The changes are focused, targeted improvements rather than extraneous modifications.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

📜 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 aa707d5.

📒 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)
🔇 Additional comments (4)
src/Testing/FakeOpenFga.php (1)

629-631: LGTM!

The typed array parameter in the filter callback improves type safety and is consistent with the similar change in the revoke() method.

src/Testing/FakesOpenFga.php (3)

7-9: LGTM!

The added imports are necessary for implementing ManagerInterface and typing the write() method parameters.


124-124: LGTM!

Implementing ManagerInterface formalizes the contract and ensures the fake manager provides all required methods with correct signatures.


134-141: Signature changes align with ManagerInterface.

The updated method signatures correctly match the ManagerInterface contract (accepting string|array $user and returning bool). However, these methods delegate to FakeOpenFga methods that only accept string, which will cause runtime errors if arrays are passed. Please ensure the underlying FakeOpenFga::grant() and FakeOpenFga::revoke() methods are updated to handle string|array as well.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 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 aa707d5 and 5b2e2d6.

📒 Files selected for processing (1)
  • src/Testing/FakesOpenFga.php (4 hunks)
🧰 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/FakesOpenFga.php
🧬 Code graph analysis (1)
src/Testing/FakesOpenFga.php (3)
src/Traits/HasAuthorization.php (2)
  • grant (160-169)
  • revoke (258-267)
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)
🔇 Additional comments (4)
src/Testing/FakesOpenFga.php (4)

7-9: LGTM: Required imports for interface implementation.

The imports are necessary for implementing ManagerInterface and declaring the write method signature with TupleKeysInterface.


124-124: LGTM: Interface implementation ensures contract compliance.

Implementing ManagerInterface on the anonymous class ensures all required methods are present and type-checked at compile time.


215-218: LGTM: Exception properly marks the method as unimplemented.

Throwing RuntimeException clearly indicates this method is not yet implemented in the fake and prevents silent failures. This addresses the past review concern about missing return statements.


220-223: LGTM: Exception properly marks the method as unimplemented.

Throwing RuntimeException clearly indicates this method is not yet implemented in the fake and prevents silent failures. This addresses the past review concern about missing return statements.

@oleghalin
Copy link
Author

Closed cause of #71

@oleghalin oleghalin closed this Oct 30, 2025
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