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.

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-2388] dev: workspace draft issues #5772

Merged
merged 26 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
9f61c30
chore: workspace draft page added
anmolsinghbhatia Oct 8, 2024
76b2308
chore: workspace draft issues services added
anmolsinghbhatia Oct 8, 2024
d1fceda
Merge branch 'preview' of github.com:makeplane/plane into dev-workspa…
anmolsinghbhatia Oct 8, 2024
2359b5c
chore: workspace draft issue store added
anmolsinghbhatia Oct 8, 2024
3bba839
chore: workspace draft issue filter store added
anmolsinghbhatia Oct 8, 2024
2fba20c
WIP: 76b23084e chore: workspace draft issues services added
anmolsinghbhatia Oct 8, 2024
bdbc763
chore: issue rendering
anmolsinghbhatia Oct 8, 2024
0cd0f6d
Merge branch 'preview' of gurusainath:makeplane/plane into dev-worksp…
gurusainath Oct 8, 2024
623ada4
conflicts: resolved merge conflicts
gurusainath Oct 8, 2024
4520959
conflicts: handled draft issue store
gurusainath Oct 8, 2024
ff4b2c1
chore: draft issue modal
anmolsinghbhatia Oct 8, 2024
90a0fca
chore: code optimisation
NarayanBavisetti Oct 9, 2024
d404671
chore: ui changes
gurusainath Oct 9, 2024
abaf8cd
Merge branch 'dev-workspace-draft-issues' of gurusainath:makeplane/pl…
gurusainath Oct 9, 2024
131519a
chore: workspace draft store and modal updated
anmolsinghbhatia Oct 9, 2024
83ed3fb
chore: workspace draft issue component added
anmolsinghbhatia Oct 9, 2024
11d1c5f
chore: updated store and workflow in draft issues
gurusainath Oct 10, 2024
ba8b20b
chore: updated issue draft store
gurusainath Oct 10, 2024
8fab9ed
chore: updated issue type cleanup in components
gurusainath Oct 10, 2024
d7d8f99
chore: code refactor
anmolsinghbhatia Oct 10, 2024
6f90514
fix: build error
anmolsinghbhatia Oct 10, 2024
54c22b3
fix: quick actions
anmolsinghbhatia Oct 10, 2024
88a9fb0
Merge branch 'preview' of github.com:makeplane/plane into dev-workspa…
anmolsinghbhatia Oct 10, 2024
684d5fc
fix: update mutation
anmolsinghbhatia Oct 10, 2024
0dd4cbe
fix: create update modal
anmolsinghbhatia Oct 10, 2024
a9f4294
chore: commented project draft issue code
anmolsinghbhatia Oct 10, 2024
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
172 changes: 46 additions & 126 deletions apiserver/plane/app/views/workspace/draft.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,11 @@


class WorkspaceDraftIssueViewSet(BaseViewSet):

model = DraftIssue

@method_decorator(gzip_page)
@allow_permission(
allowed_roles=[ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST], level="WORKSPACE"
)
def list(self, request, slug):
filters = issue_filters(request.query_params, "GET")
issues = (
DraftIssue.objects.filter(workspace__slug=slug)
.filter(created_by=request.user)
def get_queryset(self):
return (
DraftIssue.objects.filter(workspace__slug=self.kwargs.get("slug"))
.select_related("workspace", "project", "state", "parent")
.prefetch_related(
"assignees", "labels", "draft_issue_module__module"
Expand Down Expand Up @@ -91,6 +84,17 @@ def list(self, request, slug):
Value([], output_field=ArrayField(UUIDField())),
),
)
).distinct()

@method_decorator(gzip_page)
@allow_permission(
allowed_roles=[ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST], level="WORKSPACE"
)
def list(self, request, slug):
filters = issue_filters(request.query_params, "GET")
issues = (
self.get_queryset()
.filter(created_by=request.user)
.order_by("-created_at")
)

Expand Down Expand Up @@ -120,7 +124,34 @@ def create(self, request, slug):
)
if serializer.is_valid():
serializer.save()
return Response(serializer.data, status=status.HTTP_201_CREATED)
issue = (
self.get_queryset()
.filter(pk=serializer.data.get("id"))
.values(
"id",
"name",
"state_id",
"sort_order",
"completed_at",
"estimate_point",
"priority",
"start_date",
"target_date",
"project_id",
"parent_id",
"cycle_id",
"module_ids",
"label_ids",
"assignee_ids",
"created_at",
"updated_at",
"created_by",
"updated_by",
)
.first()
)

return Response(issue, status=status.HTTP_201_CREATED)
return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)

@allow_permission(
Expand All @@ -131,45 +162,7 @@ def create(self, request, slug):
)
def partial_update(self, request, slug, pk):
issue = (
DraftIssue.objects.filter(workspace__slug=slug)
.filter(pk=pk)
.filter(created_by=request.user)
.select_related("workspace", "project", "state", "parent")
.prefetch_related(
"assignees", "labels", "draft_issue_module__module"
)
.annotate(cycle_id=F("draft_issue_cycle__cycle_id"))
.annotate(
label_ids=Coalesce(
ArrayAgg(
"labels__id",
distinct=True,
filter=~Q(labels__id__isnull=True),
),
Value([], output_field=ArrayField(UUIDField())),
),
assignee_ids=Coalesce(
ArrayAgg(
"assignees__id",
distinct=True,
filter=~Q(assignees__id__isnull=True)
& Q(assignees__member_project__is_active=True),
),
Value([], output_field=ArrayField(UUIDField())),
),
module_ids=Coalesce(
ArrayAgg(
"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
),
),
Value([], output_field=ArrayField(UUIDField())),
),
)
.first()
self.get_queryset().filter(pk=pk, created_by=request.user).first()
)

if not issue:
Expand Down Expand Up @@ -202,46 +195,8 @@ def partial_update(self, request, slug, pk):
)
def retrieve(self, request, slug, pk=None):
issue = (
DraftIssue.objects.filter(workspace__slug=slug)
.filter(pk=pk)
.filter(created_by=request.user)
.select_related("workspace", "project", "state", "parent")
.prefetch_related(
"assignees", "labels", "draft_issue_module__module"
)
.annotate(cycle_id=F("draft_issue_cycle__cycle_id"))
.filter(pk=pk)
.annotate(
label_ids=Coalesce(
ArrayAgg(
"labels__id",
distinct=True,
filter=~Q(labels__id__isnull=True),
),
Value([], output_field=ArrayField(UUIDField())),
),
assignee_ids=Coalesce(
ArrayAgg(
"assignees__id",
distinct=True,
filter=~Q(assignees__id__isnull=True)
& Q(assignees__member_project__is_active=True),
),
Value([], output_field=ArrayField(UUIDField())),
),
module_ids=Coalesce(
ArrayAgg(
"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
),
),
Value([], output_field=ArrayField(UUIDField())),
),
)
).first()
self.get_queryset().filter(pk=pk, created_by=request.user).first()
)

if not issue:
return Response(
Expand All @@ -268,42 +223,7 @@ def destroy(self, request, slug, pk=None):
level="WORKSPACE",
)
def create_draft_to_issue(self, request, slug, draft_id):
draft_issue = (
DraftIssue.objects.filter(workspace__slug=slug, pk=draft_id)
.annotate(cycle_id=F("draft_issue_cycle__cycle_id"))
.annotate(
label_ids=Coalesce(
ArrayAgg(
"labels__id",
distinct=True,
filter=~Q(labels__id__isnull=True),
),
Value([], output_field=ArrayField(UUIDField())),
),
assignee_ids=Coalesce(
ArrayAgg(
"assignees__id",
distinct=True,
filter=~Q(assignees__id__isnull=True)
& Q(assignees__member_project__is_active=True),
),
Value([], output_field=ArrayField(UUIDField())),
),
module_ids=Coalesce(
ArrayAgg(
"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
),
),
Value([], output_field=ArrayField(UUIDField())),
),
)
.select_related("project", "workspace")
.first()
)
draft_issue = self.get_queryset().filter(pk=draft_id).first()
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

Add created_by filter in create_draft_to_issue for security.

In the create_draft_to_issue method, the draft_issue is retrieved without filtering by created_by=request.user. This could allow users to act on draft issues they did not create. To prevent unauthorized access, include the created_by filter.

Apply this diff to secure the query:

 def create_draft_to_issue(self, request, slug, draft_id):
-    draft_issue = self.get_queryset().filter(pk=draft_id).first()
+    draft_issue = self.get_queryset().filter(pk=draft_id, created_by=request.user).first()
📝 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
draft_issue = self.get_queryset().filter(pk=draft_id).first()
draft_issue = self.get_queryset().filter(pk=draft_id, created_by=request.user).first()


if not draft_issue.project_id:
return Response(
Expand Down
5 changes: 3 additions & 2 deletions apiserver/plane/utils/paginator.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ def get_result(self, limit=1000, cursor=None):
raise BadPaginationError("Pagination offset cannot be negative")

results = queryset[offset:stop]

print(limit, "limit")
if cursor.value != limit:
results = results[-(limit + 1) :]
Comment on lines +153 to 155
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

Remove debug print and review slicing logic

  1. The print statement on line 153 should be removed before merging to production.

  2. The modification to the slicing logic on lines 154-155 needs careful review:

    if cursor.value != limit:
        results = results[-(limit + 1):]

    This change might lead to unexpected behavior if cursor.value is significantly different from limit. It could potentially return fewer results than expected or cause issues with pagination consistency.

Consider removing the print statement and reverting the slicing logic to its previous implementation unless there's a specific reason for this change that's not apparent from the context provided.


Expand Down Expand Up @@ -761,7 +761,7 @@ def paginate(
):
"""Paginate the request"""
per_page = self.get_per_page(request, default_per_page, max_per_page)

print(per_page, "per_page")
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

Replace debug prints with proper logging

There are two print statements added for debugging purposes:

  1. Line 764: print(per_page, "per_page")
  2. Line 791: print(per_page, "per_page 2")

These should be removed or replaced with proper logging before merging to production.

Replace the print statements with appropriate logging:

import logging

logger = logging.getLogger(__name__)

# ... (in the paginate method)
logger.debug(f"Initial per_page value: {per_page}")

# ... (before calling paginator.get_result)
logger.debug(f"per_page value before get_result: {per_page}")

This approach allows for better control over log levels and output in different environments.

Also applies to: 791-791

# Convert the cursor value to integer and float from string
input_cursor = None
try:
Expand All @@ -788,6 +788,7 @@ def paginate(
paginator = paginator_cls(**paginator_kwargs)

try:
print(per_page, "per_page 2")
cursor_result = paginator.get_result(
limit=per_page, cursor=input_cursor
)
Expand Down
1 change: 1 addition & 0 deletions packages/types/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,4 @@ export * from "./pragmatic";
export * from "./publish";
export * from "./workspace-notifications";
export * from "./favorite";
export * from "./workspace-draft-issues/base";
1 change: 1 addition & 0 deletions packages/types/src/issues/base.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export * from "./issue_relation";
export * from "./issue_sub_issues";
export * from "./activity/base";


export type TLoader =
| "init-loader"
| "mutation"
Expand Down
61 changes: 61 additions & 0 deletions packages/types/src/workspace-draft-issues/base.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { TIssuePriorities } from "../issues";

export type TWorkspaceDraftIssue = {
id: string;
name: string;
sort_order: number;

state_id: string | undefined;
priority: TIssuePriorities | undefined;
label_ids: string[];
assignee_ids: string[];
estimate_point: string | undefined;

project_id: string | undefined;
parent_id: string | undefined;
cycle_id: string | undefined;
module_ids: string[] | undefined;

start_date: string | undefined;
target_date: string | undefined;
completed_at: string | undefined;

created_at: string;
updated_at: string;
created_by: string;
updated_by: string;

is_draft: boolean;
};

export type TWorkspaceDraftPaginationInfo<T> = {
next_cursor: string | undefined;
prev_cursor: string | undefined;
next_page_results: boolean | undefined;
prev_page_results: boolean | undefined;
total_pages: number | undefined;
count: number | undefined; // current paginated results count
total_count: number | undefined; // total available results count
total_results: number | undefined;
results: T[] | undefined;
extra_stats: string | undefined;
grouped_by: string | undefined;
sub_grouped_by: string | undefined;
};

export type TWorkspaceDraftQueryParams = {
per_page: number;
cursor: string;
};

export type TWorkspaceDraftIssueLoader =
| "init-loader"
| "empty-state"
| "mutation"
| "pagination"
| "loaded"
| "create"
| "update"
| "delete"
| "move"
| undefined;
62 changes: 62 additions & 0 deletions web/app/[workspaceSlug]/(projects)/drafts/header.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
"use client";

import { FC, useState } from "react";
import { observer } from "mobx-react";
import { PenSquare } from "lucide-react";
// ui
import { Breadcrumbs, Button, Header } from "@plane/ui";
// components
import { BreadcrumbLink } from "@/components/common";
import { CreateUpdateIssueModal } from "@/components/issues";
// constants
import { EIssuesStoreType } from "@/constants/issue";
// hooks
import { useUserPermissions } from "@/hooks/store";
// plane-web
import { EUserPermissions, EUserPermissionsLevel } from "@/plane-web/constants/user-permissions";

export const WorkspaceDraftHeader: FC = observer(() => {
// state
const [isDraftIssueModalOpen, setIsDraftIssueModalOpen] = useState(false);
// store hooks
const { allowPermissions } = useUserPermissions();

// check if user is authorized to create draft issue
const isAuthorizedUser = allowPermissions(
[EUserPermissions.ADMIN, EUserPermissions.MEMBER],
EUserPermissionsLevel.WORKSPACE
);

return (
<>
<CreateUpdateIssueModal
isOpen={isDraftIssueModalOpen}
storeType={EIssuesStoreType.WORKSPACE_DRAFT}
onClose={() => setIsDraftIssueModalOpen(false)}
isDraft
/>
<Header>
<Header.LeftItem>
<Breadcrumbs>
<Breadcrumbs.BreadcrumbItem
type="text"
link={<BreadcrumbLink label={`Draft`} icon={<PenSquare className="h-4 w-4 text-custom-text-300" />} />}
/>
</Breadcrumbs>
</Header.LeftItem>

<Header.RightItem>
<Button
variant="primary"
size="sm"
className="items-center gap-1"
onClick={() => setIsDraftIssueModalOpen(true)}
disabled={!isAuthorizedUser}
>
Draft <span className="hidden sm:inline-block">issue</span>
</Button>
</Header.RightItem>
</Header>
</>
);
});
13 changes: 13 additions & 0 deletions web/app/[workspaceSlug]/(projects)/drafts/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
"use client";

import { AppHeader, ContentWrapper } from "@/components/core";
import { WorkspaceDraftHeader } from "./header";

export default function WorkspaceDraftLayout({ children }: { children: React.ReactNode }) {
return (
<>
<AppHeader header={<WorkspaceDraftHeader />} />
<ContentWrapper>{children}</ContentWrapper>
</>
);
}
Loading
Loading