Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WEB-2691] chore: filtered the deleted labels and modules #5860

Merged
merged 1 commit into from
Oct 18, 2024
Merged

[WEB-2691] chore: filtered the deleted labels and modules #5860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions apiserver/plane/app/views/dashboard/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,10 @@ def dashboard_assigned_issues(self, request, slug):
ArrayAgg(
"labels__id",
distinct=True,
filter=~Q(labels__id__isnull=True),
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
),
),
Value([], output_field=ArrayField(UUIDField())),
),
Expand All @@ -235,7 +238,9 @@ def dashboard_assigned_issues(self, request, slug):
ArrayAgg(
"issue_module__module_id",
distinct=True,
filter=~Q(issue_module__module_id__isnull=True),
filter=~Q(issue_module__module_id__isnull=True)
& Q(issue_module__module__archived_at__isnull=True)
& Q(issue_module__module__deleted_at__isnull=True),
),
Value([], output_field=ArrayField(UUIDField())),
),
Expand Down Expand Up @@ -382,7 +387,10 @@ def dashboard_created_issues(self, request, slug):
ArrayAgg(
"labels__id",
distinct=True,
filter=~Q(labels__id__isnull=True),
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
),
),
Value([], output_field=ArrayField(UUIDField())),
),
Expand All @@ -399,7 +407,9 @@ def dashboard_created_issues(self, request, slug):
ArrayAgg(
"issue_module__module_id",
distinct=True,
filter=~Q(issue_module__module_id__isnull=True),
filter=~Q(issue_module__module_id__isnull=True)
& Q(issue_module__module__archived_at__isnull=True)
& Q(issue_module__module__deleted_at__isnull=True),
),
Value([], output_field=ArrayField(UUIDField())),
),
Expand Down
19 changes: 14 additions & 5 deletions apiserver/plane/app/views/inbox/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,10 @@ def get_queryset(self):
ArrayAgg(
"labels__id",
distinct=True,
filter=~Q(labels__id__isnull=True),
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
),
),
Value([], output_field=ArrayField(UUIDField())),
),
Expand All @@ -159,7 +162,8 @@ def get_queryset(self):
"issue_module__module_id",
distinct=True,
filter=~Q(issue_module__module_id__isnull=True)
& Q(issue_module__module__archived_at__isnull=True),
& Q(issue_module__module__archived_at__isnull=True)
& Q(issue_module__module__deleted_at__isnull=True),
),
Value([], output_field=ArrayField(UUIDField())),
),
Expand All @@ -186,7 +190,8 @@ def list(self, request, slug, project_id):
ArrayAgg(
"issue__labels__id",
distinct=True,
filter=~Q(issue__labels__id__isnull=True),
filter=~Q(issue__labels__id__isnull=True)
& Q(issue__labels__deleted_at__isnull=True),
),
Value([], output_field=ArrayField(UUIDField())),
)
Expand Down Expand Up @@ -298,15 +303,19 @@ def create(self, request, slug, project_id):
ArrayAgg(
"issue__labels__id",
distinct=True,
filter=~Q(issue__labels__id__isnull=True),
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
),
Comment on lines +306 to +309
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Inconsistent field references in 'label_ids' aggregation filter

There is a potential issue in the label_ids aggregation filter: the fields in the filter should match the annotated fields. Currently, the filter uses labels__id and labels__deleted_at, but the aggregation is over issue__labels__id. This mismatch may lead to incorrect filtering.

Apply this diff to correct the filter field references:

-                                filter=(
-                                    ~Q(labels__id__isnull=True)
-                                    & Q(labels__deleted_at__isnull=True)
+                                filter=(
+                                    ~Q(issue__labels__id__isnull=True)
+                                    & Q(issue__labels__deleted_at__isnull=True)
                                 ),
📝 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.

Suggested change
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
),
filter=(
~Q(issue__labels__id__isnull=True)
& Q(issue__labels__deleted_at__isnull=True)
),

),
Value([], output_field=ArrayField(UUIDField())),
),
assignee_ids=Coalesce(
ArrayAgg(
"issue__assignees__id",
distinct=True,
filter=~Q(issue__assignees__id__isnull=True),
filter=~Q(assignees__id__isnull=True)
& Q(assignees__member_project__is_active=True),
Comment on lines +317 to +318
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure consistent field references in 'assignee_ids' aggregation filter

Similar to the previous comment, the filter for assignee_ids uses assignees__id and assignees__member_project__is_active, whereas the aggregation is over issue__assignees__id. To ensure correct filtering, the field references should include the issue__ prefix.

Apply this diff to update the filter field references:

-                                filter=~Q(assignees__id__isnull=True)
-                                & Q(assignees__member_project__is_active=True),
+                                filter=~Q(issue__assignees__id__isnull=True)
+                                & Q(issue__assignees__member_project__is_active=True),
📝 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.

Suggested change
filter=~Q(assignees__id__isnull=True)
& Q(assignees__member_project__is_active=True),
filter=~Q(issue__assignees__id__isnull=True)
& Q(issue__assignees__member_project__is_active=True),

),
Value([], output_field=ArrayField(UUIDField())),
),
Expand Down
25 changes: 19 additions & 6 deletions apiserver/plane/app/views/issue/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,10 @@ def retrieve(self, request, slug, project_id, pk=None):
ArrayAgg(
"labels__id",
distinct=True,
filter=~Q(labels__id__isnull=True),
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
),
Comment on lines +474 to +477
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Archived labels are not excluded in the filter.

In the aggregation of label_ids, the filter checks for labels where deleted_at is null but does not exclude archived labels. This means archived labels may unintentionally be included in the results. To ensure only active labels are aggregated, add labels__archived_at__isnull=True to the filter condition.

Apply this diff to update the filter condition:

 filter=(
     ~Q(labels__id__isnull=True)
     & Q(labels__deleted_at__isnull=True)
+    & Q(labels__archived_at__isnull=True)
 ),
📝 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.

Suggested change
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
),
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
& Q(labels__archived_at__isnull=True)
),

),
Value([], output_field=ArrayField(UUIDField())),
),
Expand All @@ -489,7 +492,8 @@ def retrieve(self, request, slug, project_id, pk=None):
"issue_module__module_id",
distinct=True,
filter=~Q(issue_module__module_id__isnull=True)
& Q(issue_module__module__archived_at__isnull=True),
& Q(issue_module__module__archived_at__isnull=True)
& Q(issue_module__module__deleted_at__isnull=True),
),
Value([], output_field=ArrayField(UUIDField())),
),
Expand Down Expand Up @@ -568,7 +572,10 @@ def partial_update(self, request, slug, project_id, pk=None):
ArrayAgg(
"labels__id",
distinct=True,
filter=~Q(labels__id__isnull=True),
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
),
Comment on lines +575 to +578
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Archived labels are not excluded in the filter.

Similar to the previous occurrence, the filter for label_ids does not exclude archived labels. To prevent archived labels from being aggregated, add labels__archived_at__isnull=True to the filter condition.

Apply this diff:

 filter=(
     ~Q(labels__id__isnull=True)
     & Q(labels__deleted_at__isnull=True)
+    & Q(labels__archived_at__isnull=True)
 ),
📝 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.

Suggested change
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
),
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
& Q(labels__archived_at__isnull=True)
),

),
Value([], output_field=ArrayField(UUIDField())),
),
Expand All @@ -585,7 +592,9 @@ def partial_update(self, request, slug, project_id, pk=None):
ArrayAgg(
"issue_module__module_id",
distinct=True,
filter=~Q(issue_module__module_id__isnull=True),
filter=~Q(issue_module__module_id__isnull=True)
& Q(issue_module__module__archived_at__isnull=True)
& Q(issue_module__module__deleted_at__isnull=True),
),
Value([], output_field=ArrayField(UUIDField())),
),
Expand Down Expand Up @@ -855,7 +864,10 @@ def list(self, request, slug, project_id):
ArrayAgg(
"labels__id",
distinct=True,
filter=~Q(labels__id__isnull=True),
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
),
Comment on lines +867 to +870
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Archived labels are not excluded in the filter.

In the aggregation of label_ids, the filter does not exclude archived labels. Adding labels__archived_at__isnull=True ensures that only active labels are included.

Apply this diff:

 filter=(
     ~Q(labels__id__isnull=True)
     & Q(labels__deleted_at__isnull=True)
+    & Q(labels__archived_at__isnull=True)
 ),
📝 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.

Suggested change
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
),
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
& Q(labels__archived_at__isnull=True)
),

),
Value([], output_field=ArrayField(UUIDField())),
),
Expand All @@ -873,7 +885,8 @@ def list(self, request, slug, project_id):
"issue_module__module_id",
distinct=True,
filter=~Q(issue_module__module_id__isnull=True)
& Q(issue_module__module__archived_at__isnull=True),
& Q(issue_module__module__archived_at__isnull=True)
& Q(issue_module__module__deleted_at__isnull=True),
),
Value([], output_field=ArrayField(UUIDField())),
),
Expand Down
5 changes: 4 additions & 1 deletion apiserver/plane/app/views/issue/relation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,10 @@ def list(self, request, slug, project_id, issue_id):
ArrayAgg(
"labels__id",
distinct=True,
filter=~Q(labels__id__isnull=True),
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
),
),
Value([], output_field=ArrayField(UUIDField())),
),
Expand Down
9 changes: 7 additions & 2 deletions apiserver/plane/app/views/issue/sub_issue.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,10 @@ def get(self, request, slug, project_id, issue_id):
ArrayAgg(
"labels__id",
distinct=True,
filter=~Q(labels__id__isnull=True),
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
),
),
Value([], output_field=ArrayField(UUIDField())),
),
Expand All @@ -94,7 +97,9 @@ def get(self, request, slug, project_id, issue_id):
ArrayAgg(
"issue_module__module_id",
distinct=True,
filter=~Q(issue_module__module_id__isnull=True),
filter=~Q(issue_module__module_id__isnull=True)
& Q(issue_module__module__archived_at__isnull=True)
& Q(issue_module__module__deleted_at__isnull=True),
),
Value([], output_field=ArrayField(UUIDField())),
),
Expand Down
8 changes: 6 additions & 2 deletions apiserver/plane/app/views/view/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,10 @@ def get_queryset(self):
ArrayAgg(
"labels__id",
distinct=True,
filter=~Q(labels__id__isnull=True),
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
),
Comment on lines +237 to +240
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Suggestion: Also filter out archived labels in label_ids aggregation

Currently, the label_ids aggregation filters out deleted labels but not archived ones. For consistency with modules and to ensure that only active labels are included, consider adding a filter for archived labels.

Apply this change to include the archived filter:

 filter=(
     ~Q(labels__id__isnull=True)
     & Q(labels__deleted_at__isnull=True)
+    & Q(labels__archived_at__isnull=True)
 ),
📝 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.

Suggested change
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
),
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
& Q(labels__archived_at__isnull=True)
),

),
Value([], output_field=ArrayField(UUIDField())),
),
Expand All @@ -252,7 +255,8 @@ def get_queryset(self):
"issue_module__module_id",
distinct=True,
filter=~Q(issue_module__module_id__isnull=True)
& Q(issue_module__module__archived_at__isnull=True),
& Q(issue_module__module__archived_at__isnull=True)
& Q(issue_module__module__deleted_at__isnull=True),
),
Value([], output_field=ArrayField(UUIDField())),
),
Expand Down
10 changes: 6 additions & 4 deletions apiserver/plane/app/views/workspace/draft.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ def get_queryset(self):
ArrayAgg(
"labels__id",
distinct=True,
filter=~Q(labels__id__isnull=True),
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
),
),
Value([], output_field=ArrayField(UUIDField())),
),
Expand All @@ -78,9 +81,8 @@ def get_queryset(self):
"draft_issue_module__module_id",
distinct=True,
filter=~Q(draft_issue_module__module_id__isnull=True)
& Q(
draft_issue_module__module__archived_at__isnull=True
),
& Q(draft_issue_module__module__archived_at__isnull=True)
& Q(draft_issue_module__module__deleted_at__isnull=True),
),
Value([], output_field=ArrayField(UUIDField())),
),
Expand Down
9 changes: 7 additions & 2 deletions apiserver/plane/space/views/issue.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,10 @@ def get(self, request, anchor, issue_id):
ArrayAgg(
"labels__id",
distinct=True,
filter=~Q(labels__id__isnull=True),
filter=(
~Q(labels__id__isnull=True)
& Q(labels__deleted_at__isnull=True)
),
),
Value([], output_field=ArrayField(UUIDField())),
),
Expand All @@ -718,7 +721,9 @@ def get(self, request, anchor, issue_id):
ArrayAgg(
"issue_module__module_id",
distinct=True,
filter=~Q(issue_module__module_id__isnull=True),
filter=~Q(issue_module__module_id__isnull=True)
& Q(issue_module__module__archived_at__isnull=True)
& Q(issue_module__module__deleted_at__isnull=True),
),
Value([], output_field=ArrayField(UUIDField())),
),
Expand Down
6 changes: 5 additions & 1 deletion apiserver/plane/utils/grouper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,16 @@ def issue_queryset_grouper(queryset, group_by, sub_group_by):

annotations_map = {
"assignee_ids": ("assignees__id", ~Q(assignees__id__isnull=True)),
"label_ids": ("labels__id", ~Q(labels__id__isnull=True)),
"label_ids": (
"labels__id",
~Q(labels__id__isnull=True) & (Q(labels__deleted_at__isnull=True)),
),
"module_ids": (
"issue_module__module_id",
(
~Q(issue_module__module_id__isnull=True)
& Q(issue_module__module__archived_at__isnull=True)
& Q(issue_module__module__deleted_at__isnull=True)
),
),
}
Expand Down
Loading