[WEB-4712] chore: work item attachment patch endpoint #7595
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughExpanded IssueAttachmentDetail route to accept PATCH in addition to GET and DELETE and moved permission enforcement from class-level declarations to per-action checks; introduced a module-level helper to centralize creator and role-based membership checks and added explicit issue-loading pre-checks and 403 responses. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant API as IssueAttachmentDetailAPIEndpoint
participant Auth as user_has_issue_permission / Issue Loader
participant Store as Attachment Service / DB
C->>API: GET /.../issue-attachments/{pk}
API->>Auth: verify requester is active project member (load Issue)
alt allowed
API->>Store: fetch attachment
Store-->>API: attachment data
API-->>C: 200 OK
else not allowed
API-->>C: 403 "You are not allowed to download this attachment."
end
C->>API: PATCH /.../issue-attachments/{pk}
API->>Auth: load Issue + check creator or active ADMIN/MEMBER
alt allowed
API->>Store: apply partial update / confirm upload
Store-->>API: updated attachment
API-->>C: 200 OK
else not allowed
API-->>C: 403 "You are not allowed to upload this attachment."
end
C->>API: DELETE /.../issue-attachments/{pk}
API->>Auth: load Issue + check creator or active ADMIN/MEMBER
alt allowed
API->>Store: delete attachment
Store-->>API: deletion result
API-->>C: 204 No Content
else not allowed
API-->>C: 403 "You are not allowed to delete this attachment."
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 0
🧹 Nitpick comments (1)
apps/api/plane/api/urls/issue.py (1)
92-94: Add a small test to lock down PATCH behaviorRecommend a regression test to assert PATCH is allowed and performs the upload-confirmation update (status and minimal payload).
Example test skeleton (adjust factories/fixtures as per your test setup):
# tests/api/issues/test_issue_attachment_detail.py from rest_framework.test import APIClient from django.urls import reverse def test_issue_attachment_detail_patch_confirms_upload(db, project, issue, attachment, user): client = APIClient() client.force_authenticate(user=user) url = reverse( "issue-attachment", kwargs={ "slug": project.workspace.slug, "project_id": str(project.id), "issue_id": str(issue.id), "pk": str(attachment.id), }, ) # payload limited to fields required to mark upload as confirmed payload = {"status": "uploaded"} # or the actual confirmation field res = client.patch(url, payload, format="json") assert res.status_code in (200, 204) # Optionally assert the attachment state changed as expectedIf you’d like, I can wire this into your existing test module structure or generate a more complete test using your factories.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/api/plane/api/urls/issue.py(1 hunks)
🔇 Additional comments (2)
apps/api/plane/api/urls/issue.py (2)
92-94: LGTM: PATCH enabled on issue-attachment detail routeThis aligns with other detail endpoints in this module and unblocks the upload-confirmation flow as described.
92-94: PATCH handler and permissions are correctly implemented
- The
IssueAttachmentDetailAPIEndpointclass inapps/api/plane/api/views/issue.pydefines apatch()method that marksis_uploaded = True, logs activity, and queues metadata extraction.- It uses
permission_classes = [ProjectEntityPermission], matching other issue‐attachment endpoints.- The view does not deserialize arbitrary fields—updates are handled manually, only setting
is_uploaded(andcreated_by) as intended.No further changes needed.
There was a problem hiding this comment.
Pull Request Overview
This PR adds PATCH HTTP method support to the issue attachment detail endpoint to enable confirmation of asset uploads for work items. The change allows partial updates to attachments without requiring reuploading.
- Added PATCH method to the issue attachment detail endpoint URL configuration
- Implemented custom permission checks for delete, get, and patch operations
- Commented out the default permission class to use custom authorization logic
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| apps/api/plane/api/urls/issue.py | Added "patch" to allowed HTTP methods for issue attachment detail endpoint |
| apps/api/plane/api/views/issue.py | Added custom permission checks and commented out default permission class |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/api/plane/api/views/issue.py (3)
2095-2110: Project membership check uses the wrong field and download should be limited to the correct attachment.
- Replace user_id with member_id and add workspace scoping.
- Ensure the fetched asset belongs to the issue, is an ISSUE_ATTACHMENT, and not soft-deleted; otherwise, users can download unrelated assets by ID.
Apply:
- if not ProjectMember.objects.filter( - project_id=project_id, - user_id=request.user.id, - is_active=True, - ).exists(): + if not ProjectMember.objects.filter( + workspace__slug=slug, + project_id=project_id, + member_id=request.user.id, + is_active=True, + ).exists(): return Response( {"error": "You are not allowed to download this attachment"}, status=status.HTTP_403_FORBIDDEN, ) # Get the asset - asset = FileAsset.objects.get( - id=pk, workspace__slug=slug, project_id=project_id - ) + asset = FileAsset.objects.get( + id=pk, + workspace__slug=slug, + project_id=project_id, + issue_id=issue_id, + entity_type=FileAsset.EntityTypeContext.ISSUE_ATTACHMENT, + is_deleted=False, + )
2161-2181: Fix PATCH permission check (member_id, workspace scope, is_active) and bind attachment to the issue.
- Use member_id, scope to workspace, and include is_active=True for admins to be consistent with DELETE.
- Prefer created_by_id check for the creator.
- Ensure the asset belongs to the given issue, type is ISSUE_ATTACHMENT, and not soft-deleted.
Apply:
- issue = Issue.objects.get( - pk=issue_id, workspace__slug=slug, project_id=project_id - ) + issue = Issue.objects.get( + pk=issue_id, workspace__slug=slug, project_id=project_id + ) # if the user is creator or admin then allow the upload - if ( - not request.user == issue.created_by - and not ProjectMember.objects.filter( - project_id=project_id, - user_id=request.user.id, - role=ProjectMember.Role.ADMIN, - ).exists() - ): + if ( + issue.created_by_id != request.user.id + and not ProjectMember.objects.filter( + workspace__slug=slug, + project_id=project_id, + member_id=request.user.id, + role=20, # ADMIN + is_active=True, + ).exists() + ): return Response( {"error": "You are not allowed to upload this attachment"}, status=status.HTTP_403_FORBIDDEN, ) - issue_attachment = FileAsset.objects.get( - pk=pk, workspace__slug=slug, project_id=project_id - ) + issue_attachment = FileAsset.objects.get( + pk=pk, + workspace__slug=slug, + project_id=project_id, + issue_id=issue_id, + entity_type=FileAsset.EntityTypeContext.ISSUE_ATTACHMENT, + is_deleted=False, + )
2015-2036: Enforce correct permission checks and tightly scope attachment deletionThe current code uses the wrong FK field, omits workspace scoping, and doesn’t verify the attachment belongs to the issue or is undeleted. Apply these critical fixes:
• Use
created_by_idfor the creator check and filter onmember_id(notuser_id), including the workspace slug
• RestrictProjectMemberlookup to the workspace and use the Admin role constant (20)
• Fetch theFileAssetbyissue_id, requireentity_type=ISSUE_ATTACHMENTandis_deleted=FalseApply:
- if ( - not request.user == issue.created_by - and not ProjectMember.objects.filter( - project_id=project_id, - user_id=request.user.id, - role=ProjectMember.Role.ADMIN, - is_active=True, - ).exists() - ): + if ( + issue.created_by_id != request.user.id + and not ProjectMember.objects.filter( + workspace__slug=slug, + project_id=project_id, + member_id=request.user.id, + role=20, # Admin + is_active=True, + ).exists() + ): return Response( {"error": "You are not allowed to delete this attachment"}, status=status.HTTP_403_FORBIDDEN, ) - issue_attachment = FileAsset.objects.get( - pk=pk, workspace__slug=slug, project_id=project_id - ) + issue_attachment = FileAsset.objects.get( + pk=pk, + workspace__slug=slug, + project_id=project_id, + issue_id=issue_id, + entity_type=FileAsset.EntityTypeContext.ISSUE_ATTACHMENT, + is_deleted=False, + ) issue_attachment.is_deleted = True
🧹 Nitpick comments (2)
apps/api/plane/api/views/issue.py (2)
1992-1992: Do not drop the baseline permission class on the attachment detail endpoint.Re-enable ProjectEntityPermission to keep consistent, defense-in-depth access control; keep your per-action checks on top.
Apply:
- # permission_classes = [ProjectEntityPermission] + permission_classes = [ProjectEntityPermission]If this was intentionally removed to allow non-members (e.g., original creators after removal) to PATCH/DELETE, confirm the policy and we can model that with a custom permission class instead of ad-hoc checks.
2015-2031: Reduce duplication: extract a small helper for attachment permissions.Consider a private helper on the view to DRY up the “creator-or-admin” checks (same logic in DELETE/PATCH), and to centralize scoping (workspace, project, member_id, is_active) and role constants. This will also prevent drift (note PATCH currently omitted is_active).
I can sketch a helper like _is_creator_or_active_admin(request, slug, project_id, issue) and reuse it in both methods.
Also applies to: 2161-2176
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/api/plane/api/views/issue.py(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/api/plane/api/views/issue.py (3)
apps/api/plane/db/models/issue.py (1)
Issue(104-260)apps/api/plane/api/views/asset.py (1)
get(421-472)apps/api/plane/db/models/project.py (1)
ProjectMember(206-250)
⏰ 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: Analyze (javascript)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/api/plane/api/views/issue.py (1)
2176-2194: Confirm baseline permissions; keep class-level permission_classes to avoid accidental gapsMoving entirely to per-action checks risks missing auth on future methods. Recommend retaining a baseline permission (e.g., ProjectEntityPermission) at class level and keeping fine-grained checks where needed. This mirrors the rest of this module.
Would you like me to propose the minimal change to reintroduce permission_classes on IssueAttachmentListCreateAPIEndpoint and IssueAttachmentDetailAPIEndpoint?
🧹 Nitpick comments (3)
apps/api/plane/api/views/issue.py (3)
78-78: Nit: Consolidate imports from plane.app.permissionsROLE is imported separately from other permissions on Line 55-59. Consider consolidating into a single import for consistency and readability.
Proposed:
from plane.app.permissions import ProjectEntityPermission, ProjectLitePermission, ProjectMemberPermission, ROLE
1867-1884: Per-action authorization looks good; consider extracting a shared helper to avoid duplicationThe “creator or active ADMIN/MEMBER” check is repeated across POST/PATCH/DELETE. Extracting a small helper (e.g., _is_issue_creator_or_active_member) will reduce duplication and minimize drift between methods.
Example helper (outside selected lines):
def _is_issue_creator_or_active_member(self, request, issue, project_id, allowed_roles):
if request.user == issue.created_by:
return True
return ProjectMember.objects.filter(
project_id=project_id,
member_id=request.user.id,
role__in=allowed_roles,
is_active=True,
).exists()
2176-2194: Optional: Honor the is_uploaded request flag or validate its presenceThe schema advertises an is_uploaded boolean, but the code ignores the request body and always sets is_uploaded to True when it was previously False. Either:
- validate that request.data.get("is_uploaded") is True; else 400, or
- update the schema to reflect that the endpoint unconditionally confirms upload.
Example (outside selected lines):
payload_flag = request.data.get("is_uploaded")
if payload_flag is not True:
return Response({"error": "is_uploaded must be true"}, status=status.HTTP_400_BAD_REQUEST)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/api/plane/api/views/issue.py(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/api/plane/api/views/issue.py (4)
packages/constants/src/workspace.ts (1)
ROLE(129-133)apps/api/plane/db/models/issue.py (1)
Issue(104-260)apps/api/plane/api/views/asset.py (1)
get(421-472)apps/api/plane/db/models/project.py (1)
ProjectMember(206-250)
⏰ 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). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
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)
apps/api/plane/api/views/issue.py (1)
2123-2125: GET attachments: possible IDOR and soft-deleted downloadsConstrain the asset to the current issue and entity type, and exclude soft-deleted assets to avoid downloading deleted content.
- asset = FileAsset.objects.get( - id=pk, workspace__slug=slug, project_id=project_id - ) + asset = FileAsset.objects.get( + id=pk, + workspace__slug=slug, + project_id=project_id, + issue_id=issue_id, + entity_type=FileAsset.EntityTypeContext.ISSUE_ATTACHMENT, + is_deleted=False, + )
♻️ Duplicate comments (2)
apps/api/plane/api/views/issue.py (2)
2195-2197: PATCH attachments: constrain lookup and move above permission blockAs with GET/DELETE, fetch the FileAsset with strict scoping and exclude soft-deletes. Move this retrieval above the permission check so you can validate against issue_attachment.created_by.
- issue_attachment = FileAsset.objects.get( - pk=pk, workspace__slug=slug, project_id=project_id - ) + issue_attachment = FileAsset.objects.get( + pk=pk, + workspace__slug=slug, + project_id=project_id, + issue_id=issue_id, + entity_type=FileAsset.EntityTypeContext.ISSUE_ATTACHMENT, + is_deleted=False, + )
2049-2051: Constrain FileAsset lookups in issue endpoints to prevent IDORAll three
FileAsset.objects.get(…)calls inapps/api/plane/api/views/issue.pymust be tightened to scope to the containing issue, only issue-attachments, and exclude soft-deleted records:• apps/api/plane/api/views/issue.py:2049-2051 (DELETE endpoint)
• apps/api/plane/api/views/issue.py:2123-2125 (DOWNLOAD endpoint)
• apps/api/plane/api/views/issue.py:2195-2197 (UPLOAD-COMPLETE endpoint)For each, replace:
- issue_attachment = FileAsset.objects.get( - pk=pk, workspace__slug=slug, project_id=project_id - ) + issue_attachment = FileAsset.objects.get( + pk=pk, + workspace__slug=slug, + project_id=project_id, + issue_id=issue_id, + entity_type=FileAsset.EntityTypeContext.ISSUE_ATTACHMENT, + is_deleted=False, + )And in the download endpoint, likewise:
- asset = FileAsset.objects.get( - id=pk, workspace__slug=slug, project_id=project_id - ) + asset = FileAsset.objects.get( + pk=pk, + workspace__slug=slug, + project_id=project_id, + issue_id=issue_id, + entity_type=FileAsset.EntityTypeContext.ISSUE_ATTACHMENT, + is_deleted=False, + )This ensures users cannot act on attachments outside the specified issue, only attachments of type
ISSUE_ATTACHMENT, and soft-deleted assets are excluded.
🧹 Nitpick comments (5)
apps/api/plane/api/views/issue.py (5)
1867-1884: POST attachments: tighten membership filter with workspace and avoid repeating permission logicThe permission check is sound. Add workspace scoping for defense-in-depth and consider extracting this repeated permission logic (also used in PATCH/DELETE) into a small helper to keep it DRY.
Apply this minimal tightening:
- if ( - not request.user == issue.created_by - and not ProjectMember.objects.filter( - project_id=project_id, - member_id=request.user.id, - role__in=[ROLE.ADMIN.value, ROLE.MEMBER.value], - is_active=True, - ).exists() - ): + if ( + not request.user == issue.created_by + and not ProjectMember.objects.filter( + workspace__slug=slug, + project_id=project_id, + member_id=request.user.id, + role__in=[ROLE.ADMIN.value, ROLE.MEMBER.value], + is_active=True, + ).exists() + ):Follow-up: If you want, I can generate a small internal helper like
_is_admin_or_member(slug, project_id, user_id)and replace the duplicated blocks.
2031-2048: DELETE attachments: permission condition OK; add workspace scoping to membership checkThe admin-only branch for deletion is correct. Add workspace filtering to the membership check for consistency with other querysets and to avoid cross-tenant edge-cases.
- if ( - not request.user == issue.created_by - and not ProjectMember.objects.filter( - project_id=project_id, - member_id=request.user.id, - role=ROLE.ADMIN.value, - is_active=True, - ).exists() - ): + if ( + not request.user == issue.created_by + and not ProjectMember.objects.filter( + workspace__slug=slug, + project_id=project_id, + member_id=request.user.id, + role=ROLE.ADMIN.value, + is_active=True, + ).exists() + ):Note: See next comment to constrain the FileAsset lookup used for deletion (critical).
2111-2121: GET attachments: add workspace scoping to membership checkProject membership gate looks good. Add workspace scoping for consistency and tenant safety.
- if not ProjectMember.objects.filter( - project_id=project_id, - member_id=request.user.id, - is_active=True, - ).exists(): + if not ProjectMember.objects.filter( + workspace__slug=slug, + project_id=project_id, + member_id=request.user.id, + is_active=True, + ).exists():
2215-2217: Do not overwrite created_by on upload confirmationReassigning created_by on PATCH mutates authorship; keep the original creator on the asset. If you want to track who confirmed the upload, consider a separate updated_by field or rely on activity logs (already emitted).
# Update the attachment issue_attachment.is_uploaded = True - issue_attachment.created_by = request.user
1780-1786: Consider adding a coarse permission_classes guard on the attachment endpointsYou’ve moved to per-action checks (fine), but adding a broad guard like ProjectEntityPermission at the class level for IssueAttachmentListCreateAPIEndpoint and IssueAttachmentDetailAPIEndpoint helps prevent regressions if new methods are added later without explicit checks.
Would you like me to draft the minimal addition and audit other endpoints for consistency?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/api/plane/api/views/issue.py(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
PR: makeplane/plane#7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Applied to files:
apps/api/plane/api/views/issue.py
🧬 Code Graph Analysis (1)
apps/api/plane/api/views/issue.py (4)
apps/api/plane/db/models/issue.py (1)
Issue(104-260)apps/api/plane/api/views/asset.py (1)
get(421-472)apps/api/plane/api/views/member.py (2)
get(64-88)get(114-137)apps/api/plane/db/models/project.py (1)
ProjectMember(206-250)
🔇 Additional comments (1)
apps/api/plane/api/views/issue.py (1)
78-78: ROLE import is appropriateUsing ROLE centralizes role constants and improves readability of permission checks.
| issue = Issue.objects.get( | ||
| pk=issue_id, workspace__slug=slug, project_id=project_id | ||
| ) | ||
| # if the user is creator or admin then allow the upload | ||
| if ( | ||
| not request.user == issue.created_by | ||
| and not ProjectMember.objects.filter( | ||
| project_id=project_id, | ||
| member_id=request.user.id, | ||
| role__in=[ROLE.ADMIN.value, ROLE.MEMBER.value], | ||
| is_active=True, | ||
| ).exists() | ||
| ): | ||
| return Response( | ||
| {"error": "You are not allowed to upload this attachment"}, | ||
| status=status.HTTP_403_FORBIDDEN, | ||
| ) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
PATCH attachments: check attachment creator (not issue creator) and reuse membership helper
The permission should allow the attachment creator or an active ADMIN/MEMBER to confirm upload. Checking issue.created_by here blocks the actual attachment creator (if they’re not the issue creator) and allows the issue creator to confirm someone else’s upload.
Refactor to:
- First load the attachment scoped to issue/entity_type (see next comment),
- Then allow if request.user == issue_attachment.created_by or user is ADMIN/MEMBER.
- issue = Issue.objects.get(
- pk=issue_id, workspace__slug=slug, project_id=project_id
- )
- # if the user is creator or admin then allow the upload
- if (
- not request.user == issue.created_by
- and not ProjectMember.objects.filter(
- project_id=project_id,
- member_id=request.user.id,
- role__in=[ROLE.ADMIN.value, ROLE.MEMBER.value],
- is_active=True,
- ).exists()
- ):
- return Response(
- {"error": "You are not allowed to upload this attachment"},
- status=status.HTTP_403_FORBIDDEN,
- )
+ # NOTE: Retrieve the attachment first (see retrieval below)
+ # Then check if requester is the attachment creator OR an active admin/member.
+ is_admin_or_member = ProjectMember.objects.filter(
+ workspace__slug=slug,
+ project_id=project_id,
+ member_id=request.user.id,
+ role__in=[ROLE.ADMIN.value, ROLE.MEMBER.value],
+ is_active=True,
+ ).exists()
+ if not (request.user == issue_attachment.created_by or is_admin_or_member):
+ return Response(
+ {"error": "You are not allowed to upload this attachment"},
+ status=status.HTTP_403_FORBIDDEN,
+ )Because this block references issue_attachment, move the attachment retrieval (next comment) above this permission check. For clarity, you can drop the Issue.objects.get call here; the scoping is enforced by the constrained FileAsset query.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/api/plane/api/views/issue.py around lines 2177 to 2194, the permission
check incorrectly compares request.user to issue.created_by and blocks the
actual attachment creator; move the attachment retrieval (scoped by
issue/entity_type) above this permission block, drop the separate
Issue.objects.get (scoping will be enforced by the FileAsset query), and change
the condition to allow the request if request.user ==
issue_attachment.created_by OR the user is an active ADMIN/MEMBER (reuse the
existing ProjectMember membership helper/query used elsewhere); ensure the
permission returns 403 only when neither condition is true.
Description
added the patch in http method names for confirming the asset upload for a work item
[WEB-4719]
Type of Change
Summary by CodeRabbit
New Features
Security / Permissions