fix: improve file size limit feedback with adaptive KB/MB units#312
Conversation
|
|
haiphucnguyen
left a comment
haiphucnguyen
left a comment
There was a problem hiding this comment.
Hi @miguel-baptista07 , thanks for your PR.
I left some minor comments, and you could resolve the conflict of ChatSessionService then this PR is good to go
| @@ -1,4 +1,4 @@ | |||
| #!/bin/sh | |||
| git checkout main#!/bin/sh | |||
| // Return user-friendly message based on exception type | ||
| return when (exception) { | ||
| is FileSizeExceededException -> { | ||
| val fileSizeStr = if (exception.fileSize >= 1_048_576) "${String.format("%.1f", exception.fileSize / 1_048_576.0)} MB" else "${exception.fileSize / 1024} KB" |
There was a problem hiding this comment.
We should have a common method for this logic
val fileSizeStr = if (exception.fileSize >= 1_048_576) "${String.format("%.1f", exception.fileSize / 1_048_576.0)} MB" else "${exception.fileSize / 1024} KB"
val maxSizeStr = if (exception.maxAllowedSize >= 1_048_576) "${String.format("%.1f", exception.maxAllowedSize / 1_048_576.0)} MB" else "${exception.maxAllowedSize / 1024} KB"
"File is too large ($fileSizeStr). Maximum allowed size is $maxSizeStr."
since it is the same with one in the class ChatInputField?
| // Try to get a specific message first | ||
| val specificMessage = when (exception) { | ||
| is FileSizeExceededException -> { | ||
| val fileSizeStr = if (exception.fileSize >= 1_048_576) "${String.format("%.1f", exception.fileSize / 1_048_576.0)} MB" else "${exception.fileSize / 1024} KB" |
There was a problem hiding this comment.
sane like above, a common method convert to proper MB or Kb is helpful
haiphucnguyen
left a comment
haiphucnguyen
left a comment
There was a problem hiding this comment.
Thanks @miguel-baptista07 for your contribution, I left some minor comments.
Also you should resolve the conflict of ChatSessionService, then this PR is good to go
Also could you sign this CLA for once time. Thanks |
done! |
- Fix size display showing 0.0MB for small files by using adaptive units - Show KB when file is smaller than 1MB, MB otherwise - Fix secondary bug dividing by 1000x1000 instead of 1024x1024 - Applied fix in ChatInputField.kt, ErrorHandler.kt and FileContentExtractor.kt Fixes haiphucnguyen#274 Signed-off-by: Miguel <miguelkevlin23@gmail.com>
miguel-baptista07
force-pushed
the
fix/file-size-limit-feedback-274
branch
from
d191cd7 to
47ba914
Compare
|
Hi @haiphucnguyen, addressed all comments:
Ready for review! 🙏 |
|
Thank you @miguel-baptista07 ! Looking forward to your new contributions! |
3 participants
Fixes #274
Problem
The file size error message was always showing "0.0MB" for small files because the calculation always divided by 1024×1024, giving a result less than 1 for small files.
Changes
Before
File 'file_teste.txt' is too large (0,0MB). Maximum allowed size is 0,0MB.
After
File 'file_teste.txt' is too large (4 KB). Maximum allowed size is 1 KB.