-
Notifications
You must be signed in to change notification settings - Fork 669
Fix silent logo upload failure for signed-out users #394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| import { PlusIcon } from "lucide-react"; | ||
| import Image from "next/image"; | ||
| import { type ChangeEvent, type DragEvent, useRef, useState } from "react"; | ||
| import { toast } from "sonner"; | ||
| import { createClient } from "@/utils/supabase/client"; | ||
|
|
||
| interface UploadLogoProps { | ||
|
|
@@ -23,33 +24,52 @@ export default function UploadLogo({ | |
|
|
||
| const handleFile = async (file: File) => { | ||
| if (!file.type.startsWith("image/")) { | ||
| toast.error("Please select an image file."); | ||
| return; | ||
| } | ||
|
|
||
| const MAX_FILE_SIZE = 1024 * 1024; // 1MB in bytes | ||
| if (file.size > MAX_FILE_SIZE) { | ||
| toast.error("Image must be smaller than 1MB."); | ||
| return; | ||
| } | ||
|
|
||
| const previousPreview = preview; | ||
| setIsUploading(true); | ||
|
|
||
| // Show an optimistic preview while the upload is in flight. It is reverted | ||
| // below if the upload fails so the UI never shows an image that wasn't | ||
| // actually saved. | ||
| const reader = new FileReader(); | ||
| reader.onload = (e) => { | ||
| setPreview(e.target?.result as string); | ||
| }; | ||
| reader.readAsDataURL(file); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Late FileReader restores failed previewMedium Severity · Logic Bug The optimistic Additional Locations (2)Reviewed by Cursor Bugbot for commit 6f9bbb8. Configure here. |
||
|
|
||
| try { | ||
| // Create preview | ||
| const reader = new FileReader(); | ||
| reader.onload = (e) => { | ||
| const dataUrl = e.target?.result as string; | ||
| setPreview(dataUrl); | ||
| }; | ||
| reader.readAsDataURL(file); | ||
|
|
||
| // Upload to Supabase Storage | ||
| const supabase = createClient(); | ||
|
|
||
| // Uploading to the avatars bucket requires an authenticated session. The | ||
| // bucket's row-level security policy only allows the `authenticated` | ||
| // role, so without a session the request is sent as `anon` and Storage | ||
| // rejects it with "new row violates row-level security policy". | ||
| const { | ||
| data: { user }, | ||
| } = await supabase.auth.getUser(); | ||
|
|
||
| if (!user) { | ||
| toast.error("Please sign in to upload an image."); | ||
| setPreview(previousPreview); | ||
| return; | ||
| } | ||
|
|
||
| const fileExt = file.name.split(".").pop(); | ||
| const fileName = `${Math.random().toString(36).substring(2)}.${fileExt}`; | ||
| const path = `${prefix}/${fileName}`; | ||
|
|
||
| const { data, error } = await supabase.storage | ||
| const { error } = await supabase.storage | ||
| .from("avatars") | ||
| .upload(`${prefix}/${fileName}`, file, { | ||
| .upload(path, file, { | ||
| cacheControl: "3600", | ||
| upsert: false, | ||
| }); | ||
|
|
@@ -58,16 +78,18 @@ export default function UploadLogo({ | |
| throw error; | ||
| } | ||
|
|
||
| // Get public URL | ||
| const { | ||
| data: { publicUrl }, | ||
| } = supabase.storage | ||
| .from("avatars") | ||
| .getPublicUrl(`${prefix}/${fileName}`); | ||
| } = supabase.storage.from("avatars").getPublicUrl(path); | ||
|
|
||
| setPreview(publicUrl); | ||
| onUpload?.(publicUrl); | ||
| } catch (error) { | ||
| console.error("Error uploading file:", error); | ||
| toast.error( | ||
| error instanceof Error ? error.message : "Failed to upload image.", | ||
| ); | ||
| setPreview(previousPreview); | ||
| } finally { | ||
| setIsUploading(false); | ||
| } | ||
|
|
@@ -149,6 +171,12 @@ export default function UploadLogo({ | |
| <PlusIcon className="size-4" /> | ||
| </div> | ||
| )} | ||
|
|
||
| {isUploading && ( | ||
| <div className="absolute inset-0 flex items-center justify-center rounded-lg bg-black/50 text-[10px] font-medium text-white"> | ||
| Uploading... | ||
| </div> | ||
| )} | ||
| </div> | ||
| ); | ||
| } | ||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Session loading redirects signed-in users
Medium Severity · Logic Bug
isAuthenticatedstarts asnullwhilegetSessionruns, buthandleClicktreats any falsy value like a signed-out user and sends them to/login. A signed-in user who clicks Add company before the effect finishes can be redirected incorrectly.Reviewed by Cursor Bugbot for commit 6f9bbb8. Configure here.