[WEB-4316] chore: new endpoints to download an asset#7207
[WEB-4316] chore: new endpoints to download an asset#7207sriramveeraghanta merged 2 commits intopreviewfrom
Conversation
WalkthroughTwo new API endpoints were introduced to enable downloading assets at the workspace and project levels. These endpoints generate presigned S3 URLs for asset downloads and were registered in the URL configuration. The corresponding endpoint classes were implemented and imported as needed. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Server
participant S3_Storage
Client->>API_Server: GET /assets/v2/workspaces/{slug}/download/{asset_id}/
API_Server->>API_Server: Validate permissions and fetch asset
API_Server->>S3_Storage: Generate presigned download URL
S3_Storage-->>API_Server: Return presigned URL
API_Server-->>Client: Redirect to presigned S3 URL
sequenceDiagram
participant Client
participant API_Server
participant S3_Storage
Client->>API_Server: GET /assets/v2/workspaces/{slug}/projects/{project_id}/download/{asset_id}/
API_Server->>API_Server: Validate permissions and fetch asset
API_Server->>S3_Storage: Generate presigned download URL
S3_Storage-->>API_Server: Return presigned URL
API_Server-->>Client: Redirect to presigned S3 URL
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
apiserver/plane/app/views/asset/v2.py (1)
723-755: Avoid copy-pasted endpoint logic
WorkspaceAssetDownloadEndpointandProjectAssetDownloadEndpointare almost identical except for the extraproject_idfilter.
Extract a small helper (or mix-in) to fetch the asset, run the common checks, and build the presigned URL. This keeps behaviour consistent, removes duplication, and simplifies future maintenance.🧰 Tools
🪛 Ruff (0.11.9)
724-724: Line too long (96 > 88)
(E501)
740-740: Line too long (96 > 88)
(E501)
🪛 Pylint (3.3.7)
[refactor] 723-723: Too few public methods (1/2)
(R0903)
[refactor] 739-739: Too few public methods (1/2)
(R0903)
apiserver/plane/app/urls/asset.py (1)
94-103: Minor: long lines breach Ruff E501Lines 95 & 100 exceed 88 chars.
If you adopt the shorterdispositionstring proposed above this will become moot, otherwise break the arguments onto multiple lines.apiserver/plane/app/views/__init__.py (1)
110-112: Import block is growing; consider grouping V2 asset endpointsNo functional issue, but the ever-expanding flat import list is getting unwieldy.
Grouping V2 asset endpoints into a dedicated sub-module (e.g.from .asset import v2 as asset_v2) would improve readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apiserver/plane/app/urls/asset.py(2 hunks)apiserver/plane/app/views/__init__.py(1 hunks)apiserver/plane/app/views/asset/v2.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
apiserver/plane/app/views/__init__.py (1)
apiserver/plane/app/views/asset/v2.py (2)
WorkspaceAssetDownloadEndpoint(723-736)ProjectAssetDownloadEndpoint(739-755)
apiserver/plane/app/urls/asset.py (1)
apiserver/plane/app/views/asset/v2.py (2)
WorkspaceAssetDownloadEndpoint(723-736)ProjectAssetDownloadEndpoint(739-755)
🪛 Ruff (0.11.9)
apiserver/plane/app/views/asset/v2.py
724-724: Line too long (96 > 88)
(E501)
740-740: Line too long (96 > 88)
(E501)
🪛 Pylint (3.3.7)
apiserver/plane/app/views/asset/v2.py
[refactor] 723-723: Too few public methods (1/2)
(R0903)
[refactor] 739-739: Too few public methods (1/2)
(R0903)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apiserver/plane/app/views/asset/v2.py (2)
741-744: Unsafe filename inContent-Dispositionheader
The header embedsasset.asset.name, which contains the S3 prefix and unescaped characters. This replicates an earlier comment on the same issue. Use the original filename fromasset.attributes["name"](fallback toPath(...).name) and URL-quote it to comply with RFC 6266.
769-771: Unsafe filename inContent-Dispositionheader
Same issue and fix as the workspace endpoint.
🧹 Nitpick comments (1)
apiserver/plane/app/views/asset/v2.py (1)
740-746: Long line violates Ruff E501 & duplicates code — extract helper
Both download endpoints build the presigned URL in an identical way, and the line length exceeds 88 chars. Factor this into a small utility (e.g.,_get_signed_download_url(asset)) to dedupe logic and satisfy linters.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apiserver/plane/app/views/asset/v2.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
apiserver/plane/app/views/asset/v2.py
724-724: Line too long (96 > 88)
(E501)
750-750: Line too long (96 > 88)
(E501)
🪛 Pylint (3.3.7)
apiserver/plane/app/views/asset/v2.py
[refactor] 723-723: Too few public methods (1/2)
(R0903)
[refactor] 749-749: Too few public methods (1/2)
(R0903)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apiserver/plane/app/views/asset/v2.py (2)
728-734: Prefer.all_objects+ explicit filter to ensure soft-deleted rows are ignoredThe code relies on the default manager (
FileAsset.objects) to hide deleted rows. That assumption leaks the model-level deletion policy into view logic. Use.all_objectsand add an explicitdeleted_at__isnull=True(oris_deleted=False) filter for robustness and future-proofing.- asset = FileAsset.objects.get( + asset = FileAsset.all_objects.get( id=asset_id, workspace__slug=slug, is_uploaded=True, + deleted_at__isnull=True, )
755-761: Repeat of soft-delete concern for project-scoped querySame reasoning as the workspace variant—use
.all_objectsplusdeleted_at__isnull=Trueto avoid resurrecting soft-deleted assets when the default manager behavior changes.
* refactor: move web utils to packages * fix: build and lint errors * chore: update drag handle plugin * chore: update table cell type to fix build errors * fix: build errors * chore: sync few changes * feat: add pi base url to constants * chore: update all util imports in ee folder * chore: refactor web utils imports * chore: update imports * fix: build errors * fix: build errors * fix: update utils import * chore: minor fixes related to duplicate assets imports * chore: update duplicate assets service * [WEB-4316] chore: new endpoints to download an asset (#7207) * chore: new endpoints to download an asset * chore: add exception handling * [WEB-4323] refactor: Analytics refactor (#7213) * chore: updated label for epics * chore: improved export logic * refactor: move csvConfig to export.ts and clean up export logic * refactor: remove unused CSV export logic from WorkItemsInsightTable component * refactor: streamline data handling in InsightTable component for improved rendering * feat: add translation for "No. of {entity}" and update priority chart y-axis label to use new translation * refactor: cleaned up some component and added utilitites * feat: add "at_risk" translation to multiple languages in translations.json files * refactor: update TrendPiece component to use new status variants for analytics * fix: adjust TrendPiece component logic for on-track and off-track status * refactor: use nullish coalescing operator for yAxis.dx in line and scatter charts * feat: add "at_risk" translation to various languages in translations.json files * feat: add "no_of" translation to various languages in translations.json files * feat: update "at_risk" translation in Ukrainian, Vietnamese, and Chinese locales in translations.json files * refactor: rename insightsFields to ANALYTICS_INSIGHTS_FIELDS and update analytics tab import to use getAnalyticsTabs function * feat: update AnalyticsWrapper to use i18n for titles and add new translation for "no_of" in Russian * fix: update yAxis labels and offsets in various charts to use new translation key and improve layout * feat: define AnalyticsTab interface and refactor getAnalyticsTabs function for improved type safety * fix: update AnalyticsTab interface to use TAnalyticsTabsBase for improved type safety * fix: add whitespace-nowrap class to TableHead for improved header layout in DataTable component * [WEB-4311] fix: membership data handling and state reversal on error (#7205) * [WEB-4231] Pie chart tooltip #7192 * fix: build errors * fix: utils imports * chore: fix build errors * yarn lock file update --------- Co-authored-by: Aaryan Khandelwal <65252264+aaryan610@users.noreply.github.com> Co-authored-by: JayashTripathy <76092296+JayashTripathy@users.noreply.github.com> Co-authored-by: sriram veeraghanta <veeraghanta.sriram@gmail.com>
* chore: new endpoints to download an asset * chore: add exception handling
3 participants
Description
This PR creates 2 new endpoints to download assets.
Type of Change
Summary by CodeRabbit