From b7773849ecd5e40f1d3ac268509022982bdd5f5a Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Fri, 3 Jul 2026 20:05:54 -0300 Subject: [PATCH 1/2] Save the autofilled site logo as the workspace logo on brand settings update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Brand autofill on the workspace settings page already captured the site logo and rendered a preview beneath the URL, but the update flow never persisted it. The store flow attached it via LogoAttacher; the update flow was missing all three legs: the form field, the request rule, and the controller attach. - BrandTab: add logo_url to the useForm payload so autofill can set it and the form submits it. - UpdateWorkspaceRequest: validate logo_url (nullable url) — FormRequest strips any unvalidated key, so without a rule it was silently dropped. - WorkspaceController::updateSettings: pull logo_url out of the validated data (it is not a column) and attach it through LogoAttacher, mirroring store. --- .../Controllers/App/WorkspaceController.php | 21 +++++++++++++++-- .../App/Workspace/UpdateWorkspaceRequest.php | 1 + resources/js/components/settings/BrandTab.vue | 1 + tests/Feature/WorkspaceControllerTest.php | 23 +++++++++++++++++++ 4 files changed, 44 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/App/WorkspaceController.php b/app/Http/Controllers/App/WorkspaceController.php index e26a22f7..a8451a02 100644 --- a/app/Http/Controllers/App/WorkspaceController.php +++ b/app/Http/Controllers/App/WorkspaceController.php @@ -198,7 +198,7 @@ public function deleteLogo(Request $request): RedirectResponse return back()->with('flash.success', __('settings.flash.logo_deleted')); } - public function updateSettings(UpdateWorkspaceRequest $request): RedirectResponse + public function updateSettings(UpdateWorkspaceRequest $request, LogoAttacher $logoAttacher): RedirectResponse { $user = $request->user(); $workspace = $user->currentWorkspace; @@ -209,7 +209,24 @@ public function updateSettings(UpdateWorkspaceRequest $request): RedirectRespons $this->authorize('update', $workspace); - $workspace->update($request->validated()); + $validated = $request->validated(); + + $logoUrl = data_get($validated, 'logo_url'); + unset($validated['logo_url']); + + $workspace->update($validated); + + if ($logoUrl) { + try { + $logoAttacher->attach($workspace, $logoUrl); + } catch (Throwable $e) { + Log::warning('Logo attach failed during workspace update', [ + 'workspace_id' => $workspace->id, + 'logo_url' => $logoUrl, + 'error' => $e->getMessage(), + ]); + } + } return back()->with('flash.success', __('settings.flash.workspace_updated')); } diff --git a/app/Http/Requests/App/Workspace/UpdateWorkspaceRequest.php b/app/Http/Requests/App/Workspace/UpdateWorkspaceRequest.php index 4a8f0dba..6bb20ef0 100644 --- a/app/Http/Requests/App/Workspace/UpdateWorkspaceRequest.php +++ b/app/Http/Requests/App/Workspace/UpdateWorkspaceRequest.php @@ -34,6 +34,7 @@ public function rules(): array 'brand_font' => ['sometimes', 'required', 'string', Rule::in(BrandFont::values())], 'image_style' => ['sometimes', 'required', 'string', Rule::in(ImageStyle::values())], 'content_language' => ['sometimes', 'string', Rule::in(ContentLanguage::values())], + 'logo_url' => ['nullable', 'url', 'max:1024'], ]; } diff --git a/resources/js/components/settings/BrandTab.vue b/resources/js/components/settings/BrandTab.vue index dceecbb6..31b70738 100644 --- a/resources/js/components/settings/BrandTab.vue +++ b/resources/js/components/settings/BrandTab.vue @@ -40,6 +40,7 @@ const form = useForm({ brand_font: props.workspace.brand_font ?? 'Inter', image_style: props.workspace.image_style ?? 'cinematic', content_language: props.workspace.content_language ?? 'en', + logo_url: '' as string | null, }); const submit = () => { diff --git a/tests/Feature/WorkspaceControllerTest.php b/tests/Feature/WorkspaceControllerTest.php index 5f1970a1..796bef21 100644 --- a/tests/Feature/WorkspaceControllerTest.php +++ b/tests/Feature/WorkspaceControllerTest.php @@ -697,3 +697,26 @@ 'logo_url' => 'https://example.com/logo.png', ])->assertRedirect(); }); + +test('update settings attaches the autofilled logo when logo_url is provided', function () { + $logoAttacher = $this->mock(LogoAttacher::class); + $logoAttacher->shouldReceive('attach')->once(); + + $this->actingAs($this->user) + ->from(route('app.workspace.brand')) + ->put(route('app.workspace.settings.update'), [ + 'name' => $this->workspace->name, + 'logo_url' => 'https://example.com/logo.png', + ])->assertRedirect(route('app.workspace.brand')) + ->assertSessionHasNoErrors(); +}); + +test('update settings does not touch the logo when no logo_url is provided', function () { + $logoAttacher = $this->mock(LogoAttacher::class); + $logoAttacher->shouldReceive('attach')->never(); + + $this->actingAs($this->user) + ->put(route('app.workspace.settings.update'), [ + 'name' => $this->workspace->name, + ])->assertRedirect(); +}); From 3bc481ac07310e4932bf7832a05e7f220f3deb58 Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Fri, 3 Jul 2026 20:15:37 -0300 Subject: [PATCH 2/2] Make LogoAttacher honor its swallow-all contract; drop the caller try/catch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LogoAttacher::attach promised in its docblock that any failure — including a persistence error — is logged and swallowed so the caller need not handle it. But the persistence block was try/finally with no catch, so a Throwable from clearMediaCollection/addMediaFromPath escaped. Both call sites (store and updateSettings) each wrapped the call in an identical try/catch + Log::warning to compensate — a band-aid duplicated across the controller. Fix it at the root: the persistence block now catches Throwable, logs it, and returns false, matching the documented contract. Both controller call sites collapse to a single attach() line, and the now-unused Log/Throwable imports are dropped. Adds LogoAttacherTest covering the success path, the swallowed persistence failure, a failed fetch, and a rejected mime type. --- .../Controllers/App/WorkspaceController.php | 22 +------- app/Services/Brand/LogoAttacher.php | 5 ++ .../Unit/Services/Brand/LogoAttacherTest.php | 53 +++++++++++++++++++ 3 files changed, 60 insertions(+), 20 deletions(-) create mode 100644 tests/Unit/Services/Brand/LogoAttacherTest.php diff --git a/app/Http/Controllers/App/WorkspaceController.php b/app/Http/Controllers/App/WorkspaceController.php index a8451a02..d9d394ff 100644 --- a/app/Http/Controllers/App/WorkspaceController.php +++ b/app/Http/Controllers/App/WorkspaceController.php @@ -21,12 +21,10 @@ use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; use Illuminate\Http\Resources\Json\AnonymousResourceCollection; -use Illuminate\Support\Facades\Log; use Inertia\Inertia; use Inertia\Response; use RuntimeException; use Symfony\Component\HttpFoundation\Response as SymfonyResponse; -use Throwable; class WorkspaceController extends Controller { @@ -105,15 +103,7 @@ public function store(StoreWorkspaceRequest $request, LogoAttacher $logoAttacher $workspace = CreateWorkspace::execute($user, $validated); if ($logoUrl = data_get($validated, 'logo_url')) { - try { - $logoAttacher->attach($workspace, $logoUrl); - } catch (Throwable $e) { - Log::warning('Logo attach failed during workspace creation', [ - 'workspace_id' => $workspace->id, - 'logo_url' => $logoUrl, - 'error' => $e->getMessage(), - ]); - } + $logoAttacher->attach($workspace, $logoUrl); } return redirect()->route('app.accounts') @@ -217,15 +207,7 @@ public function updateSettings(UpdateWorkspaceRequest $request, LogoAttacher $lo $workspace->update($validated); if ($logoUrl) { - try { - $logoAttacher->attach($workspace, $logoUrl); - } catch (Throwable $e) { - Log::warning('Logo attach failed during workspace update', [ - 'workspace_id' => $workspace->id, - 'logo_url' => $logoUrl, - 'error' => $e->getMessage(), - ]); - } + $logoAttacher->attach($workspace, $logoUrl); } return back()->with('flash.success', __('settings.flash.workspace_updated')); diff --git a/app/Services/Brand/LogoAttacher.php b/app/Services/Brand/LogoAttacher.php index a091c7b3..71ca030a 100644 --- a/app/Services/Brand/LogoAttacher.php +++ b/app/Services/Brand/LogoAttacher.php @@ -6,6 +6,7 @@ use App\Models\Workspace; use Illuminate\Support\Facades\Log; +use Throwable; class LogoAttacher { @@ -77,6 +78,10 @@ public function attach(Workspace $workspace, string $logoUrl): bool ); return true; + } catch (Throwable $e) { + Log::warning('Logo rejected — persistence error', ['url' => $logoUrl, 'error' => $e->getMessage()]); + + return false; } finally { if (file_exists($tempPath)) { @unlink($tempPath); diff --git a/tests/Unit/Services/Brand/LogoAttacherTest.php b/tests/Unit/Services/Brand/LogoAttacherTest.php new file mode 100644 index 00000000..d43191a7 --- /dev/null +++ b/tests/Unit/Services/Brand/LogoAttacherTest.php @@ -0,0 +1,53 @@ + Http::response($body, 200, [ + 'Content-Type' => $mime, + 'Content-Length' => (string) strlen($body), + ])]); +} + +test('attaches the downloaded logo and returns true', function () { + fakeLogo(); + + $workspace = mock(Workspace::class); + $workspace->shouldReceive('clearMediaCollection')->once(); + $workspace->shouldReceive('addMediaFromPath')->once(); + + expect(app(LogoAttacher::class)->attach($workspace, LOGO_URL))->toBeTrue(); +}); + +test('swallows a persistence failure and returns false', function () { + fakeLogo(); + + $workspace = mock(Workspace::class); + $workspace->shouldReceive('clearMediaCollection')->once()->andThrow(new RuntimeException('media store down')); + + expect(app(LogoAttacher::class)->attach($workspace, LOGO_URL))->toBeFalse(); +}); + +test('returns false when the fetch fails', function () { + Http::fake(['*' => Http::response('', 500)]); + + expect(app(LogoAttacher::class)->attach(mock(Workspace::class), LOGO_URL))->toBeFalse(); +}); + +test('rejects a disallowed mime type and returns false', function () { + fakeLogo('text/html', ''); + + $workspace = mock(Workspace::class); + $workspace->shouldNotReceive('addMediaFromPath'); + + expect(app(LogoAttacher::class)->attach($workspace, LOGO_URL))->toBeFalse(); +});