[WEB 3053]feat: support LibreOffice file attachments in issues#6343
[WEB 3053]feat: support LibreOffice file attachments in issues#6343
Conversation
WalkthroughThis pull request introduces significant enhancements to the issue attachment handling in the API server. The changes focus on improving file attachment management by adding file type validation, implementing soft delete functionality, and expanding support for various file formats. The modifications include adding a new Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 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 (3)
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
sangeethailango
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
apiserver/plane/app/views/issue/attachment.py (1)
Line range hint
183-198: HandleFileAssetnot found ingetmethodWhen a
pkis provided in thegetmethod, the code assumes theFileAssetexists. If it doesn't (e.g., invalidpk), aDoesNotExistexception will be raised, resulting in a 500 Internal Server Error.Add exception handling to return a 404 response when the
FileAssetis not found.Apply this diff to handle exceptions:
if pk: + try: # Get the asset asset = FileAsset.objects.get( id=pk, workspace__slug=slug, project_id=project_id ) + except FileAsset.DoesNotExist: + return Response( + {"error": "Asset not found.", "status": False}, + status=status.HTTP_404_NOT_FOUND, + ) # Check if the asset is uploaded if not asset.is_uploaded: return Response(
🧹 Nitpick comments (3)
apiserver/plane/app/views/issue/attachment.py (2)
123-128: CreateFileAssetafter successful presigned URL generationCurrently, the
FileAssetis created before generating the presigned URL. If the presigned URL generation fails, you'll have an orphanedFileAssetrecord without a valid upload path.Consider creating the
FileAssetonly after successfully generating the presigned URL, or implement cleanup logic for failed cases.Apply this diff to adjust the creation timing:
-# Create a File Asset -asset = FileAsset.objects.create( - attributes={"name": name, "type": type, "size": size_limit}, - asset=asset_key, - size=size_limit, - workspace_id=workspace.id, - created_by=request.user, - issue_id=issue_id, - project_id=project_id, - entity_type=FileAsset.EntityTypeContext.ISSUE_ATTACHMENT, -) # Get the presigned URL storage = S3Storage(request=request) try: presigned_url = storage.generate_presigned_post( object_name=asset_key, file_type=type, file_size=size_limit ) +except Exception as e: + return Response( + {"error": "Failed to generate upload URL.", "details": str(e), "status": False}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR, + ) +# Create a File Asset only after successful presigned URL generation +asset = FileAsset.objects.create( + attributes={"name": name, "type": type, "size": size_limit}, + asset=asset_key, + size=size_limit, + workspace_id=workspace.id, + created_by=request.user, + issue_id=issue_id, + project_id=project_id, + entity_type=FileAsset.EntityTypeContext.ISSUE_ATTACHMENT, +)
Line range hint
155-160: Remove physical files upon soft deleteIn the
deletemethod, theFileAssetis marked as deleted in the database, but the actual file in storage remains. This can lead to unnecessary storage costs and potential security risks since obsolete files are still accessible.Consider implementing a background task or modifying the deletion logic to remove the physical file from storage when an asset is soft deleted.
apiserver/plane/settings/common.py (1)
364-375: Correct typo in comment and verify MIME typesThere's a typo in the comment on line 374~: "Open Office Bae" should be "OpenOffice Base."
Also, ensure that all added MIME types are accurate and necessary for your application's use cases.
Apply this diff to fix the typo:
- # Open Office Bae + # OpenOffice Base
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apiserver/plane/app/views/issue/attachment.py(1 hunks)apiserver/plane/settings/common.py(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint-apiserver
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apiserver/plane/app/views/issue/attachment.py (1)
123-128: Verify handling of file size limitsThe
size_limitis determined by thesizeprovided in the request, but there is no guarantee that the actual file uploaded will respect this limit. If a user uploads a file larger than the declaredsize, it could cause issues or exceed storage quotas.Ensure that the storage backend enforces the file size limit or consider adding server-side validation after the upload is complete.
apiserver/plane/settings/common.py (1)
364-375: Review security implications of added MIME typesSome added MIME types, such as OpenDocument formats and certain image types, might contain executable content or scripts, potentially leading to security vulnerabilities if not properly handled.
Ensure that the application performs thorough validation and sanitization of these file types to mitigate security risks.
Also applies to: 393-394
NarayanBavisetti
changed the title
NarayanBavisetti
changed the title
3 participants
Description
This PR allows LibreOffice, Microsoft Viseo, Netpbm, Open Office Bea files in the issue attachments.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes