Uh oh!
There was an error while loading. Please reload this page.
feat: per user limit - #922
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR introduces dynamic per-user file upload limits. A new Changes
Sequence DiagramsequenceDiagram
participant Client as Client (Browser)
participant Controller as FileController
participant Service as FileService
participant DB as User Database
Client->>Controller: POST /upload (file)
Controller->>Service: getUserStorageUsage(userId)
Service->>DB: Query user.size
DB-->>Service: user.size (GB)
Service->>Service: Calculate limit (user.size * 1GB in bytes)
Service-->>Controller: limit (bytes)
Controller->>Controller: Validate file ≤ limit
alt File exceeds limit
Controller-->>Client: 413 Error (exceeds size limit)
else File within limit
Controller->>DB: Save file
DB-->>Controller: Success
Controller-->>Client: 200 OK
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platforms/file-manager/api/src/controllers/FileController.ts (1)
169-176:⚠️ Potential issue | 🟡 MinorSame "0GB" formatting issue in batch upload path.
This error message has the same sub-gigabyte formatting problem as the single-file upload. Apply the same
formatByteshelper here for consistency.Proposed fix
- error: `File size exceeds ${Math.round(limit / (1024 * 1024 * 1024))}GB limit`,+ error: `File size exceeds ${this.formatBytes(limit)} limit`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/file-manager/api/src/controllers/FileController.ts` around lines 169 - 176, In the batch-upload branch inside FileController (where file.size > limit triggers errors.push), replace the hardcoded Math.round(limit/(1024*1024*1024))GB message with the existing formatBytes helper so the limit is rendered in human-friendly units; update the error object created in errors.push (referencing file.originalname, file.size and limit) to use formatBytes(limit) for the error string instead of the GB calculation.
🧹 Nitpick comments (2)
platforms/file-manager/client/src/routes/(protected)/storage/+page.svelte (1)
212-213: Clarify "Maximum file size" vs "Total storage quota" distinction.Both lines now display
formatBytes(limit), making them identical. The text "Maximum file size: X per file" and "Total storage quota: X" convey the same value, which may confuse users expecting separate per-file and total limits.If the design intent is that single files can consume the entire quota, consider updating the copy to clarify this:
Suggested copy clarification
- <li>• Maximum file size: {formatBytes(limit)} per file</li>- <li>• Total storage quota: {formatBytes(limit)}</li>+ <li>• Total storage quota: {formatBytes(limit)}</li>+ <li>• Single files may use up to your full quota</li>Alternatively, if there should be a separate per-file limit, the backend would need to return and enforce it distinctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/file-manager/client/src/routes/`(protected)/storage/+page.svelte around lines 212 - 213, The two list items both use formatBytes(limit) which makes "Maximum file size" and "Total storage quota" identical and confusing; update the UI to either (a) display a distinct per-file limit variable (e.g., use perFileLimit instead of limit and call formatBytes(perFileLimit) in the "Maximum file size" <li>) and ensure the backend returns/enforces perFileLimit, or (b) explicitly clarify in the copy that the per-file limit equals the total quota (change the text around the formatBytes(limit) usage to something like "Maximum file size (per file): {formatBytes(limit)} — equals your total quota"). Reference the existing formatBytes function and the limit variable and the two <li> elements in +page.svelte to locate the changes.platforms/file-manager/client/src/routes/(protected)/files/+page.svelte (1)
296-298: Consider showing the actual limit in the error message.The error message now uses generic text "exceeds the size limit" instead of showing the actual value. The server response includes
maxSizewhich could be formatted and displayed for better UX.Optional: Display actual limit from server response
} else { toast.error( - `File "${file.name}" exceeds the size limit`,+ `File "${file.name}" exceeds the ${formatFileSize(errorData.maxSize || 0)} limit`, ); }Note: The
formatFileSizefunction already exists in this file (lines 812-822).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/file-manager/client/src/routes/`(protected)/files/+page.svelte around lines 296 - 298, Update the toast error to include the server-provided maxSize by reading the response's maxSize and formatting it with the existing formatFileSize function; locate the toast.error call that currently says `File "${file.name}" exceeds the size limit` and change it to include `formatFileSize(maxSize)` (or a fallback if maxSize is missing) so the message becomes something like `File "X" exceeds the size limit of Y` using the `formatFileSize` helper already defined in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platforms/file-manager/api/src/controllers/FileController.ts`:
- Around line 49-55: The error message uses Math.round(limit / (1024 * 1024 *
1024)) which yields "0GB" for sub-GB limits; add a helper method (e.g., private
formatBytes(bytes: number): string) to FileController to convert bytes into a
human-readable string (Bytes/KB/MB/GB) with up to two decimals, then replace the
existing interpolation in the upload size check (the block referencing
req.file.size and limit) to use formatBytes(limit) so the response message shows
an appropriate unit and value; ensure the JSON still returns maxSize and
fileSize unchanged.
In `@platforms/file-manager/api/src/services/FileService.ts`:
- Around line 360-366: The current quota calculation uses user.size directly so
a stored value of 0 yields limit=0 and blocks uploads; add validation to enforce
a minimum of 1 on the User entity (e.g., annotate the size property with `@Min`(1)
or add a DB CHECK constraint on the users.size column) and/or add defensive
runtime handling in the FileService where userRepository.findOne is used (check
user?.size and coerce values <=0 to 1 before computing sizeInGB and limit);
update the User entity's size validation and any relevant DB migrations to
ensure the constraint is enforced at persistence and adjust FileService
(symbols: userRepository.findOne, user.size, sizeInGB, limit) to defensively
handle invalid stored values.
---
Outside diff comments:
In `@platforms/file-manager/api/src/controllers/FileController.ts`:
- Around line 169-176: In the batch-upload branch inside FileController (where
file.size > limit triggers errors.push), replace the hardcoded
Math.round(limit/(1024*1024*1024))GB message with the existing formatBytes
helper so the limit is rendered in human-friendly units; update the error object
created in errors.push (referencing file.originalname, file.size and limit) to
use formatBytes(limit) for the error string instead of the GB calculation.
---
Nitpick comments:
In `@platforms/file-manager/client/src/routes/`(protected)/files/+page.svelte:
- Around line 296-298: Update the toast error to include the server-provided
maxSize by reading the response's maxSize and formatting it with the existing
formatFileSize function; locate the toast.error call that currently says `File
"${file.name}" exceeds the size limit` and change it to include
`formatFileSize(maxSize)` (or a fallback if maxSize is missing) so the message
becomes something like `File "X" exceeds the size limit of Y` using the
`formatFileSize` helper already defined in the file.
In `@platforms/file-manager/client/src/routes/`(protected)/storage/+page.svelte:
- Around line 212-213: The two list items both use formatBytes(limit) which
makes "Maximum file size" and "Total storage quota" identical and confusing;
update the UI to either (a) display a distinct per-file limit variable (e.g.,
use perFileLimit instead of limit and call formatBytes(perFileLimit) in the
"Maximum file size" <li>) and ensure the backend returns/enforces perFileLimit,
or (b) explicitly clarify in the copy that the per-file limit equals the total
quota (change the text around the formatBytes(limit) usage to something like
"Maximum file size (per file): {formatBytes(limit)} — equals your total quota").
Reference the existing formatBytes function and the limit variable and the two
<li> elements in +page.svelte to locate the changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 68ac0370-0124-4b07-8b57-f4fdb0a1c716
📒 Files selected for processing (6)
platforms/file-manager/api/src/controllers/FileController.tsplatforms/file-manager/api/src/database/entities/User.tsplatforms/file-manager/api/src/database/migrations/1773609001333-AddUserSize.tsplatforms/file-manager/api/src/services/FileService.tsplatforms/file-manager/client/src/routes/(protected)/files/+page.svelteplatforms/file-manager/client/src/routes/(protected)/storage/+page.svelte
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of change
add capability for per user overrides
Issue Number
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit