Skip to content

🐛 has control scope #35

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
GautierDele merged 2 commits into from
Jul 27, 2025
Merged

🐛 has control scope #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd52464
🐛 has control scope
GautierDele Jul 27, 2025
847c61f
Apply fixes from StyleCI
StyleCIBot Jul 27, 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/Controls/HasControl.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public static function bootHasControl()
*
* @return Control|null The newly created control instance, or null if creation was unsuccessful.
*/
protected function newControl(): ?Control
public function newControl(): ?Control
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 27, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider alternative approaches to exposing this method for testing.

While the method functionality is correct, changing visibility from protected to public exposes internal implementation as part of the public API. This could lead to unintended usage and maintenance burden.

Consider these alternatives:

  • Use reflection in tests to access the protected method
  • Create a dedicated testing trait or helper method
  • Add a @internal annotation if this must remain public

If keeping it public, add documentation clarifying its intended use:

 /**
  * Attempts to create a new control instance.
  *
+ * @internal This method is primarily intended for internal use and testing.
+ *           External usage is not recommended and may change without notice.
  * @return Control|null The newly created control instance, or null if creation was unsuccessful.
  */
 public function newControl(): ?Control
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public function newControl(): ?Control
/**
* Attempts to create a new control instance.
*
* @internal This method is primarily intended for internal use and testing.
* External usage is not recommended and may change without notice.
* @return Control|null The newly created control instance, or null if creation was unsuccessful.
*/
public function newControl(): ?Control
🤖 Prompt for AI Agents 
In src/Controls/HasControl.php at line 24, the method newControl() was changed
from protected to public, exposing internal implementation unnecessarily. To fix
this, revert the method visibility back to protected and access it in tests
using reflection or a dedicated testing trait/helper. If it must remain public,
add a @internal annotation and documentation clarifying it is not part of the
public API and intended only for internal/testing use.

{
return Access::controlForModel(static::class);
}
Expand Down
28 changes: 28 additions & 0 deletions tests/Feature/ControlsScopeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

use Lomkit\Access\Tests\Support\Models\Model;

class ControlsScopeTest extends \Lomkit\Access\Tests\Feature\TestCase
{
public function test_control_controlled_scope(): void
{
Model::factory()
->count(50)
->create();

$query = Model::controlled()->get();

$this->assertEquals(0, $query->count());
}

public function test_control_uncontrolled_scope(): void
{
Model::factory()
->count(50)
->create();

$query = Model::uncontrolled();

$this->assertContains(\Lomkit\Access\Controls\HasControlScope::class, $query->removedScopes());
}
}