[WIKI-181] chore: asset check endpoint added#7140
Conversation
WalkthroughA new API endpoint and supporting frontend logic were introduced to check the existence of a file asset by its ID within a workspace. This includes backend Django view and URL updates, TypeScript type and service additions, and integration of the check method into editor-related handlers. Changes
Sequence Diagram(s)sequenceDiagram
participant Frontend
participant FileService
participant BackendAPI
participant Database
Frontend->>FileService: checkIfAssetExists(workspaceSlug, assetId)
FileService->>BackendAPI: GET /api/assets/v2/workspaces/{workspaceSlug}/check/{assetId}/
BackendAPI->>Database: Query FileAsset (by assetId, workspaceSlug, not deleted)
Database-->>BackendAPI: Asset existence (true/false)
BackendAPI-->>FileService: { exists: true/false }
FileService-->>Frontend: { exists: true/false }
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
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)packages/editor/src/core/types/config.ts(1 hunks)space/helpers/editor.helper.ts(1 hunks)web/core/hooks/editor/use-editor-config.ts(1 hunks)web/core/services/file.service.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
apiserver/plane/app/views/__init__.py (1)
apiserver/plane/app/views/asset/v2.py (1)
AssetCheckEndpoint(712-720)
apiserver/plane/app/urls/asset.py (1)
apiserver/plane/app/views/asset/v2.py (1)
AssetCheckEndpoint(712-720)
web/core/hooks/editor/use-editor-config.ts (1)
web/core/store/router.store.ts (1)
workspaceSlug(69-71)
web/core/services/file.service.ts (1)
web/core/store/router.store.ts (1)
workspaceSlug(69-71)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (7)
apiserver/plane/app/views/__init__.py (1)
109-109: LGTM: Clean import addition following established patterns.The import of
AssetCheckEndpointis properly placed within the asset v2 imports section and follows the existing code organization.packages/editor/src/core/types/config.ts (1)
2-2: LGTM: Well-defined type signature for asset existence checking.The method signature is clear and appropriate:
- Descriptive parameter name
assetId- Correct return type
Promise<boolean>for async existence check- Properly placed within
TReadOnlyFileHandlertypeapiserver/plane/app/urls/asset.py (2)
15-15: LGTM: Proper import of the new endpoint.The
AssetCheckEndpointimport is correctly added to support the new URL routing.
87-91: LGTM: Well-structured URL pattern following established conventions.The new URL pattern is properly designed:
- Follows RESTful naming with
/check/path segment- Uses appropriate parameter types (
str:slug,uuid:asset_id)- Consistent with other v2 asset endpoints
- Descriptive URL name "asset-check"
web/core/hooks/editor/use-editor-config.ts (1)
32-35:Details
❌ Incorrect review comment
Verify workspaceSlug availability and add error handling.
The implementation looks good and follows existing patterns. However, consider adding validation for
workspaceSlugto prevent potential runtime errors.Let me check if
workspaceSlugcan be undefined in this context:
🏁 Script executed:
#!/bin/bash # Description: Check how workspaceSlug is used in similar methods and verify its availability # Search for similar usage patterns of workspaceSlug in this file and related files rg -A 3 -B 3 "workspaceSlug" --type ts # Check if there are any null checks for workspaceSlug in the codebase ast-grep --pattern 'workspaceSlug && $_'Length of output: 68195
I need to inspect how
workspaceSlugis sourced in the hook to confirm its non-null guarantees:
🏁 Script executed:
#!/bin/bash # Print the start of the hook file to see where workspaceSlug comes from sed -n '1,200p' web/core/hooks/editor/use-editor-config.tsLength of output: 2919
No workspaceSlug null check needed
The
workspaceSlugparameter is defined as a requiredstringin theTArgstype and is always passed intogetReadOnlyEditorFileHandlers. TypeScript guarantees it cannot beundefined, so no additional runtime validation is necessary.Likely an incorrect or invalid review comment.
web/core/services/file.service.ts (1)
239-250: LGTM! Clean implementation following established patterns.The method implementation is excellent:
- Consistent with existing FileService patterns
- Proper error handling with standardized error propagation
- Clear method signature and return type
- Appropriate URL construction following the API conventions
apiserver/plane/app/views/asset/v2.py (1)
712-720:Details
❓ Verification inconclusive
LGTM! Well-structured endpoint with proper permissions and filtering.
The implementation is excellent:
- Appropriate permission decorator for workspace-level access
- Correct use of
all_objectswithdeleted_at__isnull=Truefilter to check non-deleted assets- Proper response format matching frontend expectations
- Follows established Django view patterns in the codebase
Consider database performance if this endpoint will be called frequently:
🏁 Script executed:
#!/bin/bash # Description: Check if there are database indexes on the fields used in the query # Look for database migrations or model definitions that might have indexes fd -e py -x grep -l "asset.*index\|index.*asset" {} \; rg -A 5 -B 5 "class.*FileAsset" --type pyLength of output: 4691
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== FileAsset Model Definition ===" rg -n "class FileAsset" -A 200 apiserver/plane/db/models/asset.py echo "" echo "=== Workspace Model File Path ===" rg -l "class Workspace" --type py echo "" echo "=== Workspace Model Definition ===" rg -n "class Workspace" -A 200 $(rg -l "class Workspace" --type py)Length of output: 97584
AssetCheckEndpoint Implementation Approved — Please Confirm Database Indexes
The
AssetCheckEndpointis well-structured, follows existing Django patterns, and correctly uses theall_objectsmanager withdeleted_at__isnull=Trueto check for non-deleted assets. Permissions and response format are appropriate.Performance considerations:
file_assets.idis the primary key and indexed by default.file_assets.workspace_id(foreign key) is indexed by default.- Please confirm that
workspace.slugis defined as a unique/indexed field in theWorkspacemodel to ensure efficient joins on theworkspace__slugfilter.
4 participants
Description
This PR creates a new endpoint to check whether an asset exists or not.
Type of Change
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Style
Tests
Chores
Revert