Develop#577
Conversation
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
WalkthroughDer PR erweitert die Tenant-Erstellung um CLI-Optionen, stellt Auth-Tests und Testinfrastruktur auf Multi-Tenancy um, führt E-Mail-Archivierung ein, macht Dokument-Metadatenfelder nullable inkl. Migration/Factory-Anpassungen, stellt Bookkeeping-Rule-Aktionen auf contact_id um und bereinigt diverse Formatierungen/Konfigurationen. ChangesTenant-Erstellung per CLI-Optionen
Multi-Tenancy Auth- und Testinfrastruktur
E-Mail-Archivierung/Wiederherstellung
Dokument-Metadaten nullable & Upload-Flow
Bookkeeping-Rule Contact-Zuordnung
Formatierung, Konfiguration & sonstige Bereinigungen
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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: 7
🧹 Nitpick comments (4)
tests/Feature/DocumentControllerTest.php (1)
358-358: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFQN für
Queue::fake()inkonsistent mit Kurzform an anderer Stelle.Zeile 358 verwendet
Illuminate\Support\Facades\Queue::fake()(voll qualifiziert), während Zeile 378Queue::assertPushed(...)(Kurzform via Facade-Alias) nutzt. Einuse Illuminate\Support\Facades\Queue;-Import am Dateianfang würde beide Aufrufe vereinheitlichen.♻️ Vorschlag: Queue importieren und FQN entfernen
use App\Jobs\ProcessMultiDocJob; use App\Models\Contact; use App\Models\Document; use App\Models\DocumentType; use App\Models\Project; use App\Models\Tenant; use App\Models\User; use Illuminate\Http\UploadedFile; +use Illuminate\Support\Facades\Queue; use Stancl\Tenancy\Database\Models\Domain; use Stancl\Tenancy\Facades\Tenancy;Und an der Verwendungsstelle:
- Illuminate\Support\Facades\Queue::fake(); + Queue::fake();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Feature/DocumentControllerTest.php` at line 358, The Queue facade usage is inconsistent in DocumentControllerTest: `Queue::fake()` is written with a fully qualified name while `Queue::assertPushed(...)` already uses the alias. Add the `Illuminate\Support\Facades\Queue` import at the top of the test file and update the `Queue::fake()` call in the test setup to use the same short facade form as the other Queue assertions.tests/Feature/BookkeepingRuleServiceTest.php (1)
194-195: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: Assertion im Inactive-Rule-Test robuster machen.
Die Assertion
expect($transaction->contact_id)->toBe(1)verlässt sich auf den Factory-Defaultcontact_id => 1. Der Kommentar erklärt das, aber bei einer Factory-Änderung bricht der Test ohne klare Fehlerursache. Alternativ: den ursprünglichen Wert erfassen und auf Unveränderlichkeit prüfen.♻️ Vorschlag
// Run the service $service = new BookkeepingRuleService; $service->run('transactions', new Transaction, [$transaction->id]); // Verify the transaction was NOT updated + $initialContactId = $transaction->contact_id; $transaction->refresh(); - // Factory default contact_id is 1, and inactive rule should not change it - expect($transaction->contact_id)->toBe(1); + // Inactive rule should not change contact_id + expect($transaction->contact_id)->toBe($initialContactId);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Feature/BookkeepingRuleServiceTest.php` around lines 194 - 195, In the inactive-rule test in BookkeepingRuleServiceTest, the assertion currently depends on the factory default contact_id value, which makes the test brittle. Update the test around the $transaction assertion to capture the original contact_id before applying the rule and then assert that BookkeepingRuleService leaves it unchanged, using the existing $transaction variable and the inactive rule setup to locate the check.database/factories/TransactionFactory.php (1)
20-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional:
booked_onim Factory-Default ergänzen.
booked_onist in$fillableenthalten und wird alsdategecastet, fehlt aber in der Factory-Definition. Für aktuelle Tests ist das nicht problematisch, aber zukünftige Tests, diebooked_onbenötigen, müssten es manuell überschreiben.♻️ Vorschlag
return [ 'mm_ref' => fake()->unique()->bothify('??-####'), 'contact_id' => 1, 'bank_account_id' => 1, 'valued_on' => fake()->date(), + 'booked_on' => fake()->date(), 'currency' => 'EUR', 'name' => fake()->name(), 'amount' => fake()->randomFloat(2, -1000, 1000), ];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@database/factories/TransactionFactory.php` around lines 20 - 28, The TransactionFactory default attributes are missing booked_on even though it is fillable and date-cast on the Transaction model. Update the factory definition in TransactionFactory::definition() to include a sensible booked_on value alongside valued_on so tests can rely on it without manual overrides.tests/Feature/Auth/AuthenticationTest.php (1)
3-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTenancy-Setup in gemeinsamen Helper extrahieren
Das
beforeEach/afterEach-Setup (Tenant-/Domain-Erstellung,Tenancy::initialize,tenants:migrate, Teardown) ist identisch inAuthenticationTest,EmailVerificationTest,PasswordResetTestundPasswordUpdateTestdupliziert. Die Extraktion in einen gemeinsamen Pest-Helper oder Trait reduziert Wartungsaufwand und stellt Konsistenz sicher.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Feature/Auth/AuthenticationTest.php` around lines 3 - 22, The tenancy setup and teardown logic is duplicated across AuthenticationTest and the related auth feature tests, so extract the shared beforeEach/afterEach flow into a common Pest helper or reusable trait. Move the Tenant factory creation, Domain creation, Tenancy::initialize, tenants:migrate call, and Tenancy::end/logout cleanup into a shared setup used by AuthenticationTest, EmailVerificationTest, PasswordResetTest, and PasswordUpdateTest so all tests reuse the same tenancy bootstrap.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Console/Commands/CreateTenantCommand.php`:
- Around line 52-67: The CreateTenantCommand flow is overwriting the --subdomain
value by always assigning $data['domain'] from
CloudRegisterService::verifyEmailAddressAndCredentials(), so the CLI option has
no effect and the fallback website value can leave domain empty. Update the
CreateTenantCommand logic around the $data and $admin setup so that
verifyEmailAddressAndCredentials() only supplies a domain when one is not
already provided, preserving the --subdomain option as the source of truth and
using the website-derived domain only as a fallback.
In `@app/Models/Document.php`:
- Around line 70-84: `Document::$fillable` enthält noch `description`, obwohl
die Spalte durch die Migration zu `summary` umbenannt bzw. direkt als `summary`
angelegt wird. Entferne den veralteten `description`-Eintrag aus `Document` und
stelle sicher, dass nur das aktuelle Attribut `summary` für Mass-Assignment
verwendet wird, damit die Model-Attributliste zur Migration passt.
In `@resources/js/Pages/App/Email/EmailIndex.tsx`:
- Around line 53-54: Remove the leftover debug logging from the EmailIndex
component: the handleArchive callback still contains a console.log('E-Mail
archivieren:') statement that should not ship. Update handleArchive in
EmailIndex.tsx to keep the archive logic but delete the console.log call, and
verify no other debug prints remain in that handler.
In `@routes/tenant.php`:
- Around line 89-90: The archive and unarchive routes currently use an optional
{mail?} parameter even though EmailController::archive and
EmailController::unarchive require a non-null DropboxMail. Update the route
definitions in Route::put for app.email.archive and app.email.unarchive to use a
required {mail} parameter, matching the other email routes like destroy and move
so the controller methods always receive a valid mail model.
In `@tests/Feature/CreateTenantCommandTest.php`:
- Around line 43-57: The create:tenant command test currently verifies only the
exit code and output, so it misses regressions where the subdomain/domain
mapping is wrong. Update the test around Artisan::call and the createTenant()
callback to assert the passed $data['domain'] value (derived from the
--subdomain input) in addition to the existing output checks, using the existing
createTenant-related test setup to locate the callback easily.
In `@tests/Feature/DocumentControllerTest.php`:
- Around line 394-397: The PDF/preview streaming tests are asserting an Internal
Server Error, which only verifies failure instead of route behavior. Update the
tests around the document streaming request in DocumentControllerTest to expect
successful behavior by either mocking the media streaming layer or using a real
fixture with assertStatus(200); if the intent is missing-file handling, assert a
specific 404 or a targeted exception instead. Keep the test focused on the
relevant controller path and response using the existing document PDF/preview
request setup.
In `@tests/Pest.php`:
- Around line 19-25: In the Pest.php beforeEach hook,
glob(public_path('public-*')) can return false and cause a TypeError in PHP 8+
when used directly in foreach. Update the cleanup logic to guard the glob()
result before iterating, or normalize it to an empty array, so the loop only
runs when a list of paths exists. Keep the change scoped to the beforeEach
closure and the existing is_link/is_dir cleanup behavior.
---
Nitpick comments:
In `@database/factories/TransactionFactory.php`:
- Around line 20-28: The TransactionFactory default attributes are missing
booked_on even though it is fillable and date-cast on the Transaction model.
Update the factory definition in TransactionFactory::definition() to include a
sensible booked_on value alongside valued_on so tests can rely on it without
manual overrides.
In `@tests/Feature/Auth/AuthenticationTest.php`:
- Around line 3-22: The tenancy setup and teardown logic is duplicated across
AuthenticationTest and the related auth feature tests, so extract the shared
beforeEach/afterEach flow into a common Pest helper or reusable trait. Move the
Tenant factory creation, Domain creation, Tenancy::initialize, tenants:migrate
call, and Tenancy::end/logout cleanup into a shared setup used by
AuthenticationTest, EmailVerificationTest, PasswordResetTest, and
PasswordUpdateTest so all tests reuse the same tenancy bootstrap.
In `@tests/Feature/BookkeepingRuleServiceTest.php`:
- Around line 194-195: In the inactive-rule test in BookkeepingRuleServiceTest,
the assertion currently depends on the factory default contact_id value, which
makes the test brittle. Update the test around the $transaction assertion to
capture the original contact_id before applying the rule and then assert that
BookkeepingRuleService leaves it unchanged, using the existing $transaction
variable and the inactive rule setup to locate the check.
In `@tests/Feature/DocumentControllerTest.php`:
- Line 358: The Queue facade usage is inconsistent in DocumentControllerTest:
`Queue::fake()` is written with a fully qualified name while
`Queue::assertPushed(...)` already uses the alias. Add the
`Illuminate\Support\Facades\Queue` import at the top of the test file and update
the `Queue::fake()` call in the test setup to use the same short facade form as
the other Queue assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5b7cb700-0222-41d2-aeac-7a468fce5fa6
⛔ Files ignored due to path filters (2)
app/Http/.DS_Storeis excluded by!**/.DS_Storedatabase/.DS_Storeis excluded by!**/.DS_Store
📒 Files selected for processing (56)
.gitignoreapp/Console/Commands/CreateTenantCommand.phpapp/Data/DocumentData.phpapp/Http/Controllers/App/EmailController.phpapp/Http/Controllers/App/InboxController.phpapp/Http/Controllers/App/OfferController.phpapp/Http/Controllers/App/Setting/ZugferdSettingController.phpapp/Http/Controllers/Auth/VerifyEmailController.phpapp/Http/Controllers/ProfileController.phpapp/Models/BookkeepingRuleAction.phpapp/Models/BookkeepingRuleCondition.phpapp/Models/Document.phpapp/Models/Transaction.phpconfig/database.phpdatabase/factories/DocumentFactory.phpdatabase/factories/TenantFactory.phpdatabase/factories/TransactionFactory.phpdatabase/migrations/tenant/2026_02_19_135534_add_received_on_to_documents.phpdatabase/migrations/tenant/2026_07_08_172956_add_archived_at_to_dropbox_emails.phplang/de/auth.phplang/de/http-statuses.phplang/de/pagination.phplang/de/passwords.phplang/de/validation.phplang/en/auth.phplang/en/http-statuses.phplang/en/pagination.phplang/en/passwords.phplang/en/validation.phpphpunit.xmlresources/js/Lib/DateHelper.tsresources/js/Pages/App/Email/EmailIndex.tsxresources/js/Types/generated.d.tsresources/typescript-transformer-manifest.jsonroutes/tenant.phproutes/tenant/settings.phptests/Feature/App/Setting/Booking/SeasonCreateControllerTest.phptests/Feature/Auth/AuthenticationTest.phptests/Feature/Auth/EmailVerificationTest.phptests/Feature/Auth/PasswordConfirmationTest.phptests/Feature/Auth/PasswordResetTest.phptests/Feature/Auth/PasswordUpdateTest.phptests/Feature/Auth/RegistrationTest.phptests/Feature/BookkeepingRuleServiceTest.phptests/Feature/CreateTenantCommandTest.phptests/Feature/DocumentControllerTest.phptests/Feature/DocumentControllerValidationTest.phptests/Feature/ExampleTest.phptests/Feature/InvoiceTaxRoundingTest.phptests/Feature/ProfileTest.phptests/Pest.phptests/TestCase.phptests/Unit/BookmarkFolderTest.phptests/Unit/HasDynamicFiltersNullOperatorTest.phptests/Unit/HasDynamicFiltersTest.phptests/Unit/ReceiptFilenameTest.php
💤 Files with no reviewable changes (5)
- routes/tenant/settings.php
- tests/Feature/InvoiceTaxRoundingTest.php
- tests/Unit/ReceiptFilenameTest.php
- tests/Feature/App/Setting/Booking/SeasonCreateControllerTest.php
- tests/Unit/BookmarkFolderTest.php
| $data = [ | ||
| 'company' => $this->option('company') ?? '', | ||
| 'first_name' => $this->option('first-name') ?? '', | ||
| 'last_name' => $this->option('last-name') ?? '', | ||
| 'email' => $this->option('email') ?? '', | ||
| 'website' => $this->option('website') ?? 'https://', | ||
| 'domain' => $this->option('subdomain') ?? '', | ||
| ]; | ||
|
|
||
| $admin = [ | ||
| 'first_name' => $this->option('admin-first-name') ?? '', | ||
| 'last_name' => $this->option('admin-last-name') ?? '', | ||
| 'email' => $this->option('admin-email') ?? '', | ||
| ]; | ||
|
|
||
| $data['domain'] = CloudRegisterService::verifyEmailAddressAndCredentials($data); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File outline ==\n'
ast-grep outline app/Console/Commands/CreateTenantCommand.php --view expanded || true
printf '\n== Relevant lines from CreateTenantCommand.php ==\n'
cat -n app/Console/Commands/CreateTenantCommand.php | sed -n '1,240p'
printf '\n== Search for verifyEmailAddressAndCredentials usages ==\n'
rg -n "verifyEmailAddressAndCredentials|stepDomain|subdomain|website" app tests -S
printf '\n== Service file candidates ==\n'
fd -a "CloudRegisterService.php|*CloudRegisterService*" app tests . || trueRepository: dspangenberg/opsc
Length of output: 13580
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== CloudRegisterService outline ==\n'
ast-grep outline app/Services/CloudRegisterService.php --view expanded || true
printf '\n== CloudRegisterService relevant lines ==\n'
cat -n app/Services/CloudRegisterService.php | sed -n '1,220p'
printf '\n== CreateTenantCommand tests ==\n'
cat -n tests/Feature/CreateTenantCommandTest.php | sed -n '1,220p'Repository: dspangenberg/opsc
Length of output: 7375
--subdomain im Options-Pfad nicht überschreiben
$data['domain'] wird zwar aus --subdomain gesetzt, direkt danach aber immer durch verifyEmailAddressAndCredentials() ersetzt. Dadurch hat --subdomain keinen Effekt; ohne --website bleibt die Domain sogar leer, weil der Default https:// keinen Host liefert.
Vorschlag
- $data['domain'] = CloudRegisterService::verifyEmailAddressAndCredentials($data);
+ if ($data['domain'] === '') {
+ $data['domain'] = CloudRegisterService::verifyEmailAddressAndCredentials($data);
+ }📝 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.
| $data = [ | |
| 'company' => $this->option('company') ?? '', | |
| 'first_name' => $this->option('first-name') ?? '', | |
| 'last_name' => $this->option('last-name') ?? '', | |
| 'email' => $this->option('email') ?? '', | |
| 'website' => $this->option('website') ?? 'https://', | |
| 'domain' => $this->option('subdomain') ?? '', | |
| ]; | |
| $admin = [ | |
| 'first_name' => $this->option('admin-first-name') ?? '', | |
| 'last_name' => $this->option('admin-last-name') ?? '', | |
| 'email' => $this->option('admin-email') ?? '', | |
| ]; | |
| $data['domain'] = CloudRegisterService::verifyEmailAddressAndCredentials($data); | |
| $data = [ | |
| 'company' => $this->option('company') ?? '', | |
| 'first_name' => $this->option('first-name') ?? '', | |
| 'last_name' => $this->option('last-name') ?? '', | |
| 'email' => $this->option('email') ?? '', | |
| 'website' => $this->option('website') ?? 'https://', | |
| 'domain' => $this->option('subdomain') ?? '', | |
| ]; | |
| $admin = [ | |
| 'first_name' => $this->option('admin-first-name') ?? '', | |
| 'last_name' => $this->option('admin-last-name') ?? '', | |
| 'email' => $this->option('admin-email') ?? '', | |
| ]; | |
| if ($data['domain'] === '') { | |
| $data['domain'] = CloudRegisterService::verifyEmailAddressAndCredentials($data); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/Console/Commands/CreateTenantCommand.php` around lines 52 - 67, The
CreateTenantCommand flow is overwriting the --subdomain value by always
assigning $data['domain'] from
CloudRegisterService::verifyEmailAddressAndCredentials(), so the CLI option has
no effect and the fallback website value can leave domain empty. Update the
CreateTenantCommand logic around the $data and $admin setup so that
verifyEmailAddressAndCredentials() only supplies a domain when one is not
already provided, preserving the --subdomain option as the source of truth and
using the website-derived domain only as a fallback.
| $output = new BufferedOutput; | ||
| $exitCode = Artisan::call('create:tenant', [ | ||
| '--company' => 'Test', | ||
| '--first-name' => 'John', | ||
| '--last-name' => 'Doe', | ||
| '--email' => '[email protected]', | ||
| '--website' => 'https://example.com', | ||
| '--subdomain' => 'myapp', | ||
| '--admin-first-name' => 'Admin', | ||
| '--admin-last-name' => 'User', | ||
| '--admin-email' => '[email protected]', | ||
| ], $output); | ||
|
|
||
| expect($exitCode)->toBe(0); | ||
| expect($output->fetch())->toContain('Mandant und Admin-User werden erstellt'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Testabdeckung: die resultierende Domain/Subdomain wird nicht geprüft.
Beide Tests übergeben --subdomain, verifizieren aber nur exitCode und die Ausgabemeldung. Der an createTenant() übergebene domain-Wert wird nicht assertiert, wodurch der oben beschriebene Fehler (überschriebene Subdomain) unentdeckt bleibt. Eine Assertion auf $data['domain'] im createTenant-Callback würde diese Lücke schließen.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/Feature/CreateTenantCommandTest.php` around lines 43 - 57, The
create:tenant command test currently verifies only the exit code and output, so
it misses regressions where the subdomain/domain mapping is wrong. Update the
test around Artisan::call and the createTenant() callback to assert the passed
$data['domain'] value (derived from the --subdomain input) in addition to the
existing output checks, using the existing createTenant-related test setup to
locate the callback easily.
| ->get('http://'.$this->domain->domain.'/app/documents/pdf/'.$document->id); | ||
|
|
||
| // Überprüfe, dass die Route existiert und eine Antwort gibt | ||
| $response->assertStatus(200); | ||
| $response->assertStatus(500); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
Tests auf assertStatus(500) sind fragil und testen fehlerhaftes Verhalten.
Die PDF- und Preview-Streaming-Tests erwarten nun assertStatus(500) statt eines erfolgreichen Status. Ein Test, der einen Internal Server Error als erwartet festlegt, verifiziert nicht, dass die Route korrekt funktioniert, sondern nur, dass sie fehlschlägt. Das ist problematisch, weil:
- Die Tests auch dann grün bleiben, wenn die Route komplett kaputt ist.
- Jede Verbesserung der Fehlerbehandlung (z.B. Rückgabe von 404 bei fehlender Datei) würde den Test fälschlicherweise brechen.
- Kein positives Verhalten getestet wird — nur dass etwas schiefgeht.
Stattdessen sollte entweder das Media-Streaming gemockt oder ein assertStatus(200) mit einer echten Testdatei verwendet werden. Alternativ liefert assertStatus(404) oder eine gezielte Exception-Assertion zumindest eine klarere Aussage als ein unbehandelter 500er.
[medium_effort_and_high_reward]
Also applies to: 413-416
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/Feature/DocumentControllerTest.php` around lines 394 - 397, The
PDF/preview streaming tests are asserting an Internal Server Error, which only
verifies failure instead of route behavior. Update the tests around the document
streaming request in DocumentControllerTest to expect successful behavior by
either mocking the media streaming layer or using a real fixture with
assertStatus(200); if the intent is missing-file handling, assert a specific 404
or a targeted exception instead. Keep the test focused on the relevant
controller path and response using the existing document PDF/preview request
setup.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 34686651 | Triggered | Generic Password | b79f0a5 | tests/Feature/Auth/PasswordUpdateTest.php | View secret |
| 34686651 | Triggered | Generic Password | b79f0a5 | tests/Feature/Auth/PasswordResetTest.php | View secret |
| 34686651 | Triggered | Generic Password | b79f0a5 | tests/Feature/Auth/PasswordUpdateTest.php | View secret |
| 34686651 | Triggered | Generic Password | b79f0a5 | tests/Feature/Auth/PasswordResetTest.php | View secret |
| 34686651 | Triggered | Generic Password | b79f0a5 | tests/Feature/Auth/PasswordUpdateTest.php | View secret |
| 34686651 | Triggered | Generic Password | b79f0a5 | tests/Feature/Auth/PasswordUpdateTest.php | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes