Add onlyAuthenticated, guard options and automated tests #3
Conversation
Features: - Add only_authenticated config option (default: true) - Add ->onlyAuthenticated() method for per-panel override - Add guard config option with panel auth guard fallback - Add ->guard() method for per-panel override Testing: - Add Pest test suite with Orchestra Testbench - Add tests for plugin instantiation, configuration fallbacks, and view rendering 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded@falko100 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 16 minutes and 22 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds per-panel and global authentication gating and guard selection to the Userback plugin, updates the Blade view to conditionally render the script based on authentication, and introduces PHPUnit/Pest test infrastructure and tests for the new behavior. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Panel
participant Plugin as UserbackPlugin
participant Config
participant Auth as Auth System
participant View
Note over Panel,Plugin: Boot / render lifecycle
Panel->>Plugin: boot(panel)
Plugin->>Config: read access_token, only_authenticated, guard
Plugin->>Plugin: resolveOnlyAuthenticated() (explicit or config)
Plugin->>Plugin: resolveGuard(panel) (explicit or config or panel guard)
Plugin->>Auth: checkAuthenticated(guard)
alt Access token present AND (not onlyAuthenticated OR authenticated)
Plugin->>View: render(userback.blade.php with token and user data)
View->>Client: inject Userback script + Userback.user_data
else Not allowed / no token
Plugin->>View: render(nothing)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/ConfigurationTest.php (1)
5-83: Consider integration tests instead of reflection-based unit tests.While these tests correctly validate configuration precedence, using reflection to test protected methods couples the tests tightly to implementation details. Refactoring method names or visibility will break these tests.
Consider refactoring to test the public API surface through integration tests that verify the widget's rendering behavior under different configurations. This would be more maintainable and less brittle.
Alternative testing approach
Instead of reflection-based unit tests:
it('renders widget with config token when plugin token not set', function () { config()->set('filament-userback.access_token', 'config-token'); $plugin = UserbackPlugin::make(); $panel = /* create test panel */; $plugin->boot($panel); // Assert the rendered output contains the config token });This tests the actual behavior users experience rather than internal method implementation.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.gitignoreCLAUDE.mdcomposer.jsonconfig/filament-userback.phpphpunit.xmlresources/views/userback.blade.phpsrc/UserbackPlugin.phptests/ConfigurationTest.phptests/Pest.phptests/TestCase.phptests/UserbackPluginTest.phptests/ViewTest.php
🧰 Additional context used
📓 Path-based instructions (2)
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: Usedeclare(strict_types=1)in all PHP files
Follow PSR-12 coding standards
Use fluent interfaces (return$this) for setter methods
Use namespaceCoddinWeb\FilamentUserbackfor all project classes
Files:
config/filament-userback.phpresources/views/userback.blade.phptests/ConfigurationTest.phptests/UserbackPluginTest.phptests/TestCase.phptests/ViewTest.phpsrc/UserbackPlugin.phptests/Pest.php
**/UserbackPlugin.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/UserbackPlugin.php: UserbackPlugin class should register a render hook to inject the Userback script into the page head
UserbackPlugin class should accept anaccessTokenparameter (required for Userback) via the constructor or method
UserbackPlugin class should accept an optionaluserDataUsingcallback for user identification
ImplementFilament\Contracts\Plugininterface in the main plugin class
Files:
src/UserbackPlugin.php
🧠 Learnings (5)
📚 Learning: 2026-01-03T12:15:17.364Z
Learnt from: CR
Repo: coddin-web/filament-userback-plugin PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T12:15:17.364Z
Learning: Applies to **/UserbackPlugin.php : UserbackPlugin class should accept an `accessToken` parameter (required for Userback) via the constructor or method
Applied to files:
config/filament-userback.phpresources/views/userback.blade.phptests/ConfigurationTest.phptests/UserbackPluginTest.phptests/ViewTest.phpsrc/UserbackPlugin.php
📚 Learning: 2026-01-03T12:15:17.364Z
Learnt from: CR
Repo: coddin-web/filament-userback-plugin PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T12:15:17.364Z
Learning: Applies to **/UserbackPlugin.php : Implement `Filament\Contracts\Plugin` interface in the main plugin class
Applied to files:
tests/ConfigurationTest.phptests/UserbackPluginTest.phptests/TestCase.phptests/ViewTest.phpsrc/UserbackPlugin.phptests/Pest.php
📚 Learning: 2026-01-03T12:15:17.364Z
Learnt from: CR
Repo: coddin-web/filament-userback-plugin PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T12:15:17.364Z
Learning: Applies to **/UserbackPlugin.php : UserbackPlugin class should accept an optional `userDataUsing` callback for user identification
Applied to files:
tests/UserbackPluginTest.php
📚 Learning: 2026-01-03T12:15:17.364Z
Learnt from: CR
Repo: coddin-web/filament-userback-plugin PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T12:15:17.364Z
Learning: Applies to **/UserbackPlugin.php : UserbackPlugin class should register a render hook to inject the Userback script into the page head
Applied to files:
tests/UserbackPluginTest.php
📚 Learning: 2026-01-03T12:15:17.364Z
Learnt from: CR
Repo: coddin-web/filament-userback-plugin PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T12:15:17.364Z
Learning: Applies to **/*.php : Use namespace `CoddinWeb\FilamentUserback` for all project classes
Applied to files:
tests/TestCase.phptests/Pest.php
🧬 Code graph analysis (4)
tests/ConfigurationTest.php (1)
src/UserbackPlugin.php (4)
UserbackPlugin(15-148)make(25-28)accessToken(35-40)onlyAuthenticated(49-54)
tests/UserbackPluginTest.php (1)
src/UserbackPlugin.php (5)
UserbackPlugin(15-148)accessToken(35-40)userDataUsing(42-47)onlyAuthenticated(49-54)guard(56-61)
tests/TestCase.php (1)
src/FilamentUserbackServiceProvider.php (1)
FilamentUserbackServiceProvider(10-29)
tests/Pest.php (1)
tests/TestCase.php (1)
TestCase(13-33)
🪛 PHPMD (2.15.0)
tests/TestCase.php
15-15: Avoid unused parameters such as '$app'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (18)
.gitignore (2)
3-3: LGTM!The
.phpunit.cacheentry correctly corresponds to the PHPUnit cache directory configured inphpunit.xml.
4-4: Verify whether ignoringcomposer.lockis appropriate for this package.For library/package development,
composer.lockis typically committed to ensure reproducible builds and consistent dev environments across team members and CI/CD pipelines. Ignoring it may lead to inconsistent test results if different developers or CI runs resolve dependencies differently.Consider whether this package should commit
composer.lockfor development consistency. If the decision is intentional (e.g., to test against a range of dependency versions), document this in the README or contributing guidelines.CLAUDE.md (1)
35-42: LGTM! Clear documentation of the new authentication features.The documentation clearly explains the precedence order for both
only_authenticatedandguardsettings, making it easy for developers to understand the fallback behavior.tests/Pest.php (1)
1-7: LGTM! Proper Pest configuration.The bootstrap file correctly:
- Declares strict types per coding guidelines
- Extends the project's TestCase for all tests in the directory
- Uses the proper namespace
phpunit.xml (1)
1-18: LGTM! Standard PHPUnit configuration.The configuration is properly set up with:
- Correct bootstrap path
- Cache directory matching
.gitignore- Test suite pointing to the tests directory
- Source coverage for the src directory
resources/views/userback.blade.php (1)
1-15: LGTM! Authentication gating logic is correct.The conditional correctly implements the authentication gating:
- Renders the widget only when
$accessTokenis a valid string- Respects the
$onlyAuthenticatedsetting (false = show to all, true = authenticated only)- Uses the configured guard for authentication checks
- Properly escapes variables with
@js()directivecomposer.json (1)
18-47: LGTM! Testing infrastructure properly configured.The dev dependencies (Pest, Orchestra Testbench, Pest Laravel plugin) are correctly configured with appropriate version constraints. The autoload-dev mapping, test script, and plugin allowlist follow standard practices for a Laravel package test suite.
config/filament-userback.php (1)
20-47: LGTM! Configuration options properly documented.The new
only_authenticatedandguardconfiguration entries are well-documented with clear explanations of their purpose and override behavior. The default value oftrueforonly_authenticatedis a sensible security-first default.tests/ViewTest.php (1)
1-98: LGTM! Comprehensive view rendering test coverage.The test suite thoroughly validates:
- Widget rendering with valid access tokens
- Suppression when access token is missing
- Authentication gating behavior for both authenticated and unauthenticated users
- User data injection
- Custom guard usage
The Auth facade mocking is correctly implemented for each authentication scenario.
tests/UserbackPluginTest.php (1)
1-62: LGTM! Fluent interface correctly validated.The test suite validates the plugin's fluent interface, ensuring all setter methods return
$thisfor method chaining. The tests confirm:
- Plugin instantiation and ID
- Individual configuration setters (accessToken, userDataUsing, onlyAuthenticated, guard)
- Full method chaining capability
tests/TestCase.php (3)
1-14: LGTM!The file correctly follows PSR-12 standards and includes
declare(strict_types=1)as required by the coding guidelines. The namespace and imports are properly structured for Orchestra Testbench integration.
15-23: LGTM!The service provider registration is correct and includes all necessary dependencies (Livewire, Filament core, support, and the plugin itself).
Note: The static analysis warning about the unused
$appparameter is a false positive—this parameter is required by the Orchestra Testbench method signature and must be present even when not directly referenced in the method body.
25-32: LGTM!The test environment configuration correctly sets up an SQLite in-memory database, which is the standard approach for fast, isolated integration tests with Orchestra Testbench.
src/UserbackPlugin.php (5)
19-22: LGTM!The new properties are properly typed with nullable defaults, enabling the fallback logic implemented in the getter methods. The protected visibility and clear naming align with the existing codebase patterns.
49-61: LGTM!The fluent setter methods correctly return
staticand follow the coding guidelines for fluent interfaces. The default parameter value foronlyAuthenticated(bool $onlyAuthenticated = true)provides convenient usage for the common case.
84-85: LGTM!The boot method correctly passes the resolved authentication and guard configuration to the view using the getter methods, ensuring proper fallback behavior from explicit values → config → panel defaults.
112-121: LGTM!The fallback logic is well-implemented with proper type guards. Defaulting to
truewhen no configuration is provided follows the principle of secure-by-default (widget visible only to authenticated users).
123-135: LGTM!The guard resolution logic correctly implements a three-tier fallback with appropriate type guards. Using the panel's configured auth guard as the final fallback provides a sensible default that respects the panel's authentication configuration.
- Remove reflection-based unit tests - Add panel integration tests with actual boot behavior - Test view rendering through actual template execution - Merge ViewTest into ConfigurationTest - Add helper methods to TestCase for panel creation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/ConfigurationTest.php (1)
15-54: Well-structured access token tests.The test cases appropriately cover the three main scenarios: config-based token, plugin-method token, and missing token. The direct view rendering approach effectively isolates the view logic for testing.
Optional: Consider adding edge case tests
You could optionally test additional edge cases such as:
- Empty string token
- Token with special characters that require escaping
- Very long token strings
These would further strengthen the test coverage, but the current tests are sufficient for the main use cases.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.gitignoretests/ConfigurationTest.phptests/TestCase.phptests/UserbackPluginTest.php
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/UserbackPluginTest.php
- .gitignore
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: Usedeclare(strict_types=1)in all PHP files
Follow PSR-12 coding standards
Use fluent interfaces (return$this) for setter methods
Use namespaceCoddinWeb\FilamentUserbackfor all project classes
Files:
tests/TestCase.phptests/ConfigurationTest.php
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: coddin-web/filament-userback-plugin PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T12:15:17.364Z
Learning: Applies to **/UserbackPlugin.php : Implement `Filament\Contracts\Plugin` interface in the main plugin class
📚 Learning: 2026-01-03T12:15:17.364Z
Learnt from: CR
Repo: coddin-web/filament-userback-plugin PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T12:15:17.364Z
Learning: Applies to **/UserbackPlugin.php : Implement `Filament\Contracts\Plugin` interface in the main plugin class
Applied to files:
tests/TestCase.phptests/ConfigurationTest.php
📚 Learning: 2026-01-03T12:15:17.364Z
Learnt from: CR
Repo: coddin-web/filament-userback-plugin PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T12:15:17.364Z
Learning: Applies to **/*.php : Use namespace `CoddinWeb\FilamentUserback` for all project classes
Applied to files:
tests/TestCase.php
📚 Learning: 2026-01-03T12:15:17.364Z
Learnt from: CR
Repo: coddin-web/filament-userback-plugin PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T12:15:17.364Z
Learning: Applies to **/UserbackPlugin.php : UserbackPlugin class should accept an `accessToken` parameter (required for Userback) via the constructor or method
Applied to files:
tests/ConfigurationTest.php
🧬 Code graph analysis (1)
tests/ConfigurationTest.php (2)
src/UserbackPlugin.php (4)
UserbackPlugin(15-148)accessToken(35-40)onlyAuthenticated(49-54)guard(56-61)tests/TestCase.php (1)
renderUserbackView(60-72)
🪛 PHPMD (2.15.0)
tests/TestCase.php
19-19: Avoid unused parameters such as '$app'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (8)
tests/ConfigurationTest.php (4)
1-13: LGTM! Proper test setup and isolation.The file correctly includes
declare(strict_types=1)as per coding guidelines, and thebeforeEachhook ensures test isolation by resetting configuration values before each test.
56-98: Correct Auth facade mocking approach.The tests appropriately mock the
Authfacade to verify widget rendering behavior based on authentication state and theonlyAuthenticatedflag. The mocking setup correctly chainsguard()andcheck()calls to simulate different authentication scenarios.
100-142: Guard configuration tests are comprehensive.The test suite effectively validates that the correct guard is used for authentication checks, covering default guard behavior, explicitly specified guards, and guard-specific authentication states.
144-172: User data tests validate rendering correctly.The tests appropriately verify that user data is rendered when provided (including nested structures) and omitted when null. The assertions check for both the presence of the
Userback.user_dataproperty and the actual values in the rendered output.tests/TestCase.php (4)
1-27: LGTM! Proper test infrastructure setup.The file correctly follows coding guidelines with
declare(strict_types=1)and the proper namespace. The service provider registration includes all necessary providers for Filament and the plugin.Note on static analysis hint: The PHPMD warning about unused parameter
$appon line 19 is a false positive. Orchestra Testbench requires this exact method signature forgetPackageProviders(), even though the parameter may not be used in the method body. This warning should be ignored.
29-38: Appropriate test environment configuration.The environment setup correctly configures an in-memory SQLite database for fast test execution and sets the User model for authentication testing.
40-58: Well-designed test helper methods.The
createPanelandbootPluginWithPanelhelpers provide clean, reusable infrastructure for integration testing. The optionalauthGuardparameter enables testing different guard configurations, aligning with the PR's guard functionality.
60-72: Clean view rendering helper for tests.The
renderUserbackViewmethod provides a straightforward way to test the view logic in isolation by directly passing the required parameters. The parameter types and return type are properly declared.
Runs Pest tests on PHP 8.2, 8.3, and 8.4 for push and PRs to main. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add isValidGuard() method to check guard exists in auth.guards config - Invalid guards are converted to null (uses default guard) - Prevents exceptions when invalid guard name is provided - Add tests for guard validation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/tests.yml (1)
31-32: Consider caching Composer dependencies.Adding dependency caching would speed up CI runs. The standard approach is to cache
~/.composer/cachekeyed by thecomposer.lockhash.🔎 Proposed addition for dependency caching
Add this step before "Install dependencies":
- name: Get Composer cache directory id: composer-cache run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT - name: Cache Composer dependencies uses: actions/cache@v4 with: path: ${{ steps.composer-cache.outputs.dir }} key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }} restore-keys: | ${{ runner.os }}-composer-
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/tests.yml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: coddin-web/filament-userback-plugin PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T12:15:17.364Z
Learning: Applies to **/UserbackPlugin.php : UserbackPlugin class should accept an `accessToken` parameter (required for Userback) via the constructor or method
Learnt from: CR
Repo: coddin-web/filament-userback-plugin PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T12:15:17.364Z
Learning: Applies to **/UserbackPlugin.php : UserbackPlugin class should register a render hook to inject the Userback script into the page head
🔇 Additional comments (4)
.github/workflows/tests.yml (4)
1-8: LGTM!The workflow name and trigger configuration are appropriate for a CI pipeline.
9-18: LGTM!The PHP version matrix (8.2, 8.3, 8.4) appropriately covers current and recent PHP releases. The
fail-fast: truestrategy is reasonable for quick feedback.
20-29: LGTM!The checkout and PHP setup steps are correctly configured. The extension list is appropriate for Laravel/Filament testing with SQLite.
34-35: LGTM!The test execution step correctly runs Pest via the vendor binary.
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
Summary
only_authenticatedconfig option to control widget visibility for authenticated users->onlyAuthenticated()method for per-panel overrideguardconfig option with automatic fallback to panel's auth guard->guard()method for per-panel overrideUsage
Only show to authenticated users (default behavior):
Show to all visitors:
Use a specific auth guard:
By default, the plugin uses the panel's configured auth guard.
Test plan
only_authenticatedworks as expected🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.