✨feat(backend): make DATA_UPLOAD_MAX_MEMORY_SIZE configurable via env var #394
Conversation
Add DATA_UPLOAD_MAX_MEMORY_SIZE to settings.py to allow configuration via environment variable. Django's default is 2.5MB which is too small for emails with larger attachments. Without this setting in settings.py, Django ignores the environment variable and uses its hardcoded default, causing RequestDataTooBig errors when the MTA tries to deliver emails exceeding 2.5MB. Default: 2621440 (2.5MB) Can be overridden via DATA_UPLOAD_MAX_MEMORY_SIZE environment variable.
WalkthroughAdds configurable byte-size limits for uploads, incoming emails, outgoing attachments and outgoing bodies; clamps DATA_UPLOAD_MAX_MEMORY_SIZE accordingly; enforces limits at inbound MTA intake, draft creation/update, and outbound MIME composition; updates API/OpenAPI, frontend config/uploader, translations, and tests. Changes
Sequence Diagram(s)Incoming MTA size check sequenceDiagram
participant Client
participant MTA as Inbound MTA Endpoint
participant Auth
participant Processor
Client->>MTA: POST /inbound/mta/deliver (message/rfc822)
MTA->>Auth: validate JWT
MTA->>MTA: measure raw payload size
alt payload size > MAX_INCOMING_EMAIL_SIZE
MTA-->>Client: 413 Request Entity Too Large (JSON)
Note right of MTA #f9f0c1: log size violation
else payload size ≤ limit
MTA->>Processor: continue delivery processing
Processor-->>MTA: delivery result
MTA-->>Client: 200 OK (delivered count)
end
Outbound/draft size enforcement (high level) sequenceDiagram
participant API as Send/Draft API
participant Settings
participant DraftSvc as Draft service
participant Outbound as Outbound composer/signer
participant Tasks
API->>DraftSvc: create/update draft with attachments/body
DraftSvc->>Settings: read MAX_OUTGOING_BODY_SIZE, MAX_OUTGOING_ATTACHMENT_SIZE
DraftSvc->>DraftSvc: compute body_size and attachments_total
alt body_size > MAX_OUTGOING_BODY_SIZE or attachments_total > MAX_OUTGOING_ATTACHMENT_SIZE
DraftSvc-->>API: 400 ValidationError (localized)
else within limits
DraftSvc-->>API: 200/201 OK
API->>Outbound: prepare_outbound_message()
Outbound->>Outbound: compose MIME + compute total_mime_size
alt total_mime_size attachments/body exceed limits
Outbound-->>API: ValidationError (size exceeded)
else
Outbound->>Tasks: dispatch send task
Tasks-->>API: accepted
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (4)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/backend/messages/settings.py (1)
90-94: Document the purpose, expected values, and monitoring strategy for DATA_UPLOAD_MAX_MEMORY_SIZE.Django's DATA_UPLOAD_MAX_MEMORY_SIZE limits the total request body size before raising RequestDataTooBig, and inbound MTA email delivery sends the entire email (including attachments) as the request body. The current default of 2.5 MB is appropriate only if emails consistently stay below that threshold.
The review comment appropriately flags that this setting affects all HTTP requests globally. Django documentation recommends measuring real request sizes first before increasing limits, then adjusting to 5–20 MB if needed.
To improve the implementation, consider:
- Add inline documentation explaining that this controls inbound email size (particularly with attachments)
- Recommend measuring actual email sizes before choosing a production value
- Note reverse proxy enforcement: Enforce upload size limits at the reverse proxy/load balancer level as well to prevent expensive processing in Django
- Add monitoring: Log and monitor
RequestDataTooBigexceptions to detect when emails exceed the limitExample improvement:
# Upload limits + # Controls maximum request body size (form data + email ingest). + # Inbound emails are delivered as raw RFC822 in request.body; measure typical email sizes with attachments + # before adjusting. Enforce limits at reverse proxy/load balancer level as well. DATA_UPLOAD_MAX_MEMORY_SIZE = values.PositiveIntegerValue( - 2621440, environ_name="DATA_UPLOAD_MAX_MEMORY_SIZE", environ_prefix=None - ) # Default 2.5MB, can be overridden via environment variable + 2621440, # 2.5 MB default; increase to 5–20 MB if emails with attachments exceed this + environ_name="DATA_UPLOAD_MAX_MEMORY_SIZE", + environ_prefix=None, + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/backend/messages/settings.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/backend/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
src/backend/**/*.py: Follow Django/PEP 8 style with a 100-character line limit
Use descriptive, snake_case names for variables and functions
Use Django ORM for database access; avoid raw SQL unless necessary for performance
Use Django’s built-in user model and authentication framework
Prefer try-except blocks to handle exceptions in business logic and views
Log expected and unexpected actions with appropriate log levels
Capture and report exceptions to Sentry; use capture_exception() for custom errors
Do not log sensitive information (tokens, passwords, financial/health data, PII)
Files:
src/backend/messages/settings.py
src/backend/**/{settings.py,middleware.py}
📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
Use middleware judiciously for cross-cutting concerns (authentication, logging, caching)
Files:
src/backend/messages/settings.py
src/backend/**/settings.py
📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
src/backend/**/settings.py: Leverage Django’s caching framework (e.g., Redis/Memcached) where appropriate
Use Django’s cache framework with a backend like Redis or Memcached to reduce DB load
Optimize static file handling using Django’s staticfiles pipeline (e.g., WhiteNoise)
Files:
src/backend/messages/settings.py
⏰ 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). (5)
- GitHub Check: docker-publish-socks-proxy / docker-build-push
- GitHub Check: build-front
- GitHub Check: check-api-state
- GitHub Check: test-back
- GitHub Check: lint-back
|
Good catch and I can confirm we have the issue on our instance too with medium-size incoming attachments. However I think it would be cleaner to have a single setting for the max attachment size (or 2, for incoming + outgoing?) and set that limit in accordance? |
|
Yes, I suggest we are going to use MAX_INCOMING_EMAIL_SIZE and MAX_OUTGOING_EMAIL_SIZE because I suppose we want to use the email body limited as well (gmail, for example, can do 50Mb email, and max 25mb attachment size). On incoming mail, it is quite easy to get the MAX_INCOMING_EMAIL_SIZE and return 413 when the message is too large. For outgoing mail, it is a UX question: I think it is best when creating a draft. But it is hard to determine the message body size (because it is not yet a raw email and updates very often when typing or pasting images). We can easy calculate the total attachment size when uploading, so when an attachment exceeds that total size, we can show a message. When sending the final email, we can check the total size INCLUDING the body, but it can happen your email body is too large then. Do we want to check this also when drafting your email (harder to code) or is a message 'your total message is too large' when you try to send it is good enough? |
Replace generic DATA_UPLOAD_MAX_MEMORY_SIZE with two semantic settings: - MAX_INCOMING_EMAIL_SIZE: Validates total incoming email size (default 10MB) - MAX_OUTGOING_EMAIL_SIZE: Validates total outgoing message size (default 10MB) Validation points: 1. Incoming emails: Validates raw email size at MTA delivery (HTTP 413) 2. Draft attachments: Validates cumulative attachment size when adding files 3. Outbound send: Validates total MIME message size before sending All user-facing errors are translatable using gettext_lazy. Includes comprehensive tests for all validation scenarios. Addresses review feedback from PR suitenumerique#394 to use semantic, unified settings instead of exposing Django's low-level upload configuration.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/backend/core/mda/draft.py (1)
297-327: Validate attachment size correctly with proper exclusion logic.The validation correctly excludes attachments being removed and includes only new attachments, using
select_related("blob")to optimize database queries. The error message is clear and localized.However, consider that this validation checks raw attachment sizes, but the final MIME message will be larger due to:
- Base64 encoding overhead (~33% increase)
- MIME headers, boundaries, and message body
- DKIM signature
The validation in
outbound.py(lines 201-221) catches oversized messages after composition, but users might benefit from a more conservative limit here (e.g., 80-90% of MAX_OUTGOING_EMAIL_SIZE) to provide earlier feedback during draft editing rather than at send time.Consider applying a safety margin:
- if total_size > settings.MAX_OUTGOING_EMAIL_SIZE: + # Reserve ~30% for MIME encoding overhead, headers, body, and signatures + attachment_limit = int(settings.MAX_OUTGOING_EMAIL_SIZE * 0.7) + if total_size > attachment_limit: raise drf.exceptions.ValidationError( { "attachments": _( - "Total attachment size (%(total_size)s bytes) exceeds maximum " - "allowed size of %(max_size)s bytes. Current attachments: " - "%(current_size)s bytes." + "Total attachment size (%(total_size)s bytes) exceeds attachment " + "limit of %(max_size)s bytes (reserved space for message encoding). " + "Current attachments: %(current_size)s bytes." ) % { "total_size": total_size, - "max_size": settings.MAX_OUTGOING_EMAIL_SIZE, + "max_size": attachment_limit, "current_size": current_total_size, } } )src/backend/core/mda/outbound.py (1)
156-161: Remove unused variabletotal_attachment_size.The variable
total_attachment_sizeis initialized and accumulated but never used. The actual size validation at line 202 correctly useslen(raw_mime_signed), which includes all attachments plus MIME encoding overhead.Apply this diff to remove the unused variable:
# Add attachments if present if message.attachments.exists(): attachments = [] - total_attachment_size = 0 for attachment in message.attachments.select_related("blob").all(): # Get the blob data blob = attachment.blob - total_attachment_size += blob.size # Add the attachment to the MIME data
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/backend/core/api/viewsets/inbound/mta.py(1 hunks)src/backend/core/api/viewsets/send.py(1 hunks)src/backend/core/mda/draft.py(4 hunks)src/backend/core/mda/outbound.py(4 hunks)src/backend/core/tests/api/test_attachments.py(1 hunks)src/backend/core/tests/api/test_inbound_mta.py(1 hunks)src/backend/messages/settings.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/backend/messages/settings.py
🧰 Additional context used
📓 Path-based instructions (2)
src/backend/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
src/backend/**/*.py: Follow Django/PEP 8 style with a 100-character line limit
Use descriptive, snake_case names for variables and functions
Use Django ORM for database access; avoid raw SQL unless necessary for performance
Use Django’s built-in user model and authentication framework
Prefer try-except blocks to handle exceptions in business logic and views
Log expected and unexpected actions with appropriate log levels
Capture and report exceptions to Sentry; use capture_exception() for custom errors
Do not log sensitive information (tokens, passwords, financial/health data, PII)
Files:
src/backend/core/tests/api/test_inbound_mta.pysrc/backend/core/api/viewsets/inbound/mta.pysrc/backend/core/api/viewsets/send.pysrc/backend/core/tests/api/test_attachments.pysrc/backend/core/mda/outbound.pysrc/backend/core/mda/draft.py
src/backend/**/{tests.py,tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
src/backend/**/{tests.py,tests/**/*.py}: Use Django’s testing tools (pytest-django) to ensure code quality and reliability
Unit tests should focus on a single use case, keep assertions minimal, and cover all possible cases
Files:
src/backend/core/tests/api/test_inbound_mta.pysrc/backend/core/tests/api/test_attachments.py
🧠 Learnings (2)
📚 Learning: 2025-09-02T10:12:12.835Z
Learnt from: CR
Repo: suitenumerique/messages PR: 0
File: .cursor/rules/django-python.mdc:0-0
Timestamp: 2025-09-02T10:12:12.835Z
Learning: Applies to src/backend/**/settings.py : Use Django’s cache framework with a backend like Redis or Memcached to reduce DB load
Applied to files:
src/backend/core/mda/outbound.py
📚 Learning: 2025-09-02T10:12:12.835Z
Learnt from: CR
Repo: suitenumerique/messages PR: 0
File: .cursor/rules/django-python.mdc:0-0
Timestamp: 2025-09-02T10:12:12.835Z
Learning: Applies to src/backend/**/settings.py : Leverage Django’s caching framework (e.g., Redis/Memcached) where appropriate
Applied to files:
src/backend/core/mda/outbound.py
🧬 Code graph analysis (5)
src/backend/core/tests/api/test_inbound_mta.py (2)
src/backend/core/tests/api/test_attachments.py (2)
api_client(25-30)api_client(140-145)src/backend/core/factories.py (1)
MailboxFactory(60-96)
src/backend/core/api/viewsets/send.py (1)
src/backend/core/mda/outbound.py (1)
prepare_outbound_message(45-256)
src/backend/core/tests/api/test_attachments.py (4)
src/backend/core/tests/api/test_labels.py (2)
api_client(34-38)mailbox(26-30)src/backend/core/models.py (4)
create_blob(548-574)create_blob(1358-1422)content_type(1562-1564)Attachment(1523-1574)src/backend/core/api/viewsets/send.py (1)
post(77-129)src/backend/core/api/viewsets/draft.py (2)
post(197-231)put(234-281)
src/backend/core/mda/outbound.py (3)
src/backend/core/tests/api/test_attachments.py (2)
attachment(169-173)blob(160-166)src/backend/core/factories.py (1)
blob(233-236)src/backend/core/models.py (1)
size(1567-1569)
src/backend/core/mda/draft.py (1)
src/backend/core/models.py (3)
Contact(1126-1149)size(1567-1569)Attachment(1523-1574)
🪛 Ruff (0.14.2)
src/backend/core/tests/api/test_attachments.py
342-342: import should be at the top-level of a file
(PLC0415)
381-381: import should be at the top-level of a file
(PLC0415)
453-453: import should be at the top-level of a file
(PLC0415)
505-505: import should be at the top-level of a file
(PLC0415)
574-574: import should be at the top-level of a file
(PLC0415)
⏰ 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). (5)
- GitHub Check: docker-publish-socks-proxy / docker-build-push
- GitHub Check: build-front
- GitHub Check: check-api-state
- GitHub Check: test-back
- GitHub Check: lint-back
🔇 Additional comments (14)
src/backend/core/mda/draft.py (2)
7-9: LGTM! Clean import additions for settings and i18n.These imports are properly used later in the validation logic.
53-53: LGTM! Consistent unpacking of get_or_create results.The change from
_to_createdimproves code readability, even though the created flag isn't used downstream.Also applies to: 200-200
src/backend/core/api/viewsets/inbound/mta.py (1)
142-155: LGTM! Early size validation with appropriate HTTP status.The validation correctly rejects oversized incoming emails with HTTP 413 before any parsing occurs, which is efficient. The error message is clear and includes the size limit. The logging provides good debugging information with both actual and limit values.
src/backend/core/api/viewsets/send.py (1)
109-116: LGTM! Clean exception-driven flow for outbound preparation.The change to rely on
prepare_outbound_messageraisingValidationError(instead of checking a return value) is more idiomatic for DRF. The comment clearly documents this behavior. Exceptions will be properly handled by DRF's exception handler and converted to appropriate HTTP responses.src/backend/core/mda/outbound.py (3)
10-12: LGTM! Standard imports for i18n and DRF exceptions.Both imports are properly utilized in the exception handling logic below.
187-189: LGTM! Proper exception handling for MIME composition failure.The change to raise
drf.exceptions.APIExceptioninstead of returningFalseis consistent with the new exception-driven flow. The error is logged for debugging and the user-facing message is clear and localized.
201-221: LGTM! Comprehensive size validation after DKIM signing.The validation correctly checks the final message size after DKIM signing, which captures the true size including all MIME encoding overhead, attachments, headers, and the signature. The error message is clear, localized, and includes both actual and limit sizes.
Note that this validation uses
{"message": ...}as the error key, while the draft validation (lines 314-327 in draft.py) uses{"attachments": ...}. This difference appears intentional: draft validation is field-specific, while this is a message-level validation at send time. Both patterns are valid in DRF.src/backend/core/tests/api/test_attachments.py (5)
340-378: LGTM! Clear test for attachment size limit enforcement.The test correctly validates that creating a draft with an attachment exceeding
MAX_OUTGOING_EMAIL_SIZEreturns a 400 error with the "attachments" key in the response.Regarding the static analysis hint about
importat the top level: importingoverride_settingsinside the test method is a common pattern in Django tests for clarity and test isolation. This is acceptable despite the linter warning.
379-449: LGTM! Comprehensive test for cumulative size validation.The test correctly validates that adding a second attachment that would push the total size over the limit (1 KB + 1.5 KB = 2.5 KB > 2 KB limit) is rejected with a 400 error. The test properly exercises the update flow by including both attachments in the PUT request.
451-499: LGTM! Good coverage of the success path.The test validates that attachments within the size limit (8 KB < 10 KB) are accepted and correctly stored. This complements the error case tests and ensures the validation doesn't incorrectly reject valid requests.
501-568: LGTM! Important edge case for attachment replacement.This test validates that the size validation correctly excludes attachments being removed when calculating the total. Replacing a 1.5 KB attachment with another 1.5 KB attachment should succeed even though each is close to the 2 KB limit. This aligns with the validation logic in draft.py (lines 300-303) that uses
exclude(id__in=to_remove).
570-627: LGTM! Critical test for send-time size validation.This test validates the defense-in-depth approach by bypassing draft validation and testing the send-time validation in
outbound.py. This is important because:
- Drafts might have been created before the limit was introduced
- The limit might have been changed to a lower value
- There could be bugs in draft validation
The test correctly expects a 400 error with the "message" key in the response, matching the validation in outbound.py (line 212).
src/backend/core/tests/api/test_inbound_mta.py (2)
950-978: LGTM! Validates inbound size limit enforcement.The test correctly validates that incoming emails exceeding
MAX_INCOMING_EMAIL_SIZEare rejected with HTTP 413 and an appropriate error message. The test properly uses a valid JWT token and correct Content-Type to isolate the size validation behavior.
980-1009: LGTM! Good coverage of the success path for inbound size validation.The test validates that incoming emails within the size limit (5 KB < 10 KB) are accepted and delivered successfully. The test properly creates a mailbox to ensure delivery succeeds after passing size validation. This complements the error case test and ensures the validation doesn't incorrectly reject valid emails.
- Replace MAX_OUTGOING_EMAIL_SIZE with MAX_OUTGOING_ATTACHMENT_SIZE and MAX_OUTGOING_BODY_SIZE - Add validation for body size (5MB default) and attachment size (20MB default) separately - Update DATA_UPLOAD_MAX_MEMORY_SIZE calculation to accommodate both limits together - Validate raw content sizes (before MIME encoding) to ignore implementation overhead - Add body size validation in draft updates and send operations - Expose both limits via config API for frontend - Update frontend to use MAX_OUTGOING_ATTACHMENT_SIZE from config - Add frontend cumulative attachment size validation - Update error messages to show sizes in MB instead of bytes - Update tests to use new MAX_OUTGOING_ATTACHMENT_SIZE setting Validation points: - Draft: Body size and cumulative attachment size - Send: Body size and cumulative attachment size (before MIME encoding) - Incoming: Total email size Resolves feedback on PR suitenumerique#394
…ions - Replace Field error state with toast.error() for cleaner UX - Use AttachmentHelper.getFormattedSize() for consistent size formatting - Keep attachment area clean (no red state) when files exceed limit - Add translation string for attachment size error message
There was a problem hiding this comment.
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/backend/core/mda/outbound.py (1)
184-247: Critical breaking change: selfcheck.py will crash when exceptions are raised instead of returning False.The function signature still declares
-> bool, but the implementation now raisesAPIExceptioninstead of returningFalse. This breaks the existing calling pattern insrc/backend/core/mda/selfcheck.py:77, which expects a boolean return:if not prepare_outbound_message(from_mailbox, message, text_body, html_body): raise SelfCheckError("Failed to prepare outbound message")This code is not in a DRF view context and has no exception handler. When an
APIExceptionis raised, it will propagate unhandled and crash the selfcheck flow.While
src/backend/core/api/viewsets/send.py:110is prepared to handle exceptions (as indicated by the comment),selfcheck.pyis not. The function cannot have two conflicting behaviors.Fix required: Either (1) keep the
-> boolreturn pattern across all callers, or (2) updateselfcheck.py:77to catch the exceptions instead of checking return values, and update the function signature to reflect the change.
🧹 Nitpick comments (2)
src/backend/core/api/openapi.json (2)
201-210: Document scope and add basic constraints for new size settings.Clarify whether MAX_OUTGOING_ATTACHMENT_SIZE is per‑attachment or total attachments, and ensure both have a non‑negative constraint.
Apply minimal schema polish:
"MAX_OUTGOING_ATTACHMENT_SIZE": { - "type": "integer", - "description": "Maximum size in bytes for outgoing email attachments", + "type": "integer", + "minimum": 0, + "description": "Maximum size in bytes for outgoing email attachments (bytes, PER ATTACHMENT or TOTAL — clarify)", "readOnly": true }, "MAX_OUTGOING_BODY_SIZE": { - "type": "integer", - "description": "Maximum size in bytes for outgoing email body (text + HTML)", + "type": "integer", + "minimum": 0, + "description": "Maximum size in bytes for outgoing email body (text + HTML, bytes)", "readOnly": true }
2399-2449: Image proxy endpoint: add formats and binary response schema; note SSRF considerations.
- mailbox_id should be UUID formatted; url should use URI format.
- Define 200 response content as image binary (image/*).
- Optionally document allowed URL schemes (http/https) and max fetched size trigger for 413.
Apply schema refinements:
{ "in": "path", "name": "mailbox_id", "schema": { - "type": "string" + "type": "string", + "format": "uuid" }, "description": "ID of the mailbox", "required": true }, { "in": "query", "name": "url", "schema": { - "type": "string" + "type": "string", + "format": "uri" }, - "description": "The external image URL to proxy", + "description": "The external image URL to proxy (http/https)", "required": true } ... - "responses": { - "200": { - "description": "Image content" - }, + "responses": { + "200": { + "description": "Image content", + "content": { + "image/*": { + "schema": { "type": "string", "format": "binary" } + } + } + }, "400": { "description": "Invalid request" }, "403": { "description": "Forbidden" }, "413": { "description": "Image too large" }, "502": { "description": "Failed to fetch external image" } }Please also confirm the implementation enforces:
- Allowed schemes (http/https only), size/time limits, blocked private IP ranges (SSRF), and DNS re‑resolution on connect. If desired, I can add a short “Security notes” paragraph to the endpoint description.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
src/frontend/package-lock.jsonis excluded by!**/package-lock.jsonsrc/frontend/src/features/api/gen/mailboxes/mailboxes.tsis excluded by!**/gen/**src/frontend/src/features/api/gen/models/config_retrieve200.tsis excluded by!**/gen/**src/frontend/src/features/api/gen/models/index.tsis excluded by!**/gen/**src/frontend/src/features/api/gen/models/mailboxes_image_proxy_list_params.tsis excluded by!**/gen/**
📒 Files selected for processing (9)
src/backend/core/api/openapi.json(3 hunks)src/backend/core/api/viewsets/config.py(3 hunks)src/backend/core/mda/draft.py(5 hunks)src/backend/core/mda/outbound.py(4 hunks)src/backend/core/tests/api/test_attachments.py(1 hunks)src/backend/messages/settings.py(2 hunks)src/frontend/public/locales/common/en-US.json(1 hunks)src/frontend/src/features/forms/components/message-form/attachment-uploader.tsx(4 hunks)src/frontend/src/features/providers/config.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/backend/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
src/backend/**/*.py: Follow Django/PEP 8 style with a 100-character line limit
Use descriptive, snake_case names for variables and functions
Use Django ORM for database access; avoid raw SQL unless necessary for performance
Use Django’s built-in user model and authentication framework
Prefer try-except blocks to handle exceptions in business logic and views
Log expected and unexpected actions with appropriate log levels
Capture and report exceptions to Sentry; use capture_exception() for custom errors
Do not log sensitive information (tokens, passwords, financial/health data, PII)
Files:
src/backend/core/api/viewsets/config.pysrc/backend/core/mda/draft.pysrc/backend/messages/settings.pysrc/backend/core/mda/outbound.pysrc/backend/core/tests/api/test_attachments.py
src/backend/**/{settings.py,middleware.py}
📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
Use middleware judiciously for cross-cutting concerns (authentication, logging, caching)
Files:
src/backend/messages/settings.py
src/backend/**/settings.py
📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
src/backend/**/settings.py: Leverage Django’s caching framework (e.g., Redis/Memcached) where appropriate
Use Django’s cache framework with a backend like Redis or Memcached to reduce DB load
Optimize static file handling using Django’s staticfiles pipeline (e.g., WhiteNoise)
Files:
src/backend/messages/settings.py
src/backend/**/{tests.py,tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
src/backend/**/{tests.py,tests/**/*.py}: Use Django’s testing tools (pytest-django) to ensure code quality and reliability
Unit tests should focus on a single use case, keep assertions minimal, and cover all possible cases
Files:
src/backend/core/tests/api/test_attachments.py
🧠 Learnings (3)
📚 Learning: 2025-09-02T12:20:22.006Z
Learnt from: Nastaliss
Repo: suitenumerique/messages PR: 313
File: src/frontend/src/features/layouts/components/admin/modal-create-domain/index.tsx:32-37
Timestamp: 2025-09-02T12:20:22.006Z
Learning: In the messages application, `SCHEMA_CUSTOM_ATTRIBUTES_MAILDOMAIN` from the config defaults to an empty object `{}` rather than being potentially undefined, so null checks are not needed when using this configuration value.
Applied to files:
src/backend/core/api/openapi.jsonsrc/frontend/src/features/providers/config.tsx
📚 Learning: 2025-09-02T10:12:12.835Z
Learnt from: CR
Repo: suitenumerique/messages PR: 0
File: .cursor/rules/django-python.mdc:0-0
Timestamp: 2025-09-02T10:12:12.835Z
Learning: Applies to src/backend/**/settings.py : Use Django’s cache framework with a backend like Redis or Memcached to reduce DB load
Applied to files:
src/backend/core/mda/outbound.py
📚 Learning: 2025-09-02T10:12:12.835Z
Learnt from: CR
Repo: suitenumerique/messages PR: 0
File: .cursor/rules/django-python.mdc:0-0
Timestamp: 2025-09-02T10:12:12.835Z
Learning: Applies to src/backend/**/settings.py : Leverage Django’s caching framework (e.g., Redis/Memcached) where appropriate
Applied to files:
src/backend/core/mda/outbound.py
🧬 Code graph analysis (4)
src/frontend/src/features/forms/components/message-form/attachment-uploader.tsx (4)
src/frontend/src/features/providers/config.tsx (1)
useConfig(50-56)src/frontend/src/features/forms/components/message-form/drive-attachment-picker.tsx (1)
DriveFile(11-11)src/frontend/src/hooks/use-debounce-callback.ts (1)
useDebounceCallback(7-23)src/frontend/src/features/utils/attachment-helper/index.ts (1)
AttachmentHelper(11-84)
src/backend/core/mda/draft.py (3)
src/backend/core/models.py (5)
Contact(1126-1149)create_blob(548-574)create_blob(1358-1422)size(1567-1569)Attachment(1523-1574)src/backend/core/tests/api/test_attachments.py (1)
blob(160-166)src/backend/core/factories.py (1)
blob(233-236)
src/backend/core/mda/outbound.py (2)
src/backend/core/models.py (3)
text_body(1802-1810)html_body(1791-1799)size(1567-1569)src/backend/core/factories.py (1)
blob(233-236)
src/backend/core/tests/api/test_attachments.py (5)
src/backend/core/tests/api/test_labels.py (2)
api_client(34-38)mailbox(26-30)src/backend/core/factories.py (5)
blob(233-236)ThreadFactory(125-132)ThreadAccessFactory(135-145)ContactFactory(148-157)MessageFactory(160-186)src/backend/core/models.py (4)
create_blob(548-574)create_blob(1358-1422)content_type(1562-1564)Attachment(1523-1574)src/backend/core/api/viewsets/send.py (1)
post(77-129)src/backend/core/api/viewsets/draft.py (2)
post(197-231)put(234-281)
🪛 Ruff (0.14.2)
src/backend/core/tests/api/test_attachments.py
342-342: import should be at the top-level of a file
(PLC0415)
381-381: import should be at the top-level of a file
(PLC0415)
453-453: import should be at the top-level of a file
(PLC0415)
505-505: import should be at the top-level of a file
(PLC0415)
574-574: import should be at the top-level of a file
(PLC0415)
⏰ 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). (1)
- GitHub Check: test-back
🔇 Additional comments (21)
src/frontend/public/locales/common/en-US.json (1)
55-55: LGTM!The translation string is well-structured with clear placeholder names that align with the frontend usage in
attachment-uploader.tsx. The message provides comprehensive information about the size limit violation.src/backend/core/mda/outbound.py (2)
10-12: LGTM!The new imports support localization and DRF exception handling, which are necessary for the size validation features.
153-182: Body size validation approach is sound.The validation correctly calculates raw UTF-8 byte size before MIME encoding, which is consistent with the intent to limit email body size. The error message is localized and provides clear feedback with sizes in MB.
src/frontend/src/features/providers/config.tsx (1)
14-15: LGTM!The default values align with the backend settings (20MB for attachments, 5MB for body). These defaults provide graceful degradation if the backend config endpoint fails or hasn't loaded yet.
src/backend/core/mda/draft.py (4)
7-9: LGTM!The new imports support the size validation and localization features.
53-53: Better variable naming.Changing the unpacking from
_to_createdimproves code clarity by explicitly showing that the second value is the boolean "created" flag fromget_or_create().Also applies to: 200-200
225-243: Body size validation is correctly implemented.The validation occurs before blob creation, uses consistent UTF-8 encoding for size calculation, and provides clear localized error messages.
317-354: Comprehensive attachment size validation.The validation logic correctly handles the attachment replacement scenario by excluding attachments being removed from the current total. The error message provides all relevant size information (current, new, total, limit) in a user-friendly format.
src/frontend/src/features/forms/components/message-form/attachment-uploader.tsx (3)
5-5: LGTM!The component correctly uses the config provider to retrieve the dynamic size limit, replacing the previous hardcoded value.
Also applies to: 17-17, 33-34
41-60: Client-side validation improves UX.The cumulative size validation prevents uploads before they occur, providing immediate feedback to users. The error message includes all relevant size information formatted using the localized translation string.
151-157: Simplified field state management.The component now shows a consistent informational message about the size limit instead of conditional error states, providing clearer guidance to users.
src/backend/core/api/viewsets/config.py (2)
65-74: OpenAPI schema properly updated.The schema correctly documents the new size limit fields as read-only integers with clear descriptions and marks them as required in the response.
Also applies to: 85-86
115-117: LGTM!The response handler correctly exposes the size limit settings from the Django configuration.
src/backend/core/tests/api/test_attachments.py (6)
340-378: Test coverage for single large attachment.The test correctly verifies that a single oversized attachment is rejected during draft creation with a 400 error containing
attachmentsin the response.
379-449: Test coverage for cumulative size limit.The test validates that adding multiple attachments respects the cumulative size limit by first creating a draft with one attachment, then attempting to add another that would exceed the total limit.
451-499: Test coverage for valid attachments.The test confirms that multiple attachments within the limit are accepted successfully.
501-568: Test coverage for attachment replacement.The test verifies that removing an attachment allows adding a new one within the limit, confirming the replacement logic works correctly.
570-627: Test coverage for send-time validation.The test confirms that size limits are enforced during the send operation, not just during draft creation, by manually adding an oversized attachment and attempting to send.
342-342: Static analysis hint can be safely ignored.The
importstatements inside test methods are intentional and necessary when usingoverride_settingsas a context manager. Moving these imports to the top level would break the test isolation.Also applies to: 381-381, 453-453, 505-505, 574-574
src/backend/messages/settings.py (1)
90-112: Well-structured size limit settings.The new settings provide comprehensive control over email size limits with reasonable defaults:
- DATA_UPLOAD_MAX_MEMORY_SIZE: 2.5 MB (Django default)
- MAX_INCOMING_EMAIL_SIZE: 10 MB
- MAX_OUTGOING_ATTACHMENT_SIZE: 20 MB
- MAX_OUTGOING_BODY_SIZE: 5 MB
All settings are configurable via environment variables, addressing the core issue described in the PR objectives.
src/backend/core/api/openapi.json (1)
220-223: Review comment is incorrect; fields are already properly integrated.The backend already defines
MAX_OUTGOING_ATTACHMENT_SIZEandMAX_OUTGOING_BODY_SIZEwith defaults (20MB and 5MB respectively), and the config endpoint explicitly returns these fields. These are not new undeployed fields—they're active parts of the system used for email validation. Marking them as required poses no deployment risk; the proposed diff should be ignored.Likely an incorrect or invalid review comment.
| # Ensure Django's upload limit accommodates the larger of the email size limits | ||
| # For outgoing, we need to accommodate both body + attachments together | ||
| max_outgoing_total = self.MAX_OUTGOING_ATTACHMENT_SIZE + self.MAX_OUTGOING_BODY_SIZE | ||
| self.DATA_UPLOAD_MAX_MEMORY_SIZE = max( | ||
| self.DATA_UPLOAD_MAX_MEMORY_SIZE, | ||
| self.MAX_INCOMING_EMAIL_SIZE, | ||
| max_outgoing_total, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify DATA_UPLOAD_MAX_MEMORY_SIZE computation logic.
The computation sets Django's upload limit to the maximum of:
- Configured DATA_UPLOAD_MAX_MEMORY_SIZE
- MAX_INCOMING_EMAIL_SIZE
- MAX_OUTGOING_ATTACHMENT_SIZE + MAX_OUTGOING_BODY_SIZE
This ensures Django accepts the largest possible upload scenario. However, consider whether the outgoing total (body + attachments) is the right comparison, since:
- Incoming emails come as a single payload
- Outgoing drafts may be updated incrementally (body, then attachments)
The logic appears sound for the MTA intake use case (where the entire email arrives as one payload), but verify it doesn't inadvertently allow uploads larger than intended in other scenarios.
🏁 Script executed:
#!/bin/bash
# Description: Check how DATA_UPLOAD_MAX_MEMORY_SIZE is used in request handling
# Find references to DATA_UPLOAD_MAX_MEMORY_SIZE
rg -n 'DATA_UPLOAD_MAX_MEMORY_SIZE' --type=py -A3 -B3
# Find request parsing and upload handling
rg -n 'RequestDataTooBig|request\.body|request\.FILES' --type=py -C2Length of output: 4530
---
🏁 Script executed:
```shell
#!/bin/bash
# Find outgoing email/draft creation and handling
rg -n 'MAX_OUTGOING_BODY_SIZE|MAX_OUTGOING_ATTACHMENT_SIZE' --type=py -B5 -A5
# Find endpoints/views for creating/uploading outgoing emails
rg -n 'outgoing|draft|compose' --type=py -i | grep -i 'def \|class ' | head -20
# Check if multipart/form-data is used for draft/outgoing handling
rg -n 'MultiPartParser|FormParser|multipart' --type=py -B2 -A2
Length of output: 76111
🏁 Script executed:
#!/bin/bash
# Same as above but runs without repository
echo "Need repository context - cannot skip cloning for this search"Length of output: 61
🏁 Script executed:
#!/bin/bash
# Find DraftMessageView create/update implementations
rg -n 'class DraftMessageView' --type=py -A30
# Check how draft requests are structured - look for serializer fields
rg -n 'class.*DraftSerializer\|class.*MessageSerializer' --type=py -A10 | head -60
# Find what parsers are used by draft endpoints
rg -n 'parser_classes.*=|DraftMessageView|draft.*endpoint' --type=py -B2 -A2 | grep -A5 -B5 'parser_classes'Length of output: 4184
Reconsider the computation of outgoing body + attachments sum.
The code sets DATA_UPLOAD_MAX_MEMORY_SIZE = max(current, MAX_INCOMING_EMAIL_SIZE, MAX_OUTGOING_ATTACHMENT_SIZE + MAX_OUTGOING_BODY_SIZE), but the architecture never sends body and attachments together in a single HTTP request:
- Draft creation/update:
draftBodycomes as JSON text +attachmentIdsare UUID references (not binary attachment data). - Blob uploads: Handled separately via the blob endpoint with
MultiPartParser. - Inbound MTA: Full RFC822 in
request.body(single payload, already covered byMAX_INCOMING_EMAIL_SIZE).
Since outgoing email body and attachments are uploaded separately (body in draft JSON, attachments pre-uploaded as blobs), the sum is unnecessary. Consider using max(MAX_OUTGOING_BODY_SIZE, MAX_OUTGOING_ATTACHMENT_SIZE) instead, or remove the outgoing sum entirely if MAX_INCOMING_EMAIL_SIZE already covers all scenarios.
🤖 Prompt for AI Agents
In src/backend/messages/settings.py around lines 772 to 779, the code sums
MAX_OUTGOING_ATTACHMENT_SIZE + MAX_OUTGOING_BODY_SIZE into
DATA_UPLOAD_MAX_MEMORY_SIZE even though body and attachments are never uploaded
together; change the logic to not add them together — either use
max(MAX_OUTGOING_BODY_SIZE, MAX_OUTGOING_ATTACHMENT_SIZE) or drop the outgoing
sizes entirely if MAX_INCOMING_EMAIL_SIZE already covers all cases — update the
DATA_UPLOAD_MAX_MEMORY_SIZE assignment accordingly to use the chosen alternative
and remove the unnecessary sum.
There was a problem hiding this comment.
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/frontend/src/features/forms/components/message-form/attachment-uploader.tsx (1)
144-146: Enforce the limit for drive attachments too.Drive picks skip the new total-size validation, so users can exceed
MAX_OUTGOING_ATTACHMENT_SIZEby pulling files from Drive. Mirror the drop handler’s guard (with the same toast) before callingappendToAttachments.Apply this diff:
- const handleDriveAttachmentPick = (attachments: DriveFile[]) => { - appendToAttachments(attachments); - } + const handleDriveAttachmentPick = (newAttachments: DriveFile[]) => { + const newAttachmentsSize = newAttachments.reduce((acc, attachment) => acc + attachment.size, 0); + const newTotalSize = currentTotalSize + newAttachmentsSize; + + if (newTotalSize > MAX_ATTACHMENT_SIZE) { + toast.error( + t("Cannot add attachment(s) ({{newSize}}). Total would be {{totalSize}}, exceeding the {{maxSize}} limit. Current attachments: {{currentSize}}.", { + newSize: AttachmentHelper.getFormattedSize(newAttachmentsSize, i18n.language), + totalSize: AttachmentHelper.getFormattedSize(newTotalSize, i18n.language), + maxSize: AttachmentHelper.getFormattedSize(MAX_ATTACHMENT_SIZE, i18n.language), + currentSize: AttachmentHelper.getFormattedSize(currentTotalSize, i18n.language), + }) + ); + return; + } + + appendToAttachments(newAttachments); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/frontend/src/features/forms/components/message-form/attachment-uploader.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/frontend/src/features/forms/components/message-form/attachment-uploader.tsx (4)
src/frontend/src/features/providers/config.tsx (1)
useConfig(50-56)src/frontend/src/features/forms/components/message-form/drive-attachment-picker.tsx (1)
DriveFile(11-11)src/frontend/src/hooks/use-debounce-callback.ts (1)
useDebounceCallback(7-23)src/frontend/src/features/utils/attachment-helper/index.ts (1)
AttachmentHelper(11-84)
| // Calculate current total size of attachments | ||
| const currentTotalSize = attachments.reduce((acc, attachment) => acc + attachment.size, 0); | ||
|
|
||
| const { getRootProps, getInputProps, isDragActive, fileRejections } = useDropzone({ | ||
| onDrop: async (acceptedFiles) => { | ||
| // Check cumulative size before uploading | ||
| const newFilesSize = acceptedFiles.reduce((acc, file) => acc + file.size, 0); | ||
| const totalSize = currentTotalSize + newFilesSize; | ||
|
|
||
| if (totalSize > MAX_ATTACHMENT_SIZE) { | ||
| toast.error( | ||
| t("Cannot add attachment(s) ({{newSize}}). Total would be {{totalSize}}, exceeding the {{maxSize}} limit. Current attachments: {{currentSize}}.", { | ||
| newSize: AttachmentHelper.getFormattedSize(newFilesSize, i18n.language), | ||
| totalSize: AttachmentHelper.getFormattedSize(totalSize, i18n.language), | ||
| maxSize: AttachmentHelper.getFormattedSize(MAX_ATTACHMENT_SIZE, i18n.language), | ||
| currentSize: AttachmentHelper.getFormattedSize(currentTotalSize, i18n.language) | ||
| }) | ||
| ); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Include pending uploads in the size guard.
The cumulative limit only sums attachments, so users can enqueue multiple batches while uploads are pending and blow past MAX_OUTGOING_ATTACHMENT_SIZE once everything finishes uploading. Please fold the uploadingQueue sizes into the running total before accepting more files.
Apply this diff:
- const currentTotalSize = attachments.reduce((acc, attachment) => acc + attachment.size, 0);
+ const attachmentsSize = attachments.reduce((acc, attachment) => acc + attachment.size, 0);
+ const uploadingQueueSize = uploadingQueue.reduce((acc, file) => acc + file.size, 0);
+ const currentTotalSize = attachmentsSize + uploadingQueueSize;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/frontend/src/features/forms/components/message-form/attachment-uploader.tsx
around lines 41 to 60, the size check only sums existing attachments and omits
pending uploads, allowing users to enqueue more files and exceed
MAX_ATTACHMENT_SIZE; update the total-size calculation to include sizes of files
in uploadingQueue (sum uploadingQueue.map(q=>q.file.size) or equivalent) when
computing totalSize and currentTotalSize used in the guard, then use that
combined total in the conditional and in the error message formatting so pending
uploads are counted before accepting additional files.
- Replace Intl.NumberFormat with binary (MiB) formatter for consistent sizes - Shows 5MB, 10MB, 20MB instead of 5.2MB, 10.5MB, 20.97MB - Fixes locale-specific 'mln. byte' text issue - Use ToasterItem instead of toast.error for attachment size errors - Matches file system conventions users expect
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/frontend/src/features/forms/components/message-form/attachment-uploader.tsx (1)
41-62: Count in-flight uploads in the size guard.
currentTotalSizeignoresuploadingQueue, so users can stage multiple batches while earlier files are uploading and push the eventual total pastMAX_OUTGOING_ATTACHMENT_SIZE. The toast also under-reports the current total in that situation.- const currentTotalSize = attachments.reduce((acc, attachment) => acc + attachment.size, 0); + const attachmentsSize = attachments.reduce((acc, attachment) => acc + attachment.size, 0); + const uploadingQueueSize = uploadingQueue.reduce((acc, file) => acc + file.size, 0); + const currentTotalSize = attachmentsSize + uploadingQueueSize;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/frontend/src/features/forms/components/message-form/attachment-uploader.tsx(4 hunks)src/frontend/src/features/utils/attachment-helper/index.test.ts(1 hunks)src/frontend/src/features/utils/attachment-helper/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/frontend/src/features/utils/attachment-helper/index.ts (1)
src/backend/core/models.py (1)
size(1567-1569)
src/frontend/src/features/utils/attachment-helper/index.test.ts (4)
src/frontend/src/features/utils/attachment-helper/index.ts (1)
AttachmentHelper(11-91)src/frontend/src/features/api/utils.ts (1)
getRequestUrl(39-50)src/frontend/src/features/api/gen/blob/blob.ts (1)
getBlobDownloadRetrieveUrl(49-51)src/frontend/src/features/forms/components/message-form/drive-attachment-picker.tsx (1)
DriveFile(11-11)
src/frontend/src/features/forms/components/message-form/attachment-uploader.tsx (6)
src/frontend/src/features/providers/config.tsx (1)
useConfig(50-56)src/frontend/src/features/forms/components/message-form/drive-attachment-picker.tsx (1)
DriveFile(11-11)src/frontend/src/features/api/gen/blob/blob.ts (1)
useBlobUploadCreate(304-324)src/frontend/src/hooks/use-debounce-callback.ts (1)
useDebounceCallback(7-23)src/frontend/src/features/ui/components/toaster/index.tsx (2)
addToast(63-76)ToasterItem(16-61)src/frontend/src/features/utils/attachment-helper/index.ts (1)
AttachmentHelper(11-91)
⏰ 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). (4)
- GitHub Check: docker-publish-socks-proxy / docker-build-push
- GitHub Check: check-api-state
- GitHub Check: test-back
- GitHub Check: lint-back
| // Use binary (1024) conversion for user-friendly display | ||
| // This matches file system conventions and gives clean numbers | ||
| const MB = 1024 * 1024; | ||
| const KB = 1024; | ||
|
|
||
| return formatter.format(size); | ||
| if (size >= MB) { | ||
| const mb = size / MB; | ||
| // Show whole number if it's exact, otherwise 1 decimal | ||
| return mb % 1 === 0 ? `${mb} MB` : `${mb.toFixed(1)} MB`; | ||
| } else if (size >= KB) { | ||
| const kb = size / KB; | ||
| return kb % 1 === 0 ? `${kb} KB` : `${kb.toFixed(1)} KB`; | ||
| } else { | ||
| return `${size} B`; | ||
| } |
There was a problem hiding this comment.
Restore locale-aware size formatting.
By dropping the Intl formatter we now ignore the language argument and always emit English-style decimals (e.g. 1.5 MB in French instead of 1,5 MB). That is a regression for localized UIs. Please keep the binary threshold logic but format the numeric part through Intl.NumberFormat so separators and decimal symbols follow the active locale.
static getFormattedSize(size: number, language: string = 'en') {
- const MB = 1024 * 1024;
- const KB = 1024;
-
- if (size >= MB) {
- const mb = size / MB;
- return mb % 1 === 0 ? `${mb} MB` : `${mb.toFixed(1)} MB`;
- } else if (size >= KB) {
- const kb = size / KB;
- return kb % 1 === 0 ? `${kb} KB` : `${kb.toFixed(1)} KB`;
- } else {
- return `${size} B`;
- }
+ const MB = 1024 * 1024;
+ const KB = 1024;
+ const formatNumber = (value: number) => {
+ return new Intl.NumberFormat(language, {
+ minimumFractionDigits: Number.isInteger(value) ? 0 : 1,
+ maximumFractionDigits: 1,
+ }).format(Number.isInteger(value) ? value : Number(value.toFixed(1)));
+ };
+
+ if (size >= MB) {
+ return `${formatNumber(size / MB)} MB`;
+ }
+ if (size >= KB) {
+ return `${formatNumber(size / KB)} KB`;
+ }
+ return `${formatNumber(size)} B`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Use binary (1024) conversion for user-friendly display | |
| // This matches file system conventions and gives clean numbers | |
| const MB = 1024 * 1024; | |
| const KB = 1024; | |
| return formatter.format(size); | |
| if (size >= MB) { | |
| const mb = size / MB; | |
| // Show whole number if it's exact, otherwise 1 decimal | |
| return mb % 1 === 0 ? `${mb} MB` : `${mb.toFixed(1)} MB`; | |
| } else if (size >= KB) { | |
| const kb = size / KB; | |
| return kb % 1 === 0 ? `${kb} KB` : `${kb.toFixed(1)} KB`; | |
| } else { | |
| return `${size} B`; | |
| } | |
| // Use binary (1024) conversion for user-friendly display | |
| // This matches file system conventions and gives clean numbers | |
| const MB = 1024 * 1024; | |
| const KB = 1024; | |
| const formatNumber = (value: number) => { | |
| return new Intl.NumberFormat(language, { | |
| minimumFractionDigits: Number.isInteger(value) ? 0 : 1, | |
| maximumFractionDigits: 1, | |
| }).format(Number.isInteger(value) ? value : Number(value.toFixed(1))); | |
| }; | |
| if (size >= MB) { | |
| return `${formatNumber(size / MB)} MB`; | |
| } | |
| if (size >= KB) { | |
| return `${formatNumber(size / KB)} KB`; | |
| } | |
| return `${formatNumber(size)} B`; |
🤖 Prompt for AI Agents
In src/frontend/src/features/utils/attachment-helper/index.ts around lines 70 to
84, restore locale-aware numeric formatting: keep the binary thresholds (KB =
1024, MB = 1024*1024) and compute mb/kb as before, but pass the numeric part
through Intl.NumberFormat using the existing language argument (or navigator
language fallback) so decimal separators and grouping follow the active locale;
format whole numbers with no fraction digits and non-whole values with one
decimal place (e.g. use Intl.NumberFormat(language, { maximumFractionDigits: 1,
minimumFractionDigits: 0 }) and conditionally call with 0 or 1 fraction digits
or use Number.isInteger to decide), then inject the formatted string into the
unit (e.g. "… MB", "… KB", "… B").
|
WIP ;) Long story somewhat short: init
frontend
backend
|
|
I'm wondering if the draft updates won't be larger (due to JSON serialization and the blocknote data format) than the final body size ? If so we could add a 1.x factor for DATA_UPLOAD_MAX_MEMORY_SIZE. BTW the comment from coderabbit saying that including attachment sizes in the max is useless seems relevant ? |
|
Ah, true. I was already in the users' context where it should be clear that the max 10Mb, that the user actually can use 10Mb (and not be limited tot 7Mb due to this overhead). But wé should take care for specifically this DATA_UPLOAD_MAX_MEMORY_SIZE to have indeed a factor above it, you are right. So if we communicate (and set in .env) 10Mb, the data_upload_max should be a factor 1.x. The server size check itself is not adding this overhead (I already tested this by pasting 5Mb of body text on a 5Mb limit). About coderabbits comment that is true - I was under the impression that we send ONE upload that has to deal with that DATA_UPLOAD_MAX_MEMORY_SIZE, so adding them is useless. It should be the max of the 3, max body, max incoming, max outgoing, correct. Also, I chose to have only the popup on the bottom-left for UI messages. We now have 3 options: below the attachment upload (small text), as a toast popup at the top right, or messages you get bottom-left (e.g. server errors). If we attach an attachment too big, where would we want to show the message? For me bottom-left seems the 'standard'? |
3 participants
Add DATA_UPLOAD_MAX_MEMORY_SIZE to settings.py to allow configuration via environment variable. Django's default is 2.5MB which is too small for emails with larger attachments.
Without this setting in settings.py, Django ignores the environment variable and uses its hardcoded default, causing RequestDataTooBig errors when the MTA tries to deliver emails exceeding 2.5MB.
Can be overridden via DATA_UPLOAD_MAX_MEMORY_SIZE environment variable.
Summary by CodeRabbit
New Features
Bug Fixes / Validation
UI
Tests