Add php artisan login command for quick dev login#20
Conversation
WalkthroughThis PR introduces a login workflow automation feature for testing, adding a Laravel Artisan command that executes a Playwright-based browser script to automatically log in a test user. Configuration files are updated to reflect the test domain. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. 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: 3
🧹 Nitpick comments (3)
app/Console/Commands/LoginCommand.php (3)
16-45: Add PHPDoc block for better documentation.The method lacks a PHPDoc block. Per coding guidelines, PHPDoc blocks are preferred over comments.
Add this PHPDoc block above the method:
+ /** + * Execute the console command to ensure a test user exists and log in via browser. + */ public function handle(): int
47-56: Consider renaming for clarity.The method name
shouldLogin()is misleading - it actually checks if a user exists, not whether login should proceed. Consider renaming touserExists()orhasUser()for clarity.Apply this diff:
- protected function shouldLogin(): bool + protected function userExists(): bool { $email = $this->option('email'); if (! User::where('email', $email)->exists()) { return false; } return true; }And update the call site:
- if (! $this->shouldLogin()) { + if (! $this->userExists()) { $this->fixLoginRequirements($email); }Also add a PHPDoc block:
+ /** + * Check if the user with the given email exists. + */ protected function userExists(): bool
58-69: Add PHPDoc block.Per coding guidelines, prefer PHPDoc blocks over comments.
Add this PHPDoc block:
+ /** + * Ensure login requirements are met by creating the user if needed. + */ protected function fixLoginRequirements(string $email): void
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.claude/commands/login.md(1 hunks).env.example(1 hunks)app/Console/Commands/LoginCommand.php(1 hunks)config/fortify.php(1 hunks)scripts/login.cjs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: Always use curly braces for control structures, even if it has one line
Use PHP 8 constructor property promotion in__construct()methods
Do not allow empty__construct()methods with zero parameters
Always use explicit return type declarations for methods and functions
Use appropriate PHP type hints for method parameters
Prefer PHPDoc blocks over comments. Never use comments within the code itself unless there is something very complex going on
Add useful array shape type definitions for arrays when appropriate in PHPDoc blocks
Typically, keys in an Enum should be TitleCase. For example:FavoritePerson,BestLake,Monthly
Always use proper Eloquent relationship methods with return type hints. Prefer relationship methods over raw queries or manual joins
Use Eloquent models and relationships before suggesting raw database queries
AvoidDB::queries; preferModel::query(). Generate code that leverages Laravel's ORM capabilities rather than bypassing them
Generate code that prevents N+1 query problems by using eager loading
Use Laravel's query builder for very complex database operations
When creating new models, create useful factories and seeders for them too
For APIs, default to using Eloquent API Resources and API versioning unless existing API routes do not, then you should follow existing application convention
Always create Form Request classes for validation rather than inline validation in controllers. Include both validation rules and custom error messages
Check sibling Form Requests to see if the application uses array or string based validation rules
Use queued jobs for time-consuming operations with theShouldQueueinterface
Use Laravel's built-in authentication and authorization features (gates, policies, Sanctum, etc.)
Use environment variables only in configuration files - never use theenv()function directly outside of config files. Always useconfig('app.name'), notenv('APP_NAME')
UseInertia::render()for server-s...
Files:
config/fortify.phpapp/Console/Commands/LoginCommand.php
config/fortify.php
📄 CodeRabbit inference engine (CLAUDE.md)
Check
config/fortify.phpto see what's enabled. Enable features by adding them to the'features' => []arrayEnable Fortify features by adding them to the
'features' => []array inconfig/fortify.phpCheck
config/fortify.phpto see what's enabled and enable features by adding them to the'features' => []array
Files:
config/fortify.php
app/Console/Commands/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Commands auto-register - files in
app/Console/Commands/are automatically available and do not require manual registration
Files:
app/Console/Commands/LoginCommand.php
**/app/**/*.php
📄 CodeRabbit inference engine (GEMINI.md)
Use environment variables only in configuration files - never use the
env()function directly outside of config files
Files:
app/Console/Commands/LoginCommand.php
**/app/Console/Commands/**/*.php
📄 CodeRabbit inference engine (GEMINI.md)
Commands auto-register from
app/Console/Commands/and do not require manual registration
Files:
app/Console/Commands/LoginCommand.php
@(app|src)/**/*.php
📄 CodeRabbit inference engine (AGENTS.md)
Use environment variables only in configuration files - never use the
env()function directly outside of config files. Always useconfig('app.name'), notenv('APP_NAME')
Files:
app/Console/Commands/LoginCommand.php
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T02:45:25.865Z
Learning: Applies to tests/Browser/**/*.php : Use Laravel features like `Event::fake()`, `assertAuthenticated()`, and model factories within Pest v4 browser tests, as well as `RefreshDatabase` to ensure a clean state
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T02:43:59.473Z
Learning: Applies to tests/Browser/**/*.php : You can use Laravel features like `Event::fake()`, `assertAuthenticated()`, and model factories within Pest v4 browser tests, as well as `RefreshDatabase` (when needed)
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T02:45:25.865Z
Learning: When creating tests, make use of `php artisan make:test [options] {name}` to create a feature test, and pass `--unit` to create a unit test. Most tests should be feature tests
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T02:43:59.473Z
Learning: Use `php artisan make:test [options] {name}` to create a feature test, and pass `--unit` to create a unit test. Most tests should be feature tests
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T02:43:59.473Z
Learning: Pass `--no-interaction` to all Artisan commands to ensure they work without user input
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T02:45:25.865Z
Learning: Use `php artisan make:` commands to create new files (migrations, controllers, models, etc.). Pass `--no-interaction` to all Artisan commands and use the correct options
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-01T02:44:35.071Z
Learning: Applies to **/*.php : Use Laravel's built-in authentication and authorization features (gates, policies, Sanctum, etc.)
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T02:45:25.865Z
Learning: Applies to **/*.php : Use Laravel's built-in authentication and authorization features (gates, policies, Sanctum, etc.)
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T02:43:59.473Z
Learning: Applies to **/*.php : Use Laravel's built-in authentication and authorization features (gates, policies, Sanctum, etc.)
📚 Learning: 2025-12-01T02:44:35.071Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-01T02:44:35.071Z
Learning: Applies to config/fortify.php : Enable Fortify features by adding them to the `'features' => []` array in `config/fortify.php`
Applied to files:
config/fortify.php
📚 Learning: 2025-12-01T02:45:25.865Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T02:45:25.865Z
Learning: Applies to config/fortify.php : Check `config/fortify.php` to see what's enabled and enable features by adding them to the `'features' => []` array
Applied to files:
config/fortify.php
📚 Learning: 2025-12-01T02:43:59.473Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T02:43:59.473Z
Learning: Applies to config/fortify.php : Check `config/fortify.php` to see what's enabled. Enable features by adding them to the `'features' => []` array
Applied to files:
config/fortify.php
📚 Learning: 2025-12-01T02:43:59.473Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T02:43:59.473Z
Learning: Applies to app/Actions/Fortify/**/*.php : Actions in `app/Actions/Fortify/` handle business logic (user creation, password reset, etc.). They're fully customizable, so you can modify them to change feature behavior
Applied to files:
config/fortify.php
🧬 Code graph analysis (1)
app/Console/Commands/LoginCommand.php (1)
scripts/login.cjs (2)
url(3-3)
🪛 GitHub Actions: linter
scripts/login.cjs
[error] 1-1: ESLint: A require() style import is forbidden (@typescript-eslint/no-require-imports) in scripts/login.cjs while running 'eslint . --fix'.
🔇 Additional comments (4)
.env.example (1)
5-5: LGTM!The APP_URL change to
http://chat.testaligns well with the new login automation workflow and is appropriate for the development environment.config/fortify.php (1)
150-154: LGTM - appropriate for development workflow.Disabling 2FA streamlines the automated login workflow for development and testing. This is a reasonable trade-off for dev environments where the new
php artisan logincommand is intended to be used..claude/commands/login.md (1)
1-22: LGTM!The documentation clearly describes the login workflow and provides helpful step-by-step instructions for using the feature.
app/Console/Commands/LoginCommand.php (1)
58-69: Consider email verification for test user, but use safe approach.Email verification is enabled in Fortify (config/fortify.php line 149), but the suggested approach of adding
email_verified_atto the user creation would require modifying the$fillablearray, which poses a mass assignment security risk. SinceMustVerifyEmailis not enforced on the User model (the interface is commented out), the test user will function without verification.For a dev/CLI command, either approach is acceptable:
- Leave
email_verified_atas null (current state)- Use
->forceFill(['email_verified_at' => now()])after creation to bypass mass assignment checksNo changes required unless you want verified test users explicitly.
| $result = Process::path(base_path()) | ||
| ->run(['npx', 'playwright', 'test', '--', 'node', 'scripts/login.cjs', $url, $email, 'password']); | ||
|
|
||
| if (! $result->successful()) { | ||
| // Try direct node execution | ||
| $result = Process::path(base_path()) | ||
| ->run(['node', 'scripts/login.cjs', $url, $email, 'password']); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Simplify Playwright execution - the test runner isn't needed.
The command npx playwright test -- node scripts/login.cjs is attempting to use Playwright's test runner to execute a Node script, which is unnecessary and adds complexity. The script should be run directly with Node.
Apply this diff to simplify:
- $result = Process::path(base_path())
- ->run(['npx', 'playwright', 'test', '--', 'node', 'scripts/login.cjs', $url, $email, 'password']);
-
- if (! $result->successful()) {
- // Try direct node execution
- $result = Process::path(base_path())
- ->run(['node', 'scripts/login.cjs', $url, $email, 'password']);
- }
+ $result = Process::path(base_path())
+ ->run(['node', 'scripts/login.cjs', $url, $email, 'password']);📝 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.
| $result = Process::path(base_path()) | |
| ->run(['npx', 'playwright', 'test', '--', 'node', 'scripts/login.cjs', $url, $email, 'password']); | |
| if (! $result->successful()) { | |
| // Try direct node execution | |
| $result = Process::path(base_path()) | |
| ->run(['node', 'scripts/login.cjs', $url, $email, 'password']); | |
| } | |
| $result = Process::path(base_path()) | |
| ->run(['node', 'scripts/login.cjs', $url, $email, 'password']); |
🤖 Prompt for AI Agents
In app/Console/Commands/LoginCommand.php around lines 28 to 35, the code runs
Playwright's test runner to execute a plain Node script which is unnecessary;
replace the primary Process::run call so it directly executes the script with
Node (e.g., run ['node', 'scripts/login.cjs', $url, $email, 'password']) and
remove the fallback that repeats the same node execution, ensuring the command
only attempts direct node execution once and retains the existing base_path()
context and error handling.
| @@ -0,0 +1,20 @@ | |||
| const { chromium } = require('playwright'); | |||
There was a problem hiding this comment.
Fix ESLint violation: use ES6 import syntax.
The pipeline flagged this line: CommonJS require() is forbidden. Use ES6 import syntax instead.
Apply this diff to fix the import:
-const { chromium } = require('playwright');
+import { chromium } from 'playwright';Also update package.json to include "type": "module" if not already present, or rename the file to login.mjs.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: linter
[error] 1-1: ESLint: A require() style import is forbidden (@typescript-eslint/no-require-imports) in scripts/login.cjs while running 'eslint . --fix'.
🤖 Prompt for AI Agents
In scripts/login.cjs around line 1, the file uses CommonJS require() which
triggers an ESLint rule forbidding require; replace the CommonJS import with an
ES6 import (import { chromium } from 'playwright') and ensure the runtime
recognizes ESM by either adding "type": "module" to package.json or renaming the
file to login.mjs; update any other require/exports in the file to ESM syntax
and run lint/tests to confirm no other CommonJS usages remain.
| (async () => { | ||
| const browser = await chromium.launch({ headless: false }); | ||
| const page = await browser.newPage(); | ||
|
|
||
| await page.goto(url + '/login'); | ||
| await page.fill('input[type="email"], input[name="email"]', email); | ||
| await page.fill('input[type="password"], input[name="password"]', password); | ||
| await page.click('button[type="submit"]'); | ||
|
|
||
| await page.waitForURL('**/dashboard**'); | ||
|
|
||
| const finalUrl = page.url(); | ||
| console.log('Logged in as ' + email + ' - now on ' + finalUrl); | ||
| })(); |
There was a problem hiding this comment.
Critical: Memory leak - browser never closed.
The browser instance is launched but never closed, causing a resource leak. Additionally, there's no error handling if navigation or form interactions fail.
Apply this diff to fix both issues:
(async () => {
- const browser = await chromium.launch({ headless: false });
- const page = await browser.newPage();
-
- await page.goto(url + '/login');
- await page.fill('input[type="email"], input[name="email"]', email);
- await page.fill('input[type="password"], input[name="password"]', password);
- await page.click('button[type="submit"]');
-
- await page.waitForURL('**/dashboard**');
-
- const finalUrl = page.url();
- console.log('Logged in as ' + email + ' - now on ' + finalUrl);
+ const browser = await chromium.launch({ headless: false });
+ try {
+ const page = await browser.newPage();
+
+ await page.goto(url + '/login', { timeout: 30000 });
+ await page.fill('input[type="email"], input[name="email"]', email);
+ await page.fill('input[type="password"], input[name="password"]', password);
+ await page.click('button[type="submit"]');
+
+ await page.waitForURL('**/dashboard**', { timeout: 30000 });
+
+ const finalUrl = page.url();
+ console.log('Logged in as ' + email + ' - now on ' + finalUrl);
+ } finally {
+ await browser.close();
+ }
})();📝 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.
| (async () => { | |
| const browser = await chromium.launch({ headless: false }); | |
| const page = await browser.newPage(); | |
| await page.goto(url + '/login'); | |
| await page.fill('input[type="email"], input[name="email"]', email); | |
| await page.fill('input[type="password"], input[name="password"]', password); | |
| await page.click('button[type="submit"]'); | |
| await page.waitForURL('**/dashboard**'); | |
| const finalUrl = page.url(); | |
| console.log('Logged in as ' + email + ' - now on ' + finalUrl); | |
| })(); | |
| (async () => { | |
| const browser = await chromium.launch({ headless: false }); | |
| try { | |
| const page = await browser.newPage(); | |
| await page.goto(url + '/login', { timeout: 30000 }); | |
| await page.fill('input[type="email"], input[name="email"]', email); | |
| await page.fill('input[type="password"], input[name="password"]', password); | |
| await page.click('button[type="submit"]'); | |
| await page.waitForURL('**/dashboard**', { timeout: 30000 }); | |
| const finalUrl = page.url(); | |
| console.log('Logged in as ' + email + ' - now on ' + finalUrl); | |
| } finally { | |
| await browser.close(); | |
| } | |
| })(); |
🤖 Prompt for AI Agents
In scripts/login.cjs around lines 7 to 20, the launched Playwright browser is
never closed and there is no error handling; wrap the async flow in a
try/catch/finally: declare the browser variable outside, assign it when
launching, perform navigation and interactions inside try, log and rethrow or
exit on error in catch, and always call browser.close() in finally (guarding for
undefined) to prevent the memory leak and ensure errors are surfaced.
Implements php artisan login command that automatically creates a dev user if needed and logs in via Playwright browser automation - Artisan command that creates dev user if missing and opens browser for login - Playwright script handles automated login flow with credentials - Claude slash command for /login available for quick dev access - Useful for rapid development iteration without manual login
Updates development configuration to streamline login flow and align with local development domain - APP_URL updated from localhost to chat.test for proper domain handling - Two-factor authentication disabled for development convenience
- Re-enable 2FA in fortify config (disabling breaks Wayfinder build) - Fix flaky test by specifying different providers for credentials
jordanpartridge
force-pushed
the
feature/artisan-login-command
branch
from
ed1420b to
e71011a
Compare
- Exclude scripts/ from ESLint (CommonJS files) - Remove unused InputError import from Providers.vue
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
scripts/login.cjs (1)
7-20: Critical: Browser memory leak - browser instance never closed.The browser is launched but never closed, causing a resource leak. Additionally, there's no error handling for navigation failures or timeouts.
Apply this diff to fix the issue:
(async () => { - const browser = await chromium.launch({ headless: false }); - const page = await browser.newPage(); - - await page.goto(url + '/login'); - await page.fill('input[type="email"], input[name="email"]', email); - await page.fill('input[type="password"], input[name="password"]', password); - await page.click('button[type="submit"]'); - - await page.waitForURL('**/dashboard**'); - - const finalUrl = page.url(); - console.log('Logged in as ' + email + ' - now on ' + finalUrl); + const browser = await chromium.launch({ headless: false }); + try { + const page = await browser.newPage(); + + await page.goto(url + '/login', { timeout: 30000 }); + await page.fill('input[type="email"], input[name="email"]', email); + await page.fill('input[type="password"], input[name="password"]', password); + await page.click('button[type="submit"]'); + + await page.waitForURL('**/dashboard**', { timeout: 30000 }); + + const finalUrl = page.url(); + console.log('Logged in as ' + email + ' - now on ' + finalUrl); + } catch (error) { + console.error('Login failed:', error.message); + throw error; + } finally { + await browser.close(); + } })();app/Console/Commands/LoginCommand.php (1)
28-35: Simplify: Remove unnecessary Playwright test runner.The command attempts to use Playwright's test runner to execute a Node script, which is unnecessary. The script should be run directly with Node.
Apply this diff to simplify:
- $result = Process::path(base_path()) - ->run(['npx', 'playwright', 'test', '--', 'node', 'scripts/login.cjs', $url, $email, 'password']); - - if (! $result->successful()) { - // Try direct node execution - $result = Process::path(base_path()) - ->run(['node', 'scripts/login.cjs', $url, $email, 'password']); - } + $result = Process::path(base_path()) + ->run(['node', 'scripts/login.cjs', $url, $email, 'password']);
🧹 Nitpick comments (1)
app/Console/Commands/LoginCommand.php (1)
47-56: Simplify the logic.The method can be simplified to directly return the existence check result.
Apply this diff:
protected function shouldLogin(): bool { - $email = $this->option('email'); - - if (! User::where('email', $email)->exists()) { - return false; - } - - return true; + return User::where('email', $this->option('email'))->exists(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.env.example(1 hunks)app/Console/Commands/LoginCommand.php(1 hunks)eslint.config.js(1 hunks)resources/js/pages/settings/Providers.vue(0 hunks)scripts/login.cjs(1 hunks)tests/Feature/Http/Controllers/Settings/ProviderCredentialControllerTest.php(1 hunks)
💤 Files with no reviewable changes (1)
- resources/js/pages/settings/Providers.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- .env.example
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{php,blade.php,vue,tsx,ts,jsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
When generating links to other pages, prefer named routes and the
route()function
Files:
eslint.config.jstests/Feature/Http/Controllers/Settings/ProviderCredentialControllerTest.phpapp/Console/Commands/LoginCommand.php
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use named imports for tree-shaking (e.g.,import { show } from '@/actions/...'); avoid default controller imports
Use Wayfinder HTTP method calls (.get(),.post(),.patch(),.put(),.delete()) for specific methods
For Wayfinder invokable controllers, import and invoke directly as functions
For non-controller routes in Wayfinder, import from@/routes/using named route names
Wayfinder detects route keys (e.g.,{post:slug}) and accepts matching object properties for parameter binding
Use WayfindermergeQueryto merge withwindow.location.search; set values tonullto remove them
Pass{ query: {...} }in options to Wayfinder to append query parameters
Wayfinder functions return{ url, method }shaped objects; use.url()to get just the URL string
Files:
eslint.config.js
**/*.php
📄 CodeRabbit inference engine (GEMINI.md)
**/*.php: Use PHP 8 constructor property promotion in__construct()methods instead of assigning parameters to properties
Always use explicit return type declarations for methods and functions
Use appropriate PHP type hints for method parameters
Always use curly braces for control structures, even if it has one line
Do not allow empty__construct()methods with zero parameters
Prefer PHPDoc blocks over comments; never use comments within the code itself unless there is something very complex
Add useful array shape type definitions for arrays in PHPDoc blocks when appropriate
Use Eloquent models and relationships before suggesting raw database queries; avoidDB::; preferModel::query()
Generate code that prevents N+1 query problems by using eager loading
Use Laravel's built-in authentication and authorization features (gates, policies, Sanctum, etc.)
When generating links to other pages, prefer named routes and theroute()function
**/*.php: Always use curly braces for control structures, even if it has one line
Use PHP 8 constructor property promotion in__construct()instead of traditional property assignment
Do not allow empty__construct()methods with zero parameters
Always use explicit return type declarations for methods and functions
Use appropriate PHP type hints for method parameters
Prefer PHPDoc blocks over comments. Never use comments within the code itself unless there is something very complex going on
Add useful array shape type definitions for arrays in PHPDoc blocks when appropriate
Enum keys should typically be TitleCase, for example:FavoritePerson,BestLake,Monthly
Always use proper Eloquent relationship methods with return type hints. Prefer relationship methods over raw queries or manual joins
Use Eloquent models and relationships before suggesting raw database queries. AvoidDB::; preferModel::query()
Generate code that prevents N+1 query problems by using eager loading
Use Laravel's query builder for very comple...
Files:
tests/Feature/Http/Controllers/Settings/ProviderCredentialControllerTest.phpapp/Console/Commands/LoginCommand.php
tests/**/*.php
📄 CodeRabbit inference engine (GEMINI.md)
tests/**/*.php: When creating models for tests, use the factories for the models and check if the factory has custom states
Use methods such as$this->faker->word()orfake()->randomDigit()for Faker; follow existing conventions whether to use$this->fakerorfake()
All tests must be written using Pest; usephp artisan make:test --pest {name}
Use datasets in Pest to simplify tests which have a lot of duplicated data, especially for validation rule testing
Tests should test all of the happy paths, failure paths, and weird paths
tests/**/*.php: When creating models for tests, use the factories for the models. Check if the factory has custom states before manually setting up the model
Use methods such as$this->faker->word()orfake()->randomDigit()for Faker. Follow existing conventions whether to use$this->fakerorfake()
When asserting status codes on a response in Pest, use the specific method likeassertForbiddenandassertNotFoundinstead of usingassertStatus(403)
Useuse function Pest\Laravel\mock;when using thePest\Laravel\mockfunction for mocking, or use$this->mock()if existing tests do
Use datasets in Pest to simplify tests which have duplicated data, especially when testing validation rules
All tests must be written using Pest. Usephp artisan make:test --pest {name}
Tests should test all of the happy paths, failure paths, and weird paths
tests/**/*.php: When creating models for tests, use factories for the models; check if the factory has custom states that can be used before manually setting up the model
Use faker methods such as$this->faker->word()orfake()->randomDigit()in tests; follow existing conventions for whether to use$this->fakerorfake()
All tests must be written using Pest; usephp artisan make:test --pest {name}
When asserting status codes on a response in Pest, use specific methods likeassertForbidden()andassertNotFound()instead ofassertStatus(403)
When mocking in Pest, ...
Files:
tests/Feature/Http/Controllers/Settings/ProviderCredentialControllerTest.php
tests/Feature/**/*.php
📄 CodeRabbit inference engine (GEMINI.md)
When asserting status codes on a response, use the specific method like
assertForbiddenandassertNotFoundinstead ofassertStatus(403)or similar
Files:
tests/Feature/Http/Controllers/Settings/ProviderCredentialControllerTest.php
**/{Controllers,routes}/**/*.php
📄 CodeRabbit inference engine (AGENTS.md)
Use
Inertia::render()for server-side routing instead of traditional Blade views
Files:
tests/Feature/Http/Controllers/Settings/ProviderCredentialControllerTest.php
tests/{Feature,Unit}/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Tests live in the
tests/Featureandtests/Unitdirectories
Files:
tests/Feature/Http/Controllers/Settings/ProviderCredentialControllerTest.php
**/app/**/*.php
📄 CodeRabbit inference engine (GEMINI.md)
Use environment variables only in configuration files - never use the
env()function directly outside of config files
Files:
app/Console/Commands/LoginCommand.php
**/app/Console/Commands/**/*.php
📄 CodeRabbit inference engine (GEMINI.md)
Commands auto-register from
app/Console/Commands/and do not require manual registration
Files:
app/Console/Commands/LoginCommand.php
@(app|src)/**/*.php
📄 CodeRabbit inference engine (AGENTS.md)
Use environment variables only in configuration files - never use the
env()function directly outside of config files. Always useconfig('app.name'), notenv('APP_NAME')
Files:
app/Console/Commands/LoginCommand.php
app/Console/Commands/**/*.php
📄 CodeRabbit inference engine (AGENTS.md)
Commands auto-register - files in
app/Console/Commands/are automatically available and do not require manual registration
Files:
app/Console/Commands/LoginCommand.php
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T02:45:25.865Z
Learning: Applies to tests/Browser/**/*.php : Use Laravel features like `Event::fake()`, `assertAuthenticated()`, and model factories within Pest v4 browser tests, as well as `RefreshDatabase` to ensure a clean state
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T02:49:56.681Z
Learning: Applies to tests/Browser/**/*.php : In Pest v4 browser tests, you can use Laravel features like `Event::fake()`, `assertAuthenticated()`, model factories, and `RefreshDatabase` to ensure clean state
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T02:49:56.681Z
Learning: Create feature tests using `php artisan make:test {name}`; pass `--unit` to create unit tests. Most tests should be feature tests
📚 Learning: 2025-12-01T02:45:25.865Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T02:45:25.865Z
Learning: Applies to resources/js/**/*.{ts,tsx,vue} : Avoid default controller imports in Wayfinder as it prevents tree-shaking
Applied to files:
eslint.config.js
📚 Learning: 2025-12-01T02:44:35.071Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-01T02:44:35.071Z
Learning: Applies to resources/js/**/*.{vue,ts} : Always use Wayfinder named imports for tree-shaking (e.g., `import { show } from '@/actions/...'`); avoid default controller imports
Applied to files:
eslint.config.js
📚 Learning: 2025-12-01T02:45:25.865Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T02:45:25.865Z
Learning: Applies to resources/js/**/*.{ts,tsx,vue} : Always prefer named imports for tree-shaking when importing Wayfinder-generated functions (e.g., `import { show } from '@/actions/...'`)
Applied to files:
eslint.config.js
📚 Learning: 2025-12-09T02:49:56.681Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T02:49:56.681Z
Learning: Applies to resources/js/**/*.{vue,jsx,tsx} : Forms can also be built using the `useForm` helper for more programmatic control; follow existing conventions
Applied to files:
eslint.config.js
📚 Learning: 2025-12-01T02:44:35.071Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-01T02:44:35.071Z
Learning: Applies to {resources/js/**/*.vue,resources/views/**/*.blade.php,resources/css/**/*.css} : Always use Tailwind CSS v4 and do not use deprecated utilities; use the replacement utilities from the v4 migration table
Applied to files:
eslint.config.js
📚 Learning: 2025-12-01T02:45:25.865Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T02:45:25.865Z
Learning: Applies to resources/js/**/*.{ts,tsx,vue,js} : Use camelCase for variable names
Applied to files:
eslint.config.js
📚 Learning: 2025-12-09T02:49:56.681Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T02:49:56.681Z
Learning: Applies to resources/js/Pages/**/*.{vue,jsx,tsx} : Inertia.js components should be placed in the `resources/js/Pages` directory unless specified differently in the JS bundler (vite.config.js)
Applied to files:
eslint.config.js
🧬 Code graph analysis (2)
tests/Feature/Http/Controllers/Settings/ProviderCredentialControllerTest.php (1)
database/factories/UserApiCredentialFactory.php (2)
openai(30-36)anthropic(41-47)
app/Console/Commands/LoginCommand.php (1)
scripts/login.cjs (2)
url(3-3)
🔇 Additional comments (3)
eslint.config.js (1)
10-10: LGTM!Excluding the
scripts/*directory from ESLint is appropriate for the new Playwright automation script added in this PR.tests/Feature/Http/Controllers/Settings/ProviderCredentialControllerTest.php (1)
552-553: LGTM!Good use of factory states to create provider-specific credentials. This follows the coding guideline to use factory states when available and makes the test setup more explicit. Based on coding guidelines, factories should be used with their custom states.
app/Console/Commands/LoginCommand.php (1)
47-47: Add explicit return type declarations.As per coding guidelines, all methods should have explicit return type declarations.
The method signatures already include return types (
: booland: void), so this is actually correct. Good adherence to coding guidelines!Also applies to: 58-58
⛔ Skipped due to learnings
Learnt from: CR Repo: jordanpartridge/chat PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-09T02:49:56.681Z Learning: Applies to **/*.php : Always use explicit return type declarations for methods and functions (PHP)Learnt from: CR Repo: jordanpartridge/chat PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-01T02:45:25.865Z Learning: Applies to **/*.php : Always use explicit return type declarations for methods and functionsLearnt from: CR Repo: jordanpartridge/chat PR: 0 File: GEMINI.md:0-0 Timestamp: 2025-12-01T02:44:35.071Z Learning: Applies to **/*.php : Always use explicit return type declarations for methods and functions
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/Console/Commands/LoginCommand.php (1)
28-35: Simplify Playwright execution.The past review comment correctly identified that using
npx playwright test --to run a plain Node script is unnecessary. Execute the script directly with Node.Apply this diff:
- $result = Process::path(base_path()) - ->run(['npx', 'playwright', 'test', '--', 'node', 'scripts/login.cjs', $url, $email, 'password']); - - if (! $result->successful()) { - // Try direct node execution - $result = Process::path(base_path()) - ->run(['node', 'scripts/login.cjs', $url, $email, 'password']); - } + $result = Process::path(base_path()) + ->run(['node', 'scripts/login.cjs', $url, $email, 'password']);
🧹 Nitpick comments (2)
app/Console/Commands/LoginCommand.php (2)
47-56: Consider renaming for clarity.The method name
shouldLogin()doesn't clearly convey that it's checking user existence. Since it returnstruewhen the user exists (meaning we don't need to fix requirements), a name likeuserExists()orhasRequiredUser()would be clearer.Apply this diff:
- protected function shouldLogin(): bool + protected function userExists(): bool { $email = $this->option('email'); if (! User::where('email', $email)->exists()) { return false; } return true; }And update the call site:
- if (! $this->shouldLogin()) { + if (! $this->userExists()) { $this->fixLoginRequirements($email); }
58-69: Remove redundant user existence check.Line 60 checks if the user exists, but this method is only called when
shouldLogin()returnsfalse, meaning the user definitely doesn't exist. The check is redundant.Apply this diff:
protected function fixLoginRequirements(string $email): void { - if (! User::where('email', $email)->exists()) { - $this->info("Creating user: {$email}"); + $this->info("Creating user: {$email}"); - User::create([ - 'name' => 'Dev User', - 'email' => $email, - 'password' => Hash::make('password'), - ]); - } + User::create([ + 'name' => 'Dev User', + 'email' => $email, + 'password' => Hash::make('password'), + ]); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.env.example(1 hunks)app/Console/Commands/LoginCommand.php(1 hunks)eslint.config.js(1 hunks)resources/js/pages/settings/Providers.vue(0 hunks)scripts/login.cjs(1 hunks)tests/Feature/Http/Controllers/Settings/ProviderCredentialControllerTest.php(1 hunks)
💤 Files with no reviewable changes (1)
- resources/js/pages/settings/Providers.vue
🚧 Files skipped from review as they are similar to previous changes (2)
- .env.example
- scripts/login.cjs
🧰 Additional context used
📓 Path-based instructions (11)
**/*.php
📄 CodeRabbit inference engine (GEMINI.md)
**/*.php: Use PHP 8 constructor property promotion in__construct()methods instead of assigning parameters to properties
Always use explicit return type declarations for methods and functions
Use appropriate PHP type hints for method parameters
Always use curly braces for control structures, even if it has one line
Do not allow empty__construct()methods with zero parameters
Prefer PHPDoc blocks over comments; never use comments within the code itself unless there is something very complex
Add useful array shape type definitions for arrays in PHPDoc blocks when appropriate
Use Eloquent models and relationships before suggesting raw database queries; avoidDB::; preferModel::query()
Generate code that prevents N+1 query problems by using eager loading
Use Laravel's built-in authentication and authorization features (gates, policies, Sanctum, etc.)
When generating links to other pages, prefer named routes and theroute()function
**/*.php: Always use curly braces for control structures, even if it has one line
Use PHP 8 constructor property promotion in__construct()instead of traditional property assignment
Do not allow empty__construct()methods with zero parameters
Always use explicit return type declarations for methods and functions
Use appropriate PHP type hints for method parameters
Prefer PHPDoc blocks over comments. Never use comments within the code itself unless there is something very complex going on
Add useful array shape type definitions for arrays in PHPDoc blocks when appropriate
Enum keys should typically be TitleCase, for example:FavoritePerson,BestLake,Monthly
Always use proper Eloquent relationship methods with return type hints. Prefer relationship methods over raw queries or manual joins
Use Eloquent models and relationships before suggesting raw database queries. AvoidDB::; preferModel::query()
Generate code that prevents N+1 query problems by using eager loading
Use Laravel's query builder for very comple...
Files:
tests/Feature/Http/Controllers/Settings/ProviderCredentialControllerTest.phpapp/Console/Commands/LoginCommand.php
tests/**/*.php
📄 CodeRabbit inference engine (GEMINI.md)
tests/**/*.php: When creating models for tests, use the factories for the models and check if the factory has custom states
Use methods such as$this->faker->word()orfake()->randomDigit()for Faker; follow existing conventions whether to use$this->fakerorfake()
All tests must be written using Pest; usephp artisan make:test --pest {name}
Use datasets in Pest to simplify tests which have a lot of duplicated data, especially for validation rule testing
Tests should test all of the happy paths, failure paths, and weird paths
tests/**/*.php: When creating models for tests, use the factories for the models. Check if the factory has custom states before manually setting up the model
Use methods such as$this->faker->word()orfake()->randomDigit()for Faker. Follow existing conventions whether to use$this->fakerorfake()
When asserting status codes on a response in Pest, use the specific method likeassertForbiddenandassertNotFoundinstead of usingassertStatus(403)
Useuse function Pest\Laravel\mock;when using thePest\Laravel\mockfunction for mocking, or use$this->mock()if existing tests do
Use datasets in Pest to simplify tests which have duplicated data, especially when testing validation rules
All tests must be written using Pest. Usephp artisan make:test --pest {name}
Tests should test all of the happy paths, failure paths, and weird paths
tests/**/*.php: When creating models for tests, use factories for the models; check if the factory has custom states that can be used before manually setting up the model
Use faker methods such as$this->faker->word()orfake()->randomDigit()in tests; follow existing conventions for whether to use$this->fakerorfake()
All tests must be written using Pest; usephp artisan make:test --pest {name}
When asserting status codes on a response in Pest, use specific methods likeassertForbidden()andassertNotFound()instead ofassertStatus(403)
When mocking in Pest, ...
Files:
tests/Feature/Http/Controllers/Settings/ProviderCredentialControllerTest.php
tests/Feature/**/*.php
📄 CodeRabbit inference engine (GEMINI.md)
When asserting status codes on a response, use the specific method like
assertForbiddenandassertNotFoundinstead ofassertStatus(403)or similar
Files:
tests/Feature/Http/Controllers/Settings/ProviderCredentialControllerTest.php
**/{Controllers,routes}/**/*.php
📄 CodeRabbit inference engine (AGENTS.md)
Use
Inertia::render()for server-side routing instead of traditional Blade views
Files:
tests/Feature/Http/Controllers/Settings/ProviderCredentialControllerTest.php
**/*.{php,blade.php,vue,tsx,ts,jsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
When generating links to other pages, prefer named routes and the
route()function
Files:
tests/Feature/Http/Controllers/Settings/ProviderCredentialControllerTest.phpapp/Console/Commands/LoginCommand.phpeslint.config.js
tests/{Feature,Unit}/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Tests live in the
tests/Featureandtests/Unitdirectories
Files:
tests/Feature/Http/Controllers/Settings/ProviderCredentialControllerTest.php
**/app/**/*.php
📄 CodeRabbit inference engine (GEMINI.md)
Use environment variables only in configuration files - never use the
env()function directly outside of config files
Files:
app/Console/Commands/LoginCommand.php
**/app/Console/Commands/**/*.php
📄 CodeRabbit inference engine (GEMINI.md)
Commands auto-register from
app/Console/Commands/and do not require manual registration
Files:
app/Console/Commands/LoginCommand.php
@(app|src)/**/*.php
📄 CodeRabbit inference engine (AGENTS.md)
Use environment variables only in configuration files - never use the
env()function directly outside of config files. Always useconfig('app.name'), notenv('APP_NAME')
Files:
app/Console/Commands/LoginCommand.php
app/Console/Commands/**/*.php
📄 CodeRabbit inference engine (AGENTS.md)
Commands auto-register - files in
app/Console/Commands/are automatically available and do not require manual registration
Files:
app/Console/Commands/LoginCommand.php
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use named imports for tree-shaking (e.g.,import { show } from '@/actions/...'); avoid default controller imports
Use Wayfinder HTTP method calls (.get(),.post(),.patch(),.put(),.delete()) for specific methods
For Wayfinder invokable controllers, import and invoke directly as functions
For non-controller routes in Wayfinder, import from@/routes/using named route names
Wayfinder detects route keys (e.g.,{post:slug}) and accepts matching object properties for parameter binding
Use WayfindermergeQueryto merge withwindow.location.search; set values tonullto remove them
Pass{ query: {...} }in options to Wayfinder to append query parameters
Wayfinder functions return{ url, method }shaped objects; use.url()to get just the URL string
Files:
eslint.config.js
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T02:45:25.865Z
Learning: Applies to tests/Browser/**/*.php : Use Laravel features like `Event::fake()`, `assertAuthenticated()`, and model factories within Pest v4 browser tests, as well as `RefreshDatabase` to ensure a clean state
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T02:49:56.681Z
Learning: Applies to tests/Browser/**/*.php : In Pest v4 browser tests, you can use Laravel features like `Event::fake()`, `assertAuthenticated()`, model factories, and `RefreshDatabase` to ensure clean state
📚 Learning: 2025-12-01T02:45:25.865Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T02:45:25.865Z
Learning: Applies to resources/js/**/*.{ts,tsx,vue} : Avoid default controller imports in Wayfinder as it prevents tree-shaking
Applied to files:
eslint.config.js
📚 Learning: 2025-12-01T02:44:35.071Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-01T02:44:35.071Z
Learning: Applies to resources/js/**/*.{vue,ts} : Always use Wayfinder named imports for tree-shaking (e.g., `import { show } from '@/actions/...'`); avoid default controller imports
Applied to files:
eslint.config.js
📚 Learning: 2025-12-01T02:45:25.865Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T02:45:25.865Z
Learning: Applies to resources/js/**/*.{ts,tsx,vue} : Always prefer named imports for tree-shaking when importing Wayfinder-generated functions (e.g., `import { show } from '@/actions/...'`)
Applied to files:
eslint.config.js
📚 Learning: 2025-12-09T02:49:56.681Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T02:49:56.681Z
Learning: Applies to resources/js/**/*.{vue,jsx,tsx} : Forms can also be built using the `useForm` helper for more programmatic control; follow existing conventions
Applied to files:
eslint.config.js
📚 Learning: 2025-12-01T02:44:35.071Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-01T02:44:35.071Z
Learning: Applies to {resources/js/**/*.vue,resources/views/**/*.blade.php,resources/css/**/*.css} : Always use Tailwind CSS v4 and do not use deprecated utilities; use the replacement utilities from the v4 migration table
Applied to files:
eslint.config.js
📚 Learning: 2025-12-01T02:45:25.865Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T02:45:25.865Z
Learning: Applies to resources/js/**/*.{ts,tsx,vue,js} : Use camelCase for variable names
Applied to files:
eslint.config.js
📚 Learning: 2025-12-09T02:49:56.681Z
Learnt from: CR
Repo: jordanpartridge/chat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T02:49:56.681Z
Learning: Applies to resources/js/Pages/**/*.{vue,jsx,tsx} : Inertia.js components should be placed in the `resources/js/Pages` directory unless specified differently in the JS bundler (vite.config.js)
Applied to files:
eslint.config.js
🧬 Code graph analysis (1)
app/Console/Commands/LoginCommand.php (1)
scripts/login.cjs (2)
url(3-3)
🔇 Additional comments (2)
eslint.config.js (1)
10-10: LGTM!Adding
scripts/*to the ESLint ignore patterns is appropriate for the new Playwright automation script. Utility scripts don't require frontend linting.tests/Feature/Http/Controllers/Settings/ProviderCredentialControllerTest.php (1)
552-553: LGTM!Using factory states (
->openai()and->anthropic()) instead of manually setting provider attributes follows Laravel best practices and improves test maintainability.
| class LoginCommand extends Command | ||
| { | ||
| protected $signature = 'login {--email=dev@test.com : Email for the test user}'; | ||
|
|
||
| protected $description = 'Ensure a user exists and login via browser'; |
There was a problem hiding this comment.
Add environment guards to prevent production use.
This command creates users with a known password ('password') and should never run in production. Add an environment check in the handle() method to ensure it only runs in local/testing environments.
Apply this diff to add a production guard:
public function handle(): int
{
+ if (! app()->environment(['local', 'testing'])) {
+ $this->error('This command can only be run in local or testing environments.');
+ return self::FAILURE;
+ }
+
$email = $this->option('email');Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/Console/Commands/LoginCommand.php around lines 10 to 14, the command can
create users with a known password and must not run in production; update the
handle() method to check the application environment and abort when running in
production (or non-local/testing) by detecting env via app()->environment() or
config('app.env'), outputting an error message and returning a non-zero exit
code so the command safely refuses to run outside local/testing.
1 participant
Summary
Introduces a new
php artisan logincommand that streamlines the development workflow by automatically handling user creation and browser-based login via Playwright automation.What
The implementation includes:
Why
This significantly improves rapid development iteration by eliminating repetitive manual login steps. Developers can now authenticate with a single command instead of navigating forms multiple times during development sessions.
How to test
Run the command with default dev user:
Or specify a custom email:
Use via Claude Code:
The command will create the user if needed, then open a browser window and automatically fill in credentials and submit the login form, navigating to the dashboard upon success.
Summary by CodeRabbit
Documentation
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.