[WEB-2092] fix: added unique constraints for project, module and states#5281
Conversation
WalkthroughThe recent changes significantly enhance the application's management of soft deletions by introducing a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CycleAPI
participant CycleIssueDB
User->>CycleAPI: Delete Cycle
CycleAPI->>CycleIssueDB: Remove associated CycleIssues
CycleAPI->>User: Confirm Cycle deletion
sequenceDiagram
participant User
participant ModuleAPI
participant ModuleIssueDB
User->>ModuleAPI: Delete Module
ModuleAPI->>ModuleIssueDB: Remove associated ModuleIssues
ModuleAPI->>User: Confirm Module deletion
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![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
Files selected for processing (17)
- apiserver/plane/api/serializers/cycle.py (1 hunks)
- apiserver/plane/api/serializers/module.py (1 hunks)
- apiserver/plane/api/serializers/project.py (1 hunks)
- apiserver/plane/api/views/cycle.py (1 hunks)
- apiserver/plane/api/views/module.py (1 hunks)
- apiserver/plane/app/serializers/module.py (1 hunks)
- apiserver/plane/app/serializers/project.py (1 hunks)
- apiserver/plane/app/views/cycle/base.py (1 hunks)
- apiserver/plane/app/views/cycle/issue.py (3 hunks)
- apiserver/plane/app/views/issue/attachment.py (2 hunks)
- apiserver/plane/app/views/module/base.py (1 hunks)
- apiserver/plane/app/views/module/issue.py (5 hunks)
- apiserver/plane/bgtasks/issue_activites_task.py (3 hunks)
- apiserver/plane/db/migrations/0074_alter_label_unique_together_and_more.py (1 hunks)
- apiserver/plane/db/models/issue.py (2 hunks)
- apiserver/plane/db/models/module.py (2 hunks)
- apiserver/plane/db/models/project.py (3 hunks)
Additional comments not posted (19)
apiserver/plane/api/serializers/cycle.py (1)
43-43: LGTM!The addition of the
deleted_atfield to theread_only_fieldslist in theMetaclass aligns with the objective of managing soft deletions.apiserver/plane/db/migrations/0074_alter_label_unique_together_and_more.py (1)
1-49: LGTM!The migration file correctly introduces unique constraints and conditions for the
label,module,project, andprojectidentifiermodels to handle soft deletions. This aligns with the objective of ensuring data integrity and uniqueness.apiserver/plane/api/serializers/project.py (1)
34-34: LGTM!The addition of the
deleted_atfield to theread_only_fieldslist in theMetaclass aligns with the objective of managing soft deletions.apiserver/plane/api/serializers/module.py (1)
42-42: Ensure proper handling of the new fielddeleted_at.The addition of the
deleted_atfield to thefieldslist in theMetaclass enhances the functionality of the serializer. Ensure that thedeleted_atfield is properly handled in the serializer's methods, such asto_representation,validate,create, andupdate.apiserver/plane/db/models/module.py (1)
100-107: Ensure proper handling of the new constraints.The addition of the
deleted_atfield to theunique_togetherconstraint and the newUniqueConstraintenhance data integrity and provide a mechanism for managing soft deletions. Ensure that the new constraints are properly handled in the model's methods, such assave,delete, and any custom methods that may interact with these constraints.apiserver/plane/app/serializers/module.py (1)
42-43: Addition ofdeleted_atfield approved.The
deleted_atfield has been correctly added to theread_only_fieldslist, ensuring it is included in the serialized output but not modifiable through the serializer.apiserver/plane/app/serializers/project.py (1)
31-32: Addition ofdeleted_atfield approved.The
deleted_atfield has been correctly added to theread_only_fieldslist, ensuring it is included in the serialized output but not modifiable through the serializer.apiserver/plane/db/models/project.py (2)
128-143: Modifications tounique_togetherconstraints and addition ofUniqueConstraintentries approved.The modifications to the
unique_togetherconstraints and the addition of the newUniqueConstraintentries ensure that the combination ofidentifierandworkspaceas well asnameandworkspaceare unique whendeleted_atis null. This effectively prevents duplicates of active projects and maintains data integrity.
242-249: Modifications tounique_togetherconstraints and addition ofUniqueConstraintentry approved.The modifications to the
unique_togetherconstraints and the addition of the newUniqueConstraintentry ensure that the combination ofnameandworkspaceis unique whendeleted_atis null. This effectively prevents duplicates of active project identifiers and maintains data integrity.apiserver/plane/app/views/module/issue.py (2)
Line range hint
310-323:
LGTM! But verify the function usage in the codebase.The change to use
filter()instead ofget()improves robustness by preventing exceptions when no records are found. Ensure that all usages ofdestroyare verified to confirm compatibility with this change.
Line range hint
286-299:
LGTM! But verify the function usage in the codebase.The change to use
filter()instead ofget()improves robustness by preventing exceptions when no records are found. Ensure that all usages ofcreate_issue_modulesare verified to confirm compatibility with this change.Verification successful
LGTM! The change to use
filter()instead ofget()improves robustness by preventing exceptions when no records are found.
create_issue_modulesmethod properly handles the results offilter().Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of `create_issue_modules` to ensure compatibility with the changes. # Test: Search for the function usage. Expect: No breaking changes. rg --type python -A 5 $'create_issue_modules'Length of output: 75
Script:
#!/bin/bash # Description: Verify all usages of `create_issue_modules` to ensure compatibility with the changes. # Test: Search for the function usage. Expect: No breaking changes. rg --type py -A 5 $'create_issue_modules'Length of output: 887
Script:
#!/bin/bash # Description: Examine the content of `apiserver/plane/app/views/module/issue.py` around the `create_issue_modules` method to ensure proper handling of the `filter()` results. # Test: Display the content of `apiserver/plane/app/views/module/issue.py` around the `create_issue_modules` method. rg --type py -A 20 -B 10 $'def create_issue_modules' apiserver/plane/app/views/module/issue.pyLength of output: 1320
Script:
#!/bin/bash # Description: Display the complete content of the `create_issue_modules` method in `apiserver/plane/app/views/module/issue.py`. # Test: Display the complete content of the `create_issue_modules` method. rg --type py -A 50 -B 10 $'def create_issue_modules' apiserver/plane/app/views/module/issue.pyLength of output: 2534
apiserver/plane/app/views/cycle/issue.py (1)
Line range hint
337-350:
LGTM! But verify the function usage in the codebase.The change to use
filter()instead ofget()improves robustness by preventing exceptions when no records are found. Ensure that all usages ofdestroyare verified to confirm compatibility with this change.apiserver/plane/db/models/issue.py (1)
538-545: LGTM! But verify the class usage in the codebase.The change to enhance uniqueness constraints by adding the
deleted_atfield improves data integrity. Ensure that all usages of theLabelclass are verified to confirm compatibility with this change.apiserver/plane/api/views/module.py (1)
304-307: Good addition for maintaining data integrity.The added lines ensure that
ModuleIssuerecords related to the deleted module are also removed, preventing orphaned records.apiserver/plane/app/views/module/base.py (1)
776-779: Good addition for maintaining data integrity.The added lines ensure that
ModuleIssuerecords related to the deleted module are also removed, preventing orphaned records.apiserver/plane/api/views/cycle.py (1)
407-410: Good addition for maintaining data integrity.The added lines ensure that
CycleIssuerecords related to the deleted cycle are also removed, preventing orphaned records.apiserver/plane/bgtasks/issue_activites_task.py (2)
Line range hint
1085-1088:
LGTM!The removal of the blank line is a minor formatting change and does not affect the logic or functionality.
675-675: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
delete_issue_activitymatch the new signature.Verification successful
Function usage matches the new signature.
The only usage of
delete_issue_activityis in the dictionary mapping, and no other calls were found in the codebase. Therefore, the function usage matches the new signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `delete_issue_activity` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type python -A 5 $'delete_issue_activity'Length of output: 76
Script:
#!/bin/bash # Description: Verify all function calls to `delete_issue_activity` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type py -A 5 $'delete_issue_activity'Length of output: 1127
Script:
#!/bin/bash # Description: Verify all function calls to `delete_issue_activity` match the new signature. # Test: Search for all function calls to `delete_issue_activity` across the codebase. rg '\bdelete_issue_activity\b' --type py -A 5Length of output: 1133
apiserver/plane/app/views/cycle/base.py (1)
1085-1088: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
destroyproperly handle the deletion of associatedCycleIssuerecords.
| if issue_attachment.created_by_id != request.user.id and ( | ||
| not ProjectMember.objects.filter( | ||
| workspace__slug=slug, | ||
| member=request.user, | ||
| role=20, | ||
| project_id=project_id, | ||
| is_active=True, | ||
| ).exists() | ||
| ): | ||
| return Response( | ||
| {"error": "Only admin or creator can delete the attachment"}, | ||
| status=status.HTTP_403_FORBIDDEN, | ||
| ) |
There was a problem hiding this comment.
Ensure proper error handling and security measures.
The condition correctly restricts deletion to either the creator of the attachment or a user with a specific role (admin) within the project. However, consider handling the case where IssueAttachment.objects.get(pk=pk) raises a DoesNotExist exception to avoid potential server errors.
+ from django.core.exceptions import ObjectDoesNotExist
def delete(self, request, slug, project_id, issue_id, pk):
+ try:
issue_attachment = IssueAttachment.objects.get(pk=pk)
+ except ObjectDoesNotExist:
+ return Response(
+ {"error": "Issue attachment not found"},
+ status=status.HTTP_404_NOT_FOUND,
+ )
if issue_attachment.created_by_id != request.user.id and (
not ProjectMember.objects.filter(
workspace__slug=slug,
member=request.user,
role=20,
project_id=project_id,
is_active=True,
).exists()
):
return Response(
{"error": "Only admin or creator can delete the attachment"},
status=status.HTTP_403_FORBIDDEN,
)
issue_attachment.asset.delete(save=False)
issue_attachment.delete()
issue_activity.delay(
type="attachment.activity.deleted",
requested_data=None,
actor_id=str(self.request.user.id),
issue_id=str(self.kwargs.get("issue_id", None)),
project_id=str(self.kwargs.get("project_id", None)),
current_instance=None,
epoch=int(timezone.now().timestamp()),
notification=True,
origin=request.META.get("HTTP_ORIGIN"),
)
return Response(status=status.HTTP_204_NO_CONTENT)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.
| if issue_attachment.created_by_id != request.user.id and ( | |
| not ProjectMember.objects.filter( | |
| workspace__slug=slug, | |
| member=request.user, | |
| role=20, | |
| project_id=project_id, | |
| is_active=True, | |
| ).exists() | |
| ): | |
| return Response( | |
| {"error": "Only admin or creator can delete the attachment"}, | |
| status=status.HTTP_403_FORBIDDEN, | |
| ) | |
| from django.core.exceptions import ObjectDoesNotExist | |
| def delete(self, request, slug, project_id, issue_id, pk): | |
| try: | |
| issue_attachment = IssueAttachment.objects.get(pk=pk) | |
| except ObjectDoesNotExist: | |
| return Response( | |
| {"error": "Issue attachment not found"}, | |
| status=status.HTTP_404_NOT_FOUND, | |
| ) | |
| if issue_attachment.created_by_id != request.user.id and ( | |
| not ProjectMember.objects.filter( | |
| workspace__slug=slug, | |
| member=request.user, | |
| role=20, | |
| project_id=project_id, | |
| is_active=True, | |
| ).exists() | |
| ): | |
| return Response( | |
| {"error": "Only admin or creator can delete the attachment"}, | |
| status=status.HTTP_403_FORBIDDEN, | |
| ) | |
| issue_attachment.asset.delete(save=False) | |
| issue_attachment.delete() | |
| issue_activity.delay( | |
| type="attachment.activity.deleted", | |
| requested_data=None, | |
| actor_id=str(self.request.user.id), | |
| issue_id=str(self.kwargs.get("issue_id", None)), | |
| project_id=str(self.kwargs.get("project_id", None)), | |
| current_instance=None, | |
| epoch=int(timezone.now().timestamp()), | |
| notification=True, | |
| origin=request.META.get("HTTP_ORIGIN"), | |
| ) | |
| return Response(status=status.HTTP_204_NO_CONTENT) |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apiserver/plane/db/migrations/0074_alter_label_unique_together_and_more.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- apiserver/plane/db/migrations/0074_alter_label_unique_together_and_more.py
3 participants
fix:
Issue Link: WEB-2092
Summary by CodeRabbit
New Features
Bug Fixes
Chores