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

[PE-93] refactor: accept generic function to search mentions #6249

Merged
merged 1 commit into from
Dec 20, 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
12 changes: 6 additions & 6 deletions web/core/components/editor/rich-text-editor/rich-text-editor.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React, { forwardRef } from "react";
// editor
import { EditorRefApi, IRichTextEditor, RichTextEditorWithRef } from "@plane/editor";
// plane types
import { TSearchEntityRequestPayload, TSearchResponse } from "@plane/types";
// components
import { EditorMentionsRoot } from "@/components/editor";
// helpers
Expand All @@ -11,26 +13,24 @@ import { useEditorMention } from "@/hooks/use-editor-mention";
// plane web hooks
import { useEditorFlagging } from "@/plane-web/hooks/use-editor-flagging";
import { useFileSize } from "@/plane-web/hooks/use-file-size";
// services
import { ProjectService } from "@/services/project";
const projectService = new ProjectService();

interface RichTextEditorWrapperProps
extends Omit<IRichTextEditor, "disabledExtensions" | "fileHandler" | "mentionHandler"> {
searchMentionCallback: (payload: TSearchEntityRequestPayload) => Promise<TSearchResponse>;
workspaceSlug: string;
workspaceId: string;
projectId?: string;
uploadFile: (file: File) => Promise<string>;
}

export const RichTextEditor = forwardRef<EditorRefApi, RichTextEditorWrapperProps>((props, ref) => {
const { containerClassName, workspaceSlug, workspaceId, projectId, uploadFile, ...rest } = props;
const { containerClassName, workspaceSlug, workspaceId, projectId, searchMentionCallback, uploadFile, ...rest } =
props;
// editor flaggings
const { richTextEditor: disabledExtensions } = useEditorFlagging(workspaceSlug?.toString());
// use editor mention
const { fetchMentions } = useEditorMention({
searchEntity: async (payload) =>
await projectService.searchEntity(workspaceSlug?.toString() ?? "", projectId?.toString() ?? "", payload),
searchEntity: async (payload) => await searchMentionCallback(payload),
});
// file size
const { maxFileSize } = useFileSize();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ import { useProjectInbox } from "@/hooks/store";
import { usePlatformOS } from "@/hooks/use-platform-os";
// services
import { FileService } from "@/services/file.service";
import { ProjectService } from "@/services/project";
const fileService = new FileService();
const projectService = new ProjectService();
Comment on lines +24 to +26
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

Consider dependency injection for services

Creating service instances at the module level can lead to:

  • Multiple instances across imports
  • Difficulties in testing
  • Potential memory leaks

Consider injecting the service through props or a dependency injection container:

-const projectService = new ProjectService();
+interface Props {
+  projectService: ProjectService;
+  // ... other props
+}

Committable suggestion skipped: line range outside the PR's diff.


type TInboxIssueDescription = {
containerClassName?: string;
Expand Down Expand Up @@ -72,6 +74,9 @@ export const InboxIssueDescription: FC<TInboxIssueDescription> = observer((props
dragDropEnabled={false}
onChange={(_description: object, description_html: string) => handleData("description_html", description_html)}
placeholder={getDescriptionPlaceholder}
searchMentionCallback={async (payload) =>
await projectService.searchEntity(workspaceSlug?.toString() ?? "", projectId?.toString() ?? "", payload)
}
Comment on lines +77 to +79
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

Improve null safety in searchMentionCallback

The current implementation uses optional chaining with toString() which could still result in "undefined" being passed as a string.

Consider this safer implementation:

-      searchMentionCallback={async (payload) =>
-        await projectService.searchEntity(workspaceSlug?.toString() ?? "", projectId?.toString() ?? "", payload)
-      }
+      searchMentionCallback={async (payload) => {
+        if (!workspaceSlug || !projectId) throw new Error("Workspace slug and project ID are required");
+        return await projectService.searchEntity(workspaceSlug.toString(), projectId.toString(), payload);
+      }}
📝 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
searchMentionCallback={async (payload) =>
await projectService.searchEntity(workspaceSlug?.toString() ?? "", projectId?.toString() ?? "", payload)
}
searchMentionCallback={async (payload) => {
if (!workspaceSlug || !projectId) throw new Error("Workspace slug and project ID are required");
return await projectService.searchEntity(workspaceSlug.toString(), projectId.toString(), payload);
}}

containerClassName={containerClassName}
onEnterKeyPress={onEnterKeyPress}
tabIndex={getIndex("description_html")}
Expand Down
9 changes: 9 additions & 0 deletions web/core/components/issues/description-input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ import { getDescriptionPlaceholder } from "@/helpers/issue.helper";
import { useWorkspace } from "@/hooks/store";
// services
import { FileService } from "@/services/file.service";
import { ProjectService } from "@/services/project";
const fileService = new FileService();
const projectService = new ProjectService();

export type IssueDescriptionInputProps = {
containerClassName?: string;
Expand Down Expand Up @@ -118,6 +120,13 @@ export const IssueDescriptionInput: FC<IssueDescriptionInputProps> = observer((p
placeholder={
placeholder ? placeholder : (isFocused, value) => getDescriptionPlaceholder(isFocused, value)
}
searchMentionCallback={async (payload) =>
await projectService.searchEntity(
workspaceSlug?.toString() ?? "",
projectId?.toString() ?? "",
payload
)
}
containerClassName={containerClassName}
uploadFile={async (file) => {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { usePlatformOS } from "@/hooks/use-platform-os";
// services
import { AIService } from "@/services/ai.service";
import { FileService } from "@/services/file.service";
import { ProjectService } from "@/services/project";

type TIssueDescriptionEditorProps = {
control: Control<TIssue>;
Expand All @@ -49,6 +50,7 @@ type TIssueDescriptionEditorProps = {
// services
const aiService = new AIService();
const fileService = new FileService();
const projectService = new ProjectService();

export const IssueDescriptionEditor: React.FC<TIssueDescriptionEditorProps> = observer((props) => {
const {
Expand Down Expand Up @@ -188,6 +190,13 @@ export const IssueDescriptionEditor: React.FC<TIssueDescriptionEditorProps> = ob
ref={editorRef}
tabIndex={getIndex("description_html")}
placeholder={getDescriptionPlaceholder}
searchMentionCallback={async (payload) =>
await projectService.searchEntity(
workspaceSlug?.toString() ?? "",
projectId?.toString() ?? "",
payload
)
}
containerClassName="pt-3 min-h-[120px]"
uploadFile={async (file) => {
try {
Expand Down
Loading