Skip to content

chore: code refactor #5952

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
SatishGandham merged 2 commits into from
Nov 5, 2024
Merged

chore: code refactor #5952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
adb7141
chore: code refactor
anmolsinghbhatia Nov 5, 2024
5ad5042
chore: code refactor
anmolsinghbhatia Nov 5, 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
15 changes: 15 additions & 0 deletions web/ce/components/de-dupe/de-dupe-button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
"use client";
import React, { FC } from "react";
// local components

type TDeDupeButtonRoot = {
workspaceSlug: string;
isDuplicateModalOpen: boolean;
handleOnClick: () => void;
label: string;
};

export const DeDupeButtonRoot: FC<TDeDupeButtonRoot> = (props) => {
const { workspaceSlug, isDuplicateModalOpen, label, handleOnClick } = props;
return <></>;
};
Comment on lines +12 to +15
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 5, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Component implementation is incomplete

The component currently returns an empty fragment and doesn't utilize any of its props. Based on the PR objectives and AI summary, this component is meant to replace an existing implementation for handling duplicate issues, but the functionality is missing.

Consider implementing:

  1. A button UI element using the label prop
  2. Click handler integration with handleOnClick
  3. Modal state handling with isDuplicateModalOpen

Here's a suggested implementation:

export const DeDupeButtonRoot: FC<TDeDupeButtonRoot> = (props) => {
  const { workspaceSlug, isDuplicateModalOpen, label, handleOnClick } = props;
-  return <></>;
+  return (
+    <button
+      type="button"
+      className="w-full text-custom-text-200 hover:text-custom-text-100"
+      onClick={handleOnClick}
+    >
+      {label}
+    </button>
+  );
};
📝 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 DeDupeButtonRoot: FC<TDeDupeButtonRoot> = (props) => {
const { workspaceSlug, isDuplicateModalOpen, label, handleOnClick } = props;
return <></>;
};
export const DeDupeButtonRoot: FC<TDeDupeButtonRoot> = (props) => {
const { workspaceSlug, isDuplicateModalOpen, label, handleOnClick } = props;
return (
<button
type="button"
className="w-full text-custom-text-200 hover:text-custom-text-100"
onClick={handleOnClick}
>
{label}
</button>
);
};
🧰 Tools
🪛 GitHub Check: lint-web

[warning] 13-13:
'workspaceSlug' is assigned a value but never used

[warning] 13-13:
'isDuplicateModalOpen' is assigned a value but never used

[warning] 13-13:
'label' is assigned a value but never used

[warning] 13-13:
'handleOnClick' is assigned a value but never used

💡 Codebase verification

Component integration needs to be updated

The component is actively used in two locations with consistent prop usage:

  • web/core/components/inbox/modals/create-modal/create-root.tsx
  • web/core/components/issues/issue-modal/form.tsx

Both files import and use DeDupeButtonRoot with the same props that are being removed in the empty implementation (workspaceSlug, isDuplicateModalOpen, label, handleOnClick). This will cause runtime errors.

🔗 Analysis chain

Verify component integration

Let's verify how this component is integrated in the mentioned files to ensure proper implementation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check how DeDupeButtonRoot is used in the codebase

# Check component imports and usage
echo "Checking DeDupeButtonRoot usage:"
rg -A 5 "DeDupeButtonRoot" "web/core/components/inbox/modals/create-modal/create-root.tsx" "web/core/components/issues/issue-modal/form.tsx"

# Check if there are any remnants of the old implementation
echo -e "\nChecking for old implementation:"
rg "DeDupeIssueButtonLabel"

Length of output: 3270

🧰 Tools
🪛 GitHub Check: lint-web

[warning] 13-13:
'workspaceSlug' is assigned a value but never used

[warning] 13-13:
'isDuplicateModalOpen' is assigned a value but never used

[warning] 13-13:
'label' is assigned a value but never used

[warning] 13-13:
'handleOnClick' is assigned a value but never used

1 change: 1 addition & 0 deletions web/ce/components/de-dupe/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from "./de-dupe-button";
export * from "./duplicate-modal";
export * from "./duplicate-popover";
export * from "./issue-block";
20 changes: 7 additions & 13 deletions web/core/components/inbox/modals/create-modal/create-root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { useAppRouter } from "@/hooks/use-app-router";
import useKeypress from "@/hooks/use-keypress";
import { usePlatformOS } from "@/hooks/use-platform-os";
// services
import { DeDupeIssueButtonLabel, DuplicateModalRoot } from "@/plane-web/components/de-dupe";
import { DeDupeButtonRoot, DuplicateModalRoot } from "@/plane-web/components/de-dupe";
import { useDebouncedDuplicateIssues } from "@/plane-web/hooks/use-debounced-duplicate-issues";
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 5, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Debounce implementation is missing in the duplicate issues hook

The current implementation of useDebouncedDuplicateIssues in web/ce/hooks/use-debounced-duplicate-issues.tsx is incomplete. Despite its name suggesting debounced functionality:

  • It doesn't use the useDebounce hook that exists in web/core/hooks/use-debounce.tsx
  • It returns an empty array without any actual duplicate detection logic
  • No debounce timing is configured

This needs to be addressed to:

  1. Implement proper debounce using the existing useDebounce hook
  2. Add the actual duplicate detection logic
  3. Configure appropriate debounce timing (other components use 500-1500ms)
🔗 Analysis chain

Verify the debounce timing for duplicate detection.

The code uses useDebouncedDuplicateIssues hook for duplicate detection. Let's verify its implementation to ensure optimal performance.

Also applies to: 213-218

Let me gather more information about the debounce implementation.

Let me try searching in a different location since the previous path wasn't found.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the implementation of useDebouncedDuplicateIssues hook
# Expected: Find the debounce timing configuration and any rate limiting

# Search for the hook implementation
ast-grep --pattern 'export const useDebouncedDuplicateIssues = $_'

# Search for debounce timing configuration
rg -A 5 'useDebouncedDuplicateIssues'

Length of output: 7051

Script:

#!/bin/bash
# Search for the actual implementation file and any debounce logic
rg -g '*.ts*' -A 10 'useDebouncedDuplicateIssues.*=' web/plane-web/hooks/

# Check for any debounce utility or hook being used
rg -g '*.ts*' 'useDebounce|debounce' web/plane-web/hooks/

Length of output: 257

Script:

#!/bin/bash
# Search for the actual implementation in the web directory
rg -g '*.ts*' -A 10 'useDebouncedDuplicateIssues.*=' web/

# Check for any debounce utility or hook being used
rg -g '*.ts*' 'useDebounce|debounce' web/

# Find the correct path for the hook implementation
fd -e tsx -e ts use-debounced-duplicate-issues

Length of output: 8345

import { FileService } from "@/services/file.service";

Expand Down Expand Up @@ -210,18 +210,12 @@ export const InboxIssueCreateRoot: FC<TInboxIssueCreateRoot> = observer((props)
<div className="flex items-center justify-between gap-2">
<h3 className="text-xl font-medium text-custom-text-200">Create intake issue</h3>
{duplicateIssues?.length > 0 && (
<button
onClick={(e) => {
e.stopPropagation();
e.preventDefault();
handleDuplicateIssueModal(!isDuplicateModalOpen);
}}
>
<DeDupeIssueButtonLabel
isOpen={isDuplicateModalOpen}
buttonLabel={`${duplicateIssues.length} duplicate issue${duplicateIssues.length > 1 ? "s" : ""} found!`}
/>
</button>
<DeDupeButtonRoot
workspaceSlug={workspaceSlug}
isDuplicateModalOpen={isDuplicateModalOpen}
label={`${duplicateIssues.length} duplicate issue${duplicateIssues.length > 1 ? "s" : ""} found!`}
handleOnClick={() => handleDuplicateIssueModal(!isDuplicateModalOpen)}
/>
)}
</div>
<div className="space-y-3">
Expand Down
20 changes: 7 additions & 13 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 @@ -31,7 +31,7 @@ import { useIssueDetail, useProject, useProjectState, useWorkspaceDraftIssues }
import { usePlatformOS } from "@/hooks/use-platform-os";
import { useProjectIssueProperties } from "@/hooks/use-project-issue-properties";
// plane web components
import { DeDupeIssueButtonLabel, DuplicateModalRoot } from "@/plane-web/components/de-dupe";
import { DeDupeButtonRoot, DuplicateModalRoot } from "@/plane-web/components/de-dupe";
import { IssueAdditionalProperties, IssueTypeSelect } from "@/plane-web/components/issues/issue-modal";
import { useDebouncedDuplicateIssues } from "@/plane-web/hooks/use-debounced-duplicate-issues";

Expand Down Expand Up @@ -350,18 +350,12 @@ export const IssueFormRoot: FC<IssueFormProps> = observer((props) => {
)}
</div>
{duplicateIssues.length > 0 && (
<button
onClick={(e) => {
e.stopPropagation();
e.preventDefault();
handleDuplicateIssueModal(!isDuplicateModalOpen);
}}
>
<DeDupeIssueButtonLabel
isOpen={isDuplicateModalOpen}
buttonLabel={`${duplicateIssues.length} duplicate issue${duplicateIssues.length > 1 ? "s" : ""} found!`}
/>
</button>
<DeDupeButtonRoot
workspaceSlug={workspaceSlug?.toString()}
isDuplicateModalOpen={isDuplicateModalOpen}
label={`${duplicateIssues.length} duplicate issue${duplicateIssues.length > 1 ? "s" : ""} found!`}
handleOnClick={() => handleDuplicateIssueModal(!isDuplicateModalOpen)}
/>
)}
</div>
{watch("parent_id") && selectedParentIssue && (
Expand Down
1 change: 1 addition & 0 deletions web/ee/components/de-dupe/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "ce/components/de-dupe";