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

fix: added workspaceslug in renderChildren of project settings #5951

Merged
merged 5 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion packages/types/src/inbox.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ export type TInboxIssueWithPagination = TInboxIssuePaginationInfo & {
export type TInboxForm = {
anchor: string;
id: string;
is_disabled: boolean;
is_in_app_enabled: boolean;
is_form_enabled: boolean;
};

export type TInboxIssueForm = {
Expand Down
41 changes: 41 additions & 0 deletions space/core/components/editor/rich-text-editor.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import React, { forwardRef } from "react";
// editor
import { EditorRefApi, IMentionHighlight, IRichTextEditor, RichTextEditorWithRef } from "@plane/editor";
// types
// helpers
import { cn } from "@/helpers/common.helper";
import { getEditorFileHandlers } from "@/helpers/editor.helper";

interface RichTextEditorWrapperProps extends Omit<IRichTextEditor, "fileHandler" | "mentionHandler"> {
uploadFile: (file: File) => Promise<string>;
}

export const RichTextEditor = forwardRef<EditorRefApi, RichTextEditorWrapperProps>((props, ref) => {
const { containerClassName, uploadFile, ...rest } = props;
// store hooks

// use-mention

// file size

return (
<RichTextEditorWithRef
mentionHandler={{
highlights: function (): Promise<IMentionHighlight[]> {
throw new Error("Function not implemented.");
},
suggestions: undefined,
}}
Comment on lines +23 to +28
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

Implement mention handling instead of throwing an error.

The current implementation throws an error which could cause runtime issues. Either implement the mention handling functionality or return an empty array as a safer default.

 mentionHandler={{
   highlights: function (): Promise<IMentionHighlight[]> {
-    throw new Error("Function not implemented.");
+    return Promise.resolve([]); // Safe default until implementation is ready
   },
   suggestions: undefined,
 }}
📝 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
mentionHandler={{
highlights: function (): Promise<IMentionHighlight[]> {
throw new Error("Function not implemented.");
},
suggestions: undefined,
}}
mentionHandler={{
highlights: function (): Promise<IMentionHighlight[]> {
return Promise.resolve([]); // Safe default until implementation is ready
},
suggestions: undefined,
}}

ref={ref}
fileHandler={getEditorFileHandlers({
uploadFile,
workspaceId: "",
anchor: "",
})}
Comment on lines +30 to +34
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Empty workspaceId and anchor parameters will cause file operations to fail

Based on the editor helper implementation, both workspaceId and anchor parameters are required for:

  • Generating asset source URLs via getEditorAssetSrc
  • Deleting assets via fileService.deleteOldEditorAsset and fileService.deleteNewAsset
  • Restoring assets via fileService.restoreOldEditorAsset and fileService.restoreNewAsset

The RichTextEditor component should:

  • Accept workspaceId and anchor as props
  • Pass these values to getEditorFileHandlers instead of empty strings

Example locations requiring fixes:

  • space/core/components/editor/rich-text-editor.tsx: Add props and pass them to file handler
🔗 Analysis chain

Verify workspaceId and anchor parameters.

Empty strings are being passed for workspaceId and anchor. These should likely be provided through props or context.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if these parameters are used meaningfully in the editor file handlers
rg -A 5 "workspaceId|anchor" "$(fd -t f "editor.helper")"

Length of output: 217


Script:

#!/bin/bash
# Let's check the content of the editor helper file
cat space/helpers/editor.helper.ts

Length of output: 2257


Script:

#!/bin/bash
# Let's also check where getEditorFileHandlers is used and its implementation
ast-grep --pattern 'getEditorFileHandlers'

Length of output: 1417


Script:

#!/bin/bash
# Search for any file upload related functionality
rg -A 5 "uploadFile.*workspaceId|anchor.*upload" 

Length of output: 774


Script:

#!/bin/bash
# Let's check the implementation of rich-text-editor.tsx to see how these parameters should be passed
cat space/core/components/editor/rich-text-editor.tsx

Length of output: 1232


Script:

#!/bin/bash
# Let's also check other implementations to see how they handle these parameters
rg -l "workspaceId.*anchor.*uploadFile" space/core/components/editor/

Length of output: 71

{...rest}
containerClassName={cn("relative pl-3 pb-3", containerClassName)}
/>
);
});
Comment on lines +13 to +39
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

Add error handling for file uploads.

The uploadFile function is used directly without error handling. Consider wrapping it in a try-catch block and showing appropriate error messages to users.

 export const RichTextEditor = forwardRef<EditorRefApi, RichTextEditorWrapperProps>((props, ref) => {
   const { containerClassName, uploadFile, ...rest } = props;
-  // store hooks
-
-  // use-mention
-
-  // file size

+  const handleFileUpload = async (file: File): Promise<string> => {
+    try {
+      return await uploadFile(file);
+    } catch (error) {
+      console.error('File upload failed:', error);
+      throw new Error('Failed to upload file. Please try again.');
+    }
+  };

   return (
     <RichTextEditorWithRef
       mentionHandler={{...}}
       ref={ref}
       fileHandler={getEditorFileHandlers({
-        uploadFile,
+        uploadFile: handleFileUpload,
         workspaceId: "",
         anchor: "",
       })}
       {...rest}
       containerClassName={cn("relative pl-3 pb-3", containerClassName)}
     />
   );
 });
📝 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
export const RichTextEditor = forwardRef<EditorRefApi, RichTextEditorWrapperProps>((props, ref) => {
const { containerClassName, uploadFile, ...rest } = props;
// store hooks
// use-mention
// file size
return (
<RichTextEditorWithRef
mentionHandler={{
highlights: function (): Promise<IMentionHighlight[]> {
throw new Error("Function not implemented.");
},
suggestions: undefined,
}}
ref={ref}
fileHandler={getEditorFileHandlers({
uploadFile,
workspaceId: "",
anchor: "",
})}
{...rest}
containerClassName={cn("relative pl-3 pb-3", containerClassName)}
/>
);
});
export const RichTextEditor = forwardRef<EditorRefApi, RichTextEditorWrapperProps>((props, ref) => {
const { containerClassName, uploadFile, ...rest } = props;
const handleFileUpload = async (file: File): Promise<string> => {
try {
return await uploadFile(file);
} catch (error) {
console.error('File upload failed:', error);
throw new Error('Failed to upload file. Please try again.');
}
};
return (
<RichTextEditorWithRef
mentionHandler={{
highlights: function (): Promise<IMentionHighlight[]> {
throw new Error("Function not implemented.");
},
suggestions: undefined,
}}
ref={ref}
fileHandler={getEditorFileHandlers({
uploadFile: handleFileUpload,
workspaceId: "",
anchor: "",
})}
{...rest}
containerClassName={cn("relative pl-3 pb-3", containerClassName)}
/>
);
});


RichTextEditor.displayName = "RichTextEditor";
19 changes: 19 additions & 0 deletions space/core/store/issue-detail.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export interface IIssueDetailStore {
updateIssueComment: (anchor: string, issueID: string, commentID: string, data: any) => Promise<any>;
deleteIssueComment: (anchor: string, issueID: string, commentID: string) => void;
uploadCommentAsset: (file: File, anchor: string, commentID?: string) => Promise<TFileSignedURLResponse>;
uploadIssueAsset: (file: File, anchor: string, commentID?: string) => Promise<TFileSignedURLResponse>;
addCommentReaction: (anchor: string, issueID: string, commentID: string, reactionHex: string) => void;
removeCommentReaction: (anchor: string, issueID: string, commentID: string, reactionHex: string) => void;
// reaction actions
Expand Down Expand Up @@ -79,6 +80,7 @@ export class IssueDetailStore implements IIssueDetailStore {
updateIssueComment: action,
deleteIssueComment: action,
uploadCommentAsset: action,
uploadIssueAsset: action,
addCommentReaction: action,
removeCommentReaction: action,
// reaction actions
Expand Down Expand Up @@ -245,6 +247,23 @@ export class IssueDetailStore implements IIssueDetailStore {
}
};

uploadIssueAsset = async (file: File, anchor: string, commentID?: string) => {
try {
const res = await this.fileService.uploadAsset(
anchor,
{
entity_identifier: commentID ?? "",
entity_type: EFileAssetType.ISSUE_ATTACHMENT,
},
file
);
return res;
} catch (error) {
console.log("Error in uploading comment asset:", error);
throw new Error("Asset upload failed. Please try again later.");
}
};
Comment on lines +250 to +265
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

Improve error handling and message accuracy.

Several improvements can be made to enhance reliability and maintainability:

  1. The error message incorrectly refers to "comment asset" instead of "issue asset"
  2. The commentID parameter usage could be more explicit
  3. Error handling could be more specific

Consider applying these changes:

 uploadIssueAsset = async (file: File, anchor: string, commentID?: string) => {
   try {
     const res = await this.fileService.uploadAsset(
       anchor,
       {
-        entity_identifier: commentID ?? "",
+        entity_identifier: commentID || "",
         entity_type: EFileAssetType.ISSUE_ATTACHMENT,
       },
       file
     );
     return res;
   } catch (error) {
-    console.log("Error in uploading comment asset:", error);
+    console.error("Error in uploading issue asset:", error);
+    if (error instanceof Error) {
+      throw error;
+    }
     throw new Error("Asset upload failed. Please try again later.");
   }
 };

Additionally, consider adding JSDoc documentation to clarify the purpose of the commentID parameter and when it should be provided.

📝 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
uploadIssueAsset = async (file: File, anchor: string, commentID?: string) => {
try {
const res = await this.fileService.uploadAsset(
anchor,
{
entity_identifier: commentID ?? "",
entity_type: EFileAssetType.ISSUE_ATTACHMENT,
},
file
);
return res;
} catch (error) {
console.log("Error in uploading comment asset:", error);
throw new Error("Asset upload failed. Please try again later.");
}
};
uploadIssueAsset = async (file: File, anchor: string, commentID?: string) => {
try {
const res = await this.fileService.uploadAsset(
anchor,
{
entity_identifier: commentID || "",
entity_type: EFileAssetType.ISSUE_ATTACHMENT,
},
file
);
return res;
} catch (error) {
console.error("Error in uploading issue asset:", error);
if (error instanceof Error) {
throw error;
}
throw new Error("Asset upload failed. Please try again later.");
}
};


addCommentReaction = async (anchor: string, issueID: string, commentID: string, reactionHex: string) => {
const newReaction = {
id: uuidv4(),
Expand Down
6 changes: 6 additions & 0 deletions space/core/types/intake.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export type TIntakeIssueForm = {
name: string;
email: string;
username: string;
description_html: string;
};
3 changes: 2 additions & 1 deletion web/ce/constants/project/settings/features.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ export type TProperties = {
renderChildren?: (
currentProjectDetails: IProject,
isAdmin: boolean,
handleSubmit: (featureKey: string, featureProperty: string) => Promise<void>
handleSubmit: (featureKey: string, featureProperty: string) => Promise<void>,
workspaceSlug: string
) => ReactNode;
};
export type TFeatureList = {
Expand Down
9 changes: 7 additions & 2 deletions web/core/components/inbox/sidebar/inbox-list-item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ import { FC, MouseEvent } from "react";
import { observer } from "mobx-react";
import Link from "next/link";
import { useSearchParams } from "next/navigation";
import { Tooltip, PriorityIcon, Row } from "@plane/ui";
import { Tooltip, PriorityIcon, Row, Avatar } from "@plane/ui";
// components
import { ButtonAvatars } from "@/components/dropdowns/member/avatar";
import { InboxIssueStatus } from "@/components/inbox";
// helpers
import { cn } from "@/helpers/common.helper";
import { renderFormattedDate } from "@/helpers/date-time.helper";
// hooks
import { getFileURL } from "@/helpers/file.helper";
import { useLabel, useMember, useProjectInbox } from "@/hooks/store";
import { usePlatformOS } from "@/hooks/use-platform-os";

Expand Down Expand Up @@ -116,7 +117,11 @@ export const InboxIssueListItem: FC<InboxIssueListItemProps> = observer((props)
)}
</div>
{/* created by */}
{createdByDetails && <ButtonAvatars showTooltip={false} userIds={createdByDetails?.id} />}
{createdByDetails && createdByDetails.email?.includes("intake@plane.so") ? (
<Avatar src={getFileURL("")} name={"Plane"} size="md" showTooltip />
) : createdByDetails ? (
<ButtonAvatars showTooltip={false} userIds={createdByDetails?.id} />
) : null}
</div>
</Row>
</Link>
Expand Down
2 changes: 1 addition & 1 deletion web/core/components/project/settings/features-list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export const ProjectFeaturesList: FC<Props> = observer((props) => {
<div className="pl-14">
{currentProjectDetails?.[featureItem.property as keyof IProject] &&
featureItem.renderChildren &&
featureItem.renderChildren(currentProjectDetails, isAdmin, handleSubmit)}
featureItem.renderChildren(currentProjectDetails, isAdmin, handleSubmit, workspaceSlug)}
</div>
</div>
);
Expand Down
8 changes: 3 additions & 5 deletions web/core/services/inbox/inbox-issue.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,17 +77,15 @@ export class InboxIssueService extends APIService {
}

async retrievePublishForm(workspaceSlug: string, projectId: string): Promise<TInboxForm> {
return this.get(`/api/workspaces/${workspaceSlug}/projects/${projectId}/publish-intake/`)
return this.get(`/api/workspaces/${workspaceSlug}/projects/${projectId}/intake-settings/`)
.then((response) => response?.data)
.catch((error) => {
throw error?.response?.data;
});
}

async updatePublishForm(workspaceSlug: string, projectId: string, is_disabled: boolean): Promise<TInboxIssue> {
return this.patch(`/api/workspaces/${workspaceSlug}/projects/${projectId}/publish-intake/`, {
is_disabled,
})
async updatePublishForm(workspaceSlug: string, projectId: string, data: Partial<TInboxForm>): Promise<TInboxForm> {
return this.patch(`/api/workspaces/${workspaceSlug}/projects/${projectId}/intake-settings/`, data)
.then((response) => response?.data)
.catch((error) => {
throw error?.response?.data;
Expand Down
15 changes: 9 additions & 6 deletions web/core/store/inbox/project-inbox.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export interface IProjectInboxStore {
fetchInboxPaginationIssues: (workspaceSlug: string, projectId: string) => Promise<void>;
fetchInboxIssueById: (workspaceSlug: string, projectId: string, inboxIssueId: string) => Promise<TInboxIssue>;
fetchIntakeForms: (workspaceSlug: string, projectId: string) => Promise<void>;
toggleIntakeForms: (workspaceSlug: string, projectId: string, isDisabled: boolean) => Promise<void>;
toggleIntakeForms: (workspaceSlug: string, projectId: string, data: Partial<TInboxForm>) => Promise<void>;
regenerateIntakeForms: (workspaceSlug: string, projectId: string) => Promise<void>;
createInboxIssue: (
workspaceSlug: string,
Expand Down Expand Up @@ -329,20 +329,23 @@ export class ProjectInboxStore implements IProjectInboxStore {
}
};

toggleIntakeForms = async (workspaceSlug: string, projectId: string, isDisabled: boolean) => {
toggleIntakeForms = async (workspaceSlug: string, projectId: string, data: Partial<TInboxForm>) => {
const initialData = this.intakeForms[projectId];
try {
runInAction(() => {
set(this.intakeForms, projectId, { ...this.intakeForms[projectId], is_disabled: isDisabled });
set(this.intakeForms, projectId, { ...this.intakeForms[projectId], ...data });
});
const result = await this.inboxIssueService.updatePublishForm(workspaceSlug, projectId, data);
runInAction(() => {
set(this.intakeForms, projectId, { ...this.intakeForms[projectId], anchor: result?.anchor });
SatishGandham marked this conversation as resolved.
Show resolved Hide resolved
});
await this.inboxIssueService.updatePublishForm(workspaceSlug, projectId, isDisabled);
} catch {
console.error("Error fetching the publish forms");
runInAction(() => {
set(this.intakeForms, projectId, { ...this.intakeForms[projectId], is_disabled: !isDisabled });
set(this.intakeForms, projectId, initialData);
});
}
};

regenerateIntakeForms = async (workspaceSlug: string, projectId: string) => {
try {
const form = await this.inboxIssueService.regeneratePublishForm(workspaceSlug, projectId);
Expand Down
Loading