Merge pull request #173 from trypostit/fix/autofill-save-logo

Save the autofilled site logo as the workspace logo on brand settings update

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')
@@ -198,7 +188,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 +199,16 @@ 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) {
+            $logoAttacher->attach($workspace, $logoUrl);
+        }
 
         return back()->with('flash.success', __('settings.flash.workspace_updated'));
     }

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'],
         ];
     }
 

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);

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 = () => {

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();
+});

tests/Unit/Services/Brand/LogoAttacherTest.php

@@ -0,0 +1,53 @@
+<?php
+
+declare(strict_types=1);
+
+use App\Models\Workspace;
+use App\Services\Brand\LogoAttacher;
+use Illuminate\Support\Facades\Http;
+
+// A public IP literal as the host lets SafeHttpFetcher's SSRF guard pass without
+// a real DNS lookup; Http::fake() intercepts the request before any network I/O.
+const LOGO_URL = 'https://93.184.216.34/logo.png';
+
+function fakeLogo(string $mime = 'image/png', string $body = 'PNG'): void
+{
+    Http::fake(['*' => 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', '<html></html>');
+
+    $workspace = mock(Workspace::class);
+    $workspace->shouldNotReceive('addMediaFromPath');
+
+    expect(app(LogoAttacher::class)->attach($workspace, LOGO_URL))->toBeFalse();
+});