Skip to content

[WEB-2388] chore: workspace draft issue improvements #5808

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

Merged
sriramveeraghanta merged 1 commit into from
Oct 11, 2024
Merged

[WEB-2388] chore: workspace draft issue improvements #5808

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
2 changes: 2 additions & 0 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 @@ -147,6 +147,8 @@ def create(self, request, slug):
"updated_at",
"created_by",
"updated_by",
"type_id",
"description_html",
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 11, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure description_html is properly sanitized before including in the response.

To prevent potential XSS vulnerabilities, verify that description_html is sanitized before being sent to the client. This helps protect against any malicious content that may have been injected.

)
.first()
)
Expand Down
3 changes: 3 additions & 0 deletions web/core/components/issues/issue-modal/base.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export const CreateUpdateIssueModalBase: React.FC<IssuesModalProps> = observer((
storeType: issueStoreFromProps,
isDraft = false,
fetchIssueDetails = true,
moveToIssue = false,
} = props;
const issueStoreType = useIssueStoreType();

Expand Down Expand Up @@ -308,6 +309,7 @@ export const CreateUpdateIssueModalBase: React.FC<IssuesModalProps> = observer((
isCreateMoreToggleEnabled={createMore}
onCreateMoreToggleChange={handleCreateMoreToggleChange}
isDraft={isDraft}
moveToIssue={moveToIssue}
/>
) : (
<IssueFormRoot
Expand All @@ -324,6 +326,7 @@ export const CreateUpdateIssueModalBase: React.FC<IssuesModalProps> = observer((
onSubmit={(payload) => handleFormSubmit(payload, isDraft)}
projectId={activeProjectId}
isDraft={isDraft}
moveToIssue={moveToIssue}
/>
)}
</ModalCore>
Expand Down
3 changes: 3 additions & 0 deletions web/core/components/issues/issue-modal/draft-issue-layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export interface DraftIssueProps {
onSubmit: (formData: Partial<TIssue>, is_draft_issue?: boolean) => Promise<void>;
projectId: string;
isDraft: boolean;
moveToIssue?: boolean;
}

export const DraftIssueLayout: React.FC<DraftIssueProps> = observer((props) => {
Expand All @@ -43,6 +44,7 @@ export const DraftIssueLayout: React.FC<DraftIssueProps> = observer((props) => {
isCreateMoreToggleEnabled,
onCreateMoreToggleChange,
isDraft,
moveToIssue = false,
} = props;
// states
const [issueDiscardModal, setIssueDiscardModal] = useState(false);
Expand Down Expand Up @@ -156,6 +158,7 @@ export const DraftIssueLayout: React.FC<DraftIssueProps> = observer((props) => {
onSubmit={onSubmit}
projectId={projectId}
isDraft={isDraft}
moveToIssue={moveToIssue}
/>
</>
);
Expand Down
22 changes: 19 additions & 3 deletions web/core/components/issues/issue-modal/form.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { useForm } from "react-hook-form";
// editor
import { EditorRefApi } from "@plane/editor";
// types
import type { TIssue, ISearchIssueResponse } from "@plane/types";
import type { TIssue, ISearchIssueResponse, TWorkspaceDraftIssue } from "@plane/types";
// hooks
import { Button, ToggleSwitch, TOAST_TYPE, setToast } from "@plane/ui";
// components
Expand All @@ -26,7 +26,7 @@ import { getChangedIssuefields } from "@/helpers/issue.helper";
import { getTabIndex } from "@/helpers/tab-indices.helper";
// hooks
import { useIssueModal } from "@/hooks/context/use-issue-modal";
import { useIssueDetail, useProject, useProjectState } from "@/hooks/store";
import { useIssueDetail, useProject, useProjectState, useWorkspaceDraftIssues } from "@/hooks/store";
import { usePlatformOS } from "@/hooks/use-platform-os";
import { useProjectIssueProperties } from "@/hooks/use-project-issue-properties";
// plane web components
Expand Down Expand Up @@ -59,6 +59,7 @@ export interface IssueFormProps {
onSubmit: (values: Partial<TIssue>, is_draft_issue?: boolean) => Promise<void>;
projectId: string;
isDraft: boolean;
moveToIssue?: boolean;
}

export const IssueFormRoot: FC<IssueFormProps> = observer((props) => {
Expand All @@ -72,6 +73,7 @@ export const IssueFormRoot: FC<IssueFormProps> = observer((props) => {
isCreateMoreToggleEnabled,
onCreateMoreToggleChange,
isDraft,
moveToIssue,
} = props;

// states
Expand All @@ -91,6 +93,7 @@ export const IssueFormRoot: FC<IssueFormProps> = observer((props) => {
const { getIssueTypeIdOnProjectChange, getActiveAdditionalPropertiesLength, handlePropertyValuesValidation } =
useIssueModal();
const { isMobile } = usePlatformOS();
const { moveIssue } = useWorkspaceDraftIssues();

const {
issue: { getIssueById },
Expand Down Expand Up @@ -400,7 +403,7 @@ export const IssueFormRoot: FC<IssueFormProps> = observer((props) => {
Discard
</Button>
<Button
variant="primary"
variant={moveToIssue ? "neutral-primary" : "primary"}
type="submit"
size="sm"
ref={submitBtnRef}
Expand All @@ -417,6 +420,19 @@ export const IssueFormRoot: FC<IssueFormProps> = observer((props) => {
? "Create draft issue"
: "Create"}
</Button>
{moveToIssue && (
<Button
variant="primary"
type="button"
size="sm"
loading={isSubmitting}
onClick={() =>
Comment on lines +428 to +429
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 11, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Use Dedicated Loading State for the "Add to project" Button

The loading prop for the "Add to project" button uses isSubmitting, which is associated with form submission. This may not accurately reflect the loading state of the moveIssue action, potentially causing confusion.

Introduce a separate loading state for the moveIssue action to ensure accurate feedback to the user:

+const [isMovingIssue, setIsMovingIssue] = useState(false);

...

<Button
  variant="primary"
  type="button"
  size="sm"
- loading={isSubmitting}
+ loading={isMovingIssue}
  onClick={async () => {
+   setIsMovingIssue(true);
    if (data?.id && data) {
-     moveIssue(workspaceSlug.toString(), data.id, data as TWorkspaceDraftIssue);
+     await moveIssue(workspaceSlug.toString(), data.id, data as TWorkspaceDraftIssue);
    }
+   setIsMovingIssue(false);
  }}
>
  Add to project
</Button>

Committable suggestion was skipped due to low confidence.

data?.id && data && moveIssue(workspaceSlug.toString(), data?.id, data as TWorkspaceDraftIssue)
}
Comment on lines +429 to +431
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 11, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add Error Handling to the moveIssue Operation

Currently, the moveIssue function call lacks error handling, which could result in unhandled exceptions and a poor user experience if the operation fails.

Wrap the moveIssue function call in a try-catch block to handle potential errors and provide feedback to the user:

onClick={async () => {
  setIsMovingIssue(true);
  try {
    if (data?.id && data) {
      await moveIssue(workspaceSlug.toString(), data.id, data as TWorkspaceDraftIssue);
    }
  } catch (error) {
    setToast({
      type: TOAST_TYPE.ERROR,
      title: "Error!",
      message: "Failed to move the issue to the project. Please try again.",
    });
  } finally {
    setIsMovingIssue(false);
  }
}}

Committable suggestion was skipped due to low confidence.

>
Add to project
</Button>
)}
Comment on lines +423 to +435
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 11, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure Type Safety When Casting data to TWorkspaceDraftIssue

In the onClick handler for the "Add to project" button, data is cast to TWorkspaceDraftIssue without proper validation. Since data is of type Partial<TIssue>, directly casting it may lead to runtime errors if required fields are missing. It's important to ensure that data conforms to TWorkspaceDraftIssue before passing it to moveIssue.

To address this, explicitly construct a TWorkspaceDraftIssue object with the necessary fields:

onClick={() => {
  if (data?.id && data) {
-   moveIssue(workspaceSlug.toString(), data.id, data as TWorkspaceDraftIssue);
+   const draftIssue: TWorkspaceDraftIssue = {
+     id: data.id,
+     // Add all required fields here, ensuring they come from 'data' and are not undefined
+     // For example:
+     name: data.name ?? '',
+     description_html: data.description_html ?? '<p></p>',
+     // Include other necessary fields
+   };
+   moveIssue(workspaceSlug.toString(), data.id, draftIssue);
  }
}}

Committable suggestion was skipped due to low confidence.

</div>
</div>
</div>
Expand Down
1 change: 1 addition & 0 deletions web/core/components/issues/issue-modal/modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export interface IssuesModalProps {
storeType?: EIssuesStoreType;
isDraft?: boolean;
fetchIssueDetails?: boolean;
moveToIssue?: boolean;
}

export const CreateUpdateIssueModal: React.FC<IssuesModalProps> = observer(
Expand Down
11 changes: 10 additions & 1 deletion web/core/components/issues/workspace-draft/quick-action.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export const WorkspaceDraftIssueQuickActions: React.FC<IQuickActionProps> = obse
parentRef,
} = props;
// states
const [moveToIssue, setMoveToIssue] = useState(false);
const [createUpdateIssueModal, setCreateUpdateIssueModal] = useState(false);
const [issueToEdit, setIssueToEdit] = useState<TWorkspaceDraftIssue | undefined>(undefined);
const [deleteIssueModal, setDeleteIssueModal] = useState(false);
Expand Down Expand Up @@ -77,7 +78,13 @@ export const WorkspaceDraftIssueQuickActions: React.FC<IQuickActionProps> = obse
key: "move-to-issues",
title: "Move to issues",
icon: SquareStackIcon,
action: () => handleMoveToIssues && handleMoveToIssues(),
action: () => {
if (handleMoveToIssues) {
setMoveToIssue(true);
setIssueToEdit(issue);
setCreateUpdateIssueModal(true);
}
},
},
{
key: "delete",
Expand All @@ -102,13 +109,15 @@ export const WorkspaceDraftIssueQuickActions: React.FC<IQuickActionProps> = obse
onClose={() => {
setCreateUpdateIssueModal(false);
setIssueToEdit(undefined);
setMoveToIssue(false);
}}
data={issueToEdit ?? duplicateIssuePayload}
onSubmit={async (data) => {
if (issueToEdit && handleUpdate) await handleUpdate(data as TWorkspaceDraftIssue);
}}
storeType={EIssuesStoreType.WORKSPACE_DRAFT}
fetchIssueDetails={false}
moveToIssue={moveToIssue}
isDraft
/>
<ContextMenu parentRef={parentRef} items={MENU_ITEMS} />
Expand Down