Skip to content

Conversation

@HarshMN2345
Copy link
Member

@HarshMN2345 HarshMN2345 commented Dec 7, 2025

What does this PR do?

image

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)

Related PRs and Issues

(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)

Have you read the Contributing Guidelines on issues?

(Write your answer here.)

Summary by CodeRabbit

  • New Features

    • Added user-facing error notifications for file upload failures.
  • Improvements

    • File picker helper text now dynamically reflects allowed file types.
    • File picker is configurable for MIME-type filtering and specific allowed extensions, enabling more flexible file selection and clearer guidance.

✏️ Tip: You can customize this high-level summary in your review settings.

@appwrite
Copy link

appwrite bot commented Dec 7, 2025

Console (appwrite/console)

Project ID: 688b7bf400350cbd60e9

Sites (1)
Site Status Logs Preview QR
 console-stage
688b7cf6003b1842c9dc
Ready Ready View Logs Preview URL QR Code

Tip

Build commands execute in runtime containers during deployment

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 7, 2025

Walkthrough

Added two props to inputFilePicker.svelte: mimeTypeQuery (default "image/") and allowedExtension (default "") and forwarded them to FilePicker. In filePicker.svelte, Upload.Dropzone extensions is now undefined when allowedExtension === "" or an array containing the single extension otherwise. The dropzone tooltip content is conditional: shows a mimeTypeQuery-based message when allowedExtension is "*" or the specific extension otherwise. On upload errors, console.error(e) was replaced with addNotification({ type: 'error', message: e.message }).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify exported props and default values in src/lib/elements/forms/inputFilePicker.svelte.
  • Confirm props are forwarded/bound correctly where FilePicker is used.
  • Review src/lib/components/filePicker.svelte for correct conditional extensions handling and that Upload.Dropzone accepts undefined for extensions.
  • Check tooltip conditional rendering paths for correctness and phrasing.
  • Validate addNotification usage on error (handles missing message) and that no console.error regressions remain.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: enable file upload for push notifications' is directly related to the changeset, which adds file upload functionality through new props and notification handling.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-SER-468-file-upload-for-push-notifications

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1503e21 and 04a7ef0.

📒 Files selected for processing (1)
  • src/lib/elements/forms/inputFilePicker.svelte (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,svelte}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports

Files:

  • src/lib/elements/forms/inputFilePicker.svelte
**/*.{ts,tsx,js,jsx,svelte,json}

📄 CodeRabbit inference engine (AGENTS.md)

Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration

Files:

  • src/lib/elements/forms/inputFilePicker.svelte
**/*.svelte

📄 CodeRabbit inference engine (AGENTS.md)

Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development

Files:

  • src/lib/elements/forms/inputFilePicker.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: e2e
  • GitHub Check: build
🔇 Additional comments (2)
src/lib/elements/forms/inputFilePicker.svelte (2)

98-105: Prop wiring into FilePicker looks correct

The new FilePicker invocation correctly passes through selectedFile, selectedBucket, bind:show, onSelect, showLocalFileBucket, mimeTypeQuery, and allowedExtension. This aligns with the described changes in filePicker.svelte and the earlier suggestion to expose showLocalFileBucket.


15-16: No backward compatibility issues with new prop defaults

The new defaults (mimeTypeQuery = 'image/' and allowedExtension = '*') are appropriate and do not break existing usage. All three usages of InputFilePicker in the codebase are in push notification contexts (pushFormList.svelte, push.svelte, and pushMessage.svelte), all labeled "Media", and none override these props—they rely on image filtering, which aligns with the intended use case for push notification media attachments.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

}
selectFile(file);
} catch (e) {
console.error(e);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can remove this.

console.error(e);
addNotification({
type: 'error',
message: e.message || 'Failed to upload file'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there will always be a message from AppwriteException. No need for a fallback.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/lib/components/filePicker.svelte (1)

238-238: accept value is likely incorrect when allowedExtension === '*'

The extension variable becomes mimeTypeQuery directly when allowedExtension === '*', so with the default mimeTypeQuery = 'image/', the <input accept={extension}> attribute is set to accept="image/". This is not a valid browser accept pattern according to HTML5 specifications—valid wildcard patterns require an asterisk (e.g., image/*). The invalid pattern may cause the browser file picker to ignore the filter or behave inconsistently.

Consider deriving a proper accept pattern for the input case:

-    $: extension = allowedExtension === '*' ? mimeTypeQuery : `.${allowedExtension}`;
+    $: extension =
+        allowedExtension === '*'
+            ? `${mimeTypeQuery}*` // e.g. "image/*"
+            : `.${allowedExtension}`;

This keeps mimeTypeQuery as‑is for queries while ensuring the file input constraint conforms to the HTML5 standard.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 577410f and 44898e5.

📒 Files selected for processing (2)
  • src/lib/components/filePicker.svelte (4 hunks)
  • src/lib/elements/forms/inputFilePicker.svelte (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,svelte}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports

Files:

  • src/lib/elements/forms/inputFilePicker.svelte
  • src/lib/components/filePicker.svelte
**/*.{ts,tsx,js,jsx,svelte,json}

📄 CodeRabbit inference engine (AGENTS.md)

Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration

Files:

  • src/lib/elements/forms/inputFilePicker.svelte
  • src/lib/components/filePicker.svelte
**/*.svelte

📄 CodeRabbit inference engine (AGENTS.md)

Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development

Files:

  • src/lib/elements/forms/inputFilePicker.svelte
  • src/lib/components/filePicker.svelte
src/lib/components/**/*.svelte

📄 CodeRabbit inference engine (AGENTS.md)

Use PascalCase for component file names and place them in src/lib/components/[feature]/ directory structure

Files:

  • src/lib/components/filePicker.svelte
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to **/*.svelte : Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: e2e
  • GitHub Check: build
🔇 Additional comments (3)
src/lib/elements/forms/inputFilePicker.svelte (1)

15-16: Prop defaults and wiring look consistent with filePicker.svelte

mimeTypeQuery / allowedExtension are sensibly defaulted and correctly forwarded to <FilePicker>, keeping showLocalFileBucket={true} and existing bindings/handlers intact. This should be backwards compatible for existing usages that rely on images as the default.

Also applies to: 98-105

src/lib/components/filePicker.svelte (2)

36-36: Good addition of user-facing error notification on upload failure

Importing addNotification and surfacing an error when uploadFile() fails improves UX and debuggability, while keeping the existing console.error for dev insight.

If your TS config enables useUnknownInCatchVariables, you may want to narrow e first to avoid type errors and safely handle non-Error throws, for example:

const message =
    e instanceof Error ? e.message : 'Failed to upload file';

addNotification({ type: 'error', message });

Also applies to: 115-118


205-212: Query logic for mime vs extension filtering is reasonable

getProperQuery() switching between Query.startsWith('mimeType', mimeTypeQuery) when allowedExtension === '*' and Query.endsWith('name', \.${allowedExtension}`)` otherwise is a clear, simple way to support both mime-based and extension-based filtering without complicating the API.

Please sanity‑check that the backend consistently stores mimeType values prefixed with mimeTypeQuery (e.g. image/png for image/), and that filenames always contain the expected .${allowedExtension} suffix, especially for existing buckets.

Comment on lines +359 to +361
<Upload.Dropzone
bind:files={localFile}
extensions={allowedExtension === '*' ? undefined : [allowedExtension]}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Find the filePicker.svelte file and read the relevant sections
fd filePicker.svelte --type f

Repository: appwrite/console

Length of output: 96


🏁 Script executed:

# Read the filePicker.svelte file to see the relevant sections
cat -n src/lib/components/filePicker.svelte | sed -n '350,390p'

Repository: appwrite/console

Length of output: 2850


🏁 Script executed:

# Get the full file to understand context
wc -l src/lib/components/filePicker.svelte

Repository: appwrite/console

Length of output: 100


🏁 Script executed:

# Read the relevant sections of filePicker.svelte
head -n 400 src/lib/components/filePicker.svelte | tail -n +330

Repository: appwrite/console

Length of output: 4221


🏁 Script executed:

# Search for allowedExtension in the file to understand its format
rg 'allowedExtension' src/lib/components/filePicker.svelte -n -C 3

Repository: appwrite/console

Length of output: 2158


🏁 Script executed:

# Look for Upload.Dropzone in the codebase to find the component definition
find . -type f -name "*.svelte" -o -name "*.ts" -o -name "*.tsx" -o -name "*.js" | xargs rg "Upload\s*[\{\.]\s*Dropzone" -l

Repository: appwrite/console

Length of output: 973


🏁 Script executed:

# Check imports in filePicker.svelte to understand where Upload comes from
head -n 50 src/lib/components/filePicker.svelte

Repository: appwrite/console

Length of output: 2075


🏁 Script executed:

# Search for Upload imports in the file
rg "import.*Upload" src/lib/components/filePicker.svelte -A 2 -B 2

Repository: appwrite/console

Length of output: 42


🏁 Script executed:

# Let's read the first 100 lines to see imports
sed -n '1,100p' src/lib/components/filePicker.svelte

Repository: appwrite/console

Length of output: 3657


🌐 Web query:

@appwrite.io/pink-svelte Upload.Dropzone extensions prop format documentation

💡 Result:

I couldn't find official documentation for a pink-svelte Upload.Dropzone "extensions" prop at the URL you gave. Can you paste the exact page/repo link or a screenshot (or say whether you mean Appwrite's Pink UI repo or a different package)? I can then look up the exact prop format and examples.


🏁 Script executed:

# Read more of the filePicker file, especially around the relevant lines
sed -n '350,400p' src/lib/components/filePicker.svelte | cat -n

Repository: appwrite/console

Length of output: 3424


🏁 Script executed:

# Get a wider view of the file context
sed -n '1,20p' src/lib/components/filePicker.svelte && echo "---" && sed -n '340,395p' src/lib/components/filePicker.svelte

Repository: appwrite/console

Length of output: 4024


🏁 Script executed:

# Let's check the entire filePicker.svelte structure - maybe smaller sections
head -n 30 src/lib/components/filePicker.svelte

Repository: appwrite/console

Length of output: 1034


🏁 Script executed:

# Find all Svelte files in lib/components to understand the structure
ls -la src/lib/components/ | head -20

Repository: appwrite/console

Length of output: 1328


🏁 Script executed:

# Search for filePicker in all formats
find src/lib/components -name "*ilePicker*" -o -name "*file*icker*"

Repository: appwrite/console

Length of output: 96


🏁 Script executed:

# List all files in src/lib/components recursively
find src/lib/components -type f -name "*.svelte" | grep -i file

Repository: appwrite/console

Length of output: 96


🏁 Script executed:

# Get all svelte files in components
find src/lib/components -type f -name "*.svelte"

Repository: appwrite/console

Length of output: 7376


🏁 Script executed:

# Search for filePicker across the entire repo
find . -type f -name "*filePicker*"

Repository: appwrite/console

Length of output: 98


🏁 Script executed:

# Search for PascalCase FilePicker
find . -type f -name "*FilePicker*"

Repository: appwrite/console

Length of output: 107


🏁 Script executed:

# Use git to find the file
git ls-files | grep -i filepicker

Repository: appwrite/console

Length of output: 142


🏁 Script executed:

# Try searching for the Upload.Dropzone reference instead
rg "Upload\.Dropzone" --type svelte

Repository: appwrite/console

Length of output: 90


🏁 Script executed:

# Search more broadly for Upload.Dropzone
rg "Upload\.Dropzone"

Repository: appwrite/console

Length of output: 2479


🏁 Script executed:

# Try searching for Upload component imports
rg "from.*Upload" --type svelte --type ts --type tsx

Repository: appwrite/console

Length of output: 90


🏁 Script executed:

# Check the structure of the repo
ls -la

Repository: appwrite/console

Length of output: 2027


🏁 Script executed:

# List src directory
ls -la src/

Repository: appwrite/console

Length of output: 662


🏁 Script executed:

# Check if src/lib/components exists and its contents
ls -la src/lib/components/ 2>/dev/null || echo "Directory not found"

Repository: appwrite/console

Length of output: 7661


🏁 Script executed:

# Try to find if lib exists somewhere
find . -type d -name "components" 2>/dev/null | head -5

Repository: appwrite/console

Length of output: 410


🏁 Script executed:

# Get the full file structure of src
find src -type f | head -20

Repository: appwrite/console

Length of output: 579


🏁 Script executed:

# Check if the file path from the review exists
test -f "src/lib/components/filePicker.svelte" && echo "File exists" || echo "File not found"

Repository: appwrite/console

Length of output: 71


🏁 Script executed:

# Maybe case sensitive - try different variations
find . -iname "*filepicker*" -type f

Repository: appwrite/console

Length of output: 146


🏁 Script executed:

# Check git status to see what files are in the repo
git ls-files | head -30

Repository: appwrite/console

Length of output: 662


🏁 Script executed:

# Check package.json for @appwrite.io/pink-svelte version and dependencies
grep -A 5 -B 5 "pink-svelte" package.json

Repository: appwrite/console

Length of output: 666


🏁 Script executed:

# Search for any other Upload.Dropzone usages with extensions prop to understand the pattern
rg "extensions\s*=" src/ -A 1 -B 1

Repository: appwrite/console

Length of output: 4495


🏁 Script executed:

# Look at package.json to understand dependencies
cat package.json

Repository: appwrite/console

Length of output: 3596


🌐 Web query:

@appwrite.io/pink-svelte Upload Dropzone extensions prop expected format with/without dot

💡 Result:

Short answer: use the leading dot (e.g. ".png", ".jpg"). This follows the HTML file input/accept spec and how Dropzone-style components commonly expect extensions. [1][2]

Sources

  • MDN: input accept attribute — filename extensions must start with a period (e.g. ".jpg"). [1]
  • Example Dropzone docs showing accept object using extensions with leading dots (e.g. ['.png']). [2]

🏁 Script executed:

# Check if there's a node_modules or any documentation
find . -name "*.md" -type f | xargs grep -l "Upload.Dropzone\|extensions" 2>/dev/null | head -5

Repository: appwrite/console

Length of output: 42


🏁 Script executed:

# Let's check the specific code paths where allowedExtension is passed and used
# Look at the storage bucket creation page where extensions is also used
sed -n '1,50p' src/routes/\(console\)/project-[region]-[project]/storage/bucket-[bucket]/create/+page.svelte | head -30

Repository: appwrite/console

Length of output: 1356


🏁 Script executed:

# Search for Upload.Dropzone with extensions in that file
rg "Upload.Dropzone" "src/routes/(console)/project-[region]-[project]/storage/bucket-[bucket]/create/+page.svelte" -A 10 -B 2

Repository: appwrite/console

Length of output: 927


🏁 Script executed:

# Get the full context of how allowedExtension is used through the component
# Check what the line 211 context shows about extension format
sed -n '200,220p' src/lib/components/filePicker.svelte

Repository: appwrite/console

Length of output: 714


Normalize extension format with leading dots and clarify wildcard notation in tooltip

The extensions prop should use leading dots per HTML input standards (e.g., ['.json'] instead of ['json']). Update line 361:

-            extensions={allowedExtension === '*' ? undefined : [allowedExtension]}>
+            extensions={allowedExtension === '*' ? undefined : [`.${allowedExtension}`]}>

Also improve the tooltip clarity at lines 380-382 to explicitly show wildcard notation:

-    {allowedExtension === '*'
-        ? `${mimeTypeQuery} files are allowed`
-        : `${allowedExtension} files are allowed`}
+    {allowedExtension === '*'
+        ? `${mimeTypeQuery}* files are allowed`
+        : `${allowedExtension} files are allowed`}

This aligns with Upload.Dropzone expectations and gives users a clearer view of wildcard filtering (e.g., "image/*").

Also applies to: 380-382

🤖 Prompt for AI Agents
In src/lib/components/filePicker.svelte around lines 359-361 and 380-382, the
Upload.Dropzone extensions prop and its tooltip need updating: normalize
allowedExtension so when it's '*' you pass undefined, otherwise pass an array
with a leading dot (e.g., ['.json']) — implement a small normalization (prepend
'.' if missing) before passing to extensions; and update the tooltip text at
380-382 to explicitly show wildcard notation (for example "image/*" or
"video/*") and explain that '*' will be treated as no restriction, so users see
the exact format expected.

Co-authored-by: Darshan <itznotabug@gmail.com>
@ItzNotABug ItzNotABug merged commit 6c838e3 into main Dec 10, 2025
4 checks passed
@ItzNotABug ItzNotABug deleted the fix-SER-468-file-upload-for-push-notifications branch December 10, 2025 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants