Skip to content
Closed
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
4 changes: 2 additions & 2 deletions web/app/[workspaceSlug]/(projects)/notifications/layout.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
"use client";

// components
import { NotificationsSidebar } from "@/components/workspace-notifications";
import { NotificationsSidebarRoot } from "@/components/workspace-notifications";

export default function ProjectInboxIssuesLayout({ children }: { children: React.ReactNode }) {
return (
<div className="relative w-full h-full overflow-hidden flex items-center">
<NotificationsSidebar />
<NotificationsSidebarRoot />
<div className="w-full h-full overflow-hidden overflow-y-auto">{children}</div>
</div>
);
Expand Down
1 change: 1 addition & 0 deletions web/ce/components/workspace-notifications/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './sidebar'
2 changes: 2 additions & 0 deletions web/ce/components/workspace-notifications/sidebar/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './notification-card'
export * from './root'
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { observer } from "mobx-react";
import { Clock } from "lucide-react";
import { Avatar, Row } from "@plane/ui";
// components
import { NotificationOption } from "@/components/workspace-notifications";
// helpers
import { cn } from "@/helpers/common.helper";
import { calculateTimeAgo, renderFormattedDate, renderFormattedTime } from "@/helpers/date-time.helper";
Expand All @@ -14,6 +13,7 @@ import { sanitizeCommentForNotification } from "@/helpers/notification.helper";
import { replaceUnderscoreIfSnakeCase, stripAndTruncateHTML } from "@/helpers/string.helper";
// hooks
import { useIssueDetail, useNotification, useWorkspaceNotifications } from "@/hooks/store";
import { NotificationOption } from "@/plane-web/components/workspace-notifications";

type TNotificationItem = {
workspaceSlug: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ import { FC } from "react";
import { observer } from "mobx-react";
import { ArchiveRestore } from "lucide-react";
import { ArchiveIcon, TOAST_TYPE, setToast } from "@plane/ui";
// components
import { NotificationItemOptionButton } from "@/components/workspace-notifications";
// constants
import { NOTIFICATION_ARCHIVED } from "@/constants/event-tracker";
// hooks
import { useEventTracker, useWorkspaceNotifications } from "@/hooks/store";
// components
import { NotificationItemOptionButton } from "@/plane-web/components/workspace-notifications";
// store
import { INotification } from "@/store/notifications/notification";
import { INotification } from "@/plane-web/store/notifications/notification";
Comment on lines +11 to +14
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 28, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Migration to new import paths is incomplete

The codebase shows inconsistent usage of import paths for notification-related components:

  • Several files still use the old path @/components/workspace-notifications:

    • web/core/components/workspace/sidebar/user-menu.tsx
    • web/core/components/workspace-notifications/sidebar/header/*
    • web/core/components/workspace-notifications/sidebar/filters/*
    • web/app/[workspaceSlug]/(projects)/notifications/layout.tsx
  • The new path @/plane-web/components/workspace-notifications is being used in other files, primarily in the CE components.

  • The INotification interface imports are consistently using the new path @/plane-web/store/notifications/notification, which is correct.

🔗 Analysis chain

Verify consistent usage of new import paths across the codebase

The import paths have been updated to use the new @/plane-web prefix. Let's ensure this change is consistent across all notification-related components.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistent usage of new import paths

# Check for any remaining old import paths
echo "Checking for old import paths..."
rg -l "@/components/workspace-notifications" --type typescript

# Check for consistent usage of new import paths
echo "Checking new import path usage..."
rg -l "@/plane-web/components/workspace-notifications" --type typescript

# Check for any mixed usage of INotification imports
echo "Checking INotification import consistency..."
rg -l "import.*INotification.*from.*@/store/notifications/notification" --type typescript
rg -l "import.*INotification.*from.*@/plane-web/store/notifications/notification" --type typescript

Length of output: 720


Script:

#!/bin/bash
# Description: Verify consistent usage of new import paths (corrected file type)

# Check for any remaining old import paths
echo "Checking for old import paths..."
rg -l "@/components/workspace-notifications" -t ts -t tsx

# Check for consistent usage of new import paths
echo "Checking new import path usage..."
rg -l "@/plane-web/components/workspace-notifications" -t ts -t tsx

# Check for any mixed usage of INotification imports
echo "Checking INotification import consistency..."
rg -l "import.*INotification.*from.*@/store/notifications/notification" -t ts -t tsx
rg -l "import.*INotification.*from.*@/plane-web/store/notifications/notification" -t ts -t tsx

Length of output: 672


Script:

#!/bin/bash
# Description: Verify consistent usage of new import paths (without file type flags)

# Check for any remaining old import paths
echo "Checking for old import paths..."
rg "@/components/workspace-notifications"

# Check for consistent usage of new import paths
echo "Checking new import path usage..."
rg "@/plane-web/components/workspace-notifications"

# Check for any mixed usage of INotification imports
echo "Checking INotification import consistency..."
rg "import.*INotification.*from.*@/store/notifications/notification"
rg "import.*INotification.*from.*@/plane-web/store/notifications/notification"

Length of output: 3995

Copy link
Member Author

Choose a reason for hiding this comment

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

Same here. Imported components are still in core.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mathalav55, thank you for the clarification. Since the imported components are still in core, it's appropriate for these files to use the old import paths. Apologies for the confusion.


✏️ Learnings added
Learnt from: mathalav55
PR: makeplane/plane#6107
File: web/ce/components/workspace-notifications/sidebar/notification-card/options/archive.tsx:11-14
Timestamp: 2024-11-28T07:02:54.415Z
Learning: When components are still located in `core`, it's appropriate for files to import them using `@/components/...`, and the migration to the new import paths is not necessary in such cases.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


type TNotificationItemArchiveOption = {
workspaceSlug: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ import { FC } from "react";
import { observer } from "mobx-react";
import { MessageSquare } from "lucide-react";
import { TOAST_TYPE, setToast } from "@plane/ui";
// components
import { NotificationItemOptionButton } from "@/components/workspace-notifications";
// constants
import { NOTIFICATIONS_READ } from "@/constants/event-tracker";
// hooks
import { useEventTracker, useWorkspaceNotifications } from "@/hooks/store";
// components
import { NotificationItemOptionButton } from "@/plane-web/components/workspace-notifications";
Comment on lines +11 to +12
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 28, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Import paths need to be updated for consistency

Several files are still using the old import path @/components/workspace-notifications instead of the new @/plane-web/components/workspace-notifications:

  • web/core/components/workspace/sidebar/user-menu.tsx
  • web/core/components/workspace-notifications/sidebar/header/root.tsx
  • web/core/components/workspace-notifications/sidebar/header/options/root.tsx
  • web/core/components/workspace-notifications/sidebar/header/options/menu-option/root.tsx
  • web/core/components/workspace-notifications/sidebar/header/options/menu-option/menu-item.tsx
  • web/core/components/workspace-notifications/sidebar/filters/menu/root.tsx
  • web/ce/components/workspace-notifications/sidebar/root.tsx
  • web/app/[workspaceSlug]/(projects)/notifications/layout.tsx
🔗 Analysis chain

Verify consistency of import path changes

The import paths have been updated to use the new @/plane-web/ namespace, which aligns with the file restructuring objective.

Let's verify the consistency of these changes across related files:

Also applies to: 14-14

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that related notification components use consistent import paths

# Check for any remaining old import paths
rg "from \"@/components/workspace-notifications\"" 

# Verify consistent usage of new import paths
rg "from \"@/plane-web/components/workspace-notifications\""

# Check for any inconsistencies in notification store imports
rg "from \"@/plane-web/store/notifications/notification\""

Length of output: 3669

Copy link
Member Author

Choose a reason for hiding this comment

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

Other imported components are still in core not moved.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mathalav55, understood. Thanks for the clarification that the other imported components are still in core and have not been moved yet.

(・・)?


✏️ Learnings added
Learnt from: mathalav55
PR: makeplane/plane#6107
File: web/ce/components/workspace-notifications/sidebar/notification-card/options/read.tsx:11-12
Timestamp: 2024-11-28T07:02:15.337Z
Learning: Some components are still in core and have not been moved yet, so their import paths remain the same.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

// store
import { INotification } from "@/store/notifications/notification";
import { INotification } from "@/plane-web/store/notifications/notification";

type TNotificationItemReadOption = {
workspaceSlug: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@

import { FC, Dispatch, SetStateAction } from "react";
import { observer } from "mobx-react";
// helpers
import { cn } from "@/helpers/common.helper";
// hooks
import { useNotification } from "@/hooks/store";
// components
import {
NotificationItemReadOption,
NotificationItemArchiveOption,
NotificationItemSnoozeOption,
} from "@/components/workspace-notifications";
// helpers
import { cn } from "@/helpers/common.helper";
// hooks
import { useNotification } from "@/hooks/store";
} from "@/plane-web/components/workspace-notifications";

type TNotificationOption = {
workspaceSlug: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@ import { observer } from "mobx-react";
import { Clock } from "lucide-react";
import { Popover, Transition } from "@headlessui/react";
import { Tooltip, setToast, TOAST_TYPE } from "@plane/ui";
// components
import { NotificationSnoozeModal } from "@/components/workspace-notifications";
// constants
import { NOTIFICATION_SNOOZE_OPTIONS } from "@/constants/notification";
import { cn } from "@/helpers/common.helper";
// hooks
import { useWorkspaceNotifications } from "@/hooks/store";
import { usePlatformOS } from "@/hooks/use-platform-os";
// components
import { NotificationSnoozeModal } from "@/plane-web/components/workspace-notifications";
// store
import { INotification } from "@/store/notifications/notification";
import { INotification } from "@/plane-web/store/notifications/notification";

type TNotificationItemSnoozeOption = {
workspaceSlug: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@

import { FC } from "react";
import { observer } from "mobx-react";
// components
import { NotificationItem } from "@/components/workspace-notifications";
// constants
import { ENotificationLoader, ENotificationQueryParamType } from "@/constants/notification";
// hooks
import { useWorkspaceNotifications } from "@/hooks/store";
// components
import { NotificationItem } from "@/plane-web/components/workspace-notifications";
Comment on lines +9 to +10
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

Import path inconsistency detected across the codebase

The verification reveals mixed usage of both old and new import paths:

  • Several files still use the old path @/components/workspace-notifications
  • Multiple files have been updated to use the new path @/plane-web/components/workspace-notifications
  • The target import path web/plane-web/components doesn't exist in the repository

Files still using old import path:

  • web/app/[workspaceSlug]/(projects)/notifications/layout.tsx
  • Multiple files under web/core/components/workspace-notifications/
  • web/core/components/workspace/sidebar/user-menu.tsx
🔗 Analysis chain

Verify the new import path structure

The import path change from @/components/workspace-notifications to @/plane-web/components/workspace-notifications aligns with the file rearrangement objective.

Let's verify the new import path structure:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the new import path exists and check for consistency

# Check if the new path exists
fd "workspace-notifications" "web/plane-web/components"

# Check for any remaining old imports that need updating
rg -l "@/components/workspace-notifications" "web"

# Check for mixed usage of old and new paths
rg -l "@/plane-web/components/workspace-notifications" "web"

Length of output: 1554


type TNotificationCardListRoot = {
workspaceSlug: string;
Expand Down
113 changes: 113 additions & 0 deletions web/ce/components/workspace-notifications/sidebar/root.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
"use client";

import { FC, useCallback } from "react";
import { observer } from "mobx-react";
import { useParams } from "next/navigation";
// components
import { Header, Row, ERowVariant, EHeaderVariant, ContentWrapper } from "@plane/ui";
import { CountChip } from "@/components/common";
import {
NotificationsLoader,
NotificationEmptyState,
NotificationSidebarHeader,
AppliedFilters,
} from "@/components/workspace-notifications";
// constants
import { NOTIFICATION_TABS, TNotificationTab } from "@/constants/notification";
// helpers
import { cn } from "@/helpers/common.helper";
import { getNumberCount } from "@/helpers/string.helper";
// hooks
import { useWorkspace, useWorkspaceNotifications } from "@/hooks/store";

import { NotificationCardListRoot } from "@/plane-web/components/workspace-notifications";
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

Import path appears to be incorrect

The import path @/plane-web/components/workspace-notifications seems incorrect as:

  1. The component is exported from:
    • web/ce/components/workspace-notifications/sidebar/notification-card/root.tsx
    • web/ee/components/workspace-notifications/sidebar/notification-card/root.tsx
  2. The attempted import path web/plane-web/components/ does not exist in the codebase

The import statement should be updated to use the correct path based on the CE/EE component location.

🔗 Analysis chain

Verify the integration with NotificationCardListRoot component.

Since this PR involves file rearrangement, ensure that the import path for NotificationCardListRoot is correct and the component is properly exported from its new location.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the NotificationCardListRoot component's export and usage

# Check if the component is properly exported
rg -g '*.{ts,tsx}' -l 'export.*NotificationCardListRoot'

# Check for any other imports of this component to ensure they're updated
rg -g '*.{ts,tsx}' 'import.*NotificationCardListRoot'

Length of output: 418


Script:

#!/bin/bash
# Let's verify the actual export in plane-web components
rg -g '*.{ts,tsx}' -A 5 'export.*NotificationCardListRoot' 'web/plane-web/components/'

# Also check the barrel exports in workspace-notifications
rg -g '*.{ts,tsx}' 'export.*NotificationCardListRoot' 'web/plane-web/components/workspace-notifications/'

Length of output: 348


export const NotificationsSidebar: FC = observer(() => {
const { workspaceSlug } = useParams();
// hooks
const { getWorkspaceBySlug } = useWorkspace();
const {
currentSelectedNotificationId,
unreadNotificationsCount,
loader,
notificationIdsByWorkspaceId,
currentNotificationTab,
setCurrentNotificationTab,
} = useWorkspaceNotifications();
// derived values
const workspace = workspaceSlug ? getWorkspaceBySlug(workspaceSlug.toString()) : undefined;
const notificationIds = workspace ? notificationIdsByWorkspaceId(workspace.id) : undefined;

const handleTabClick = useCallback((tabValue: TNotificationTab)=>{
if(currentSelectedNotificationId !== tabValue) {
setCurrentNotificationTab(tabValue);
}
},[ setCurrentNotificationTab, currentSelectedNotificationId ]);
Comment on lines +41 to +45
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

Fix incorrect comparison in handleTabClick

The condition is comparing currentSelectedNotificationId with tabValue, but these appear to be different types:

  • currentSelectedNotificationId: likely a notification ID
  • tabValue: a tab value of type TNotificationTab

The correct comparison should be with currentNotificationTab.

-    if(currentSelectedNotificationId !== tabValue) {
+    if(currentNotificationTab !== tabValue) {
       setCurrentNotificationTab(tabValue);
     }
📝 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
const handleTabClick = useCallback((tabValue: TNotificationTab)=>{
if(currentSelectedNotificationId !== tabValue) {
setCurrentNotificationTab(tabValue);
}
},[ setCurrentNotificationTab, currentSelectedNotificationId ]);
const handleTabClick = useCallback((tabValue: TNotificationTab)=>{
if(currentNotificationTab !== tabValue) {
setCurrentNotificationTab(tabValue);
}
},[ setCurrentNotificationTab, currentSelectedNotificationId ]);



if (!workspaceSlug || !workspace) return <></>;

return (
<div
className={cn(
"relative border-0 md:border-r border-custom-border-200 z-[10] flex-shrink-0 bg-custom-background-100 h-full transition-all overflow-hidden",
currentSelectedNotificationId ? "w-0 md:w-2/6" : "w-full md:w-2/6"
)}
>
<div className="relative w-full h-full overflow-hidden flex flex-col">
<Row className="h-[3.75rem] border-b border-custom-border-200 flex">
<NotificationSidebarHeader workspaceSlug={workspaceSlug.toString()} />
</Row>

<Header variant={EHeaderVariant.SECONDARY} className="justify-start">
{NOTIFICATION_TABS.map((tab) => (
<div
key={tab.value}
className="h-full px-3 relative cursor-pointer"
onClick={() => handleTabClick(tab.value)}
>
<div
className={cn(
`relative h-full flex justify-center items-center gap-1 text-sm transition-all`,
currentNotificationTab === tab.value
? "text-custom-primary-100"
: "text-custom-text-100 hover:text-custom-text-200"
)}
>
<div className="font-medium">{tab.label}</div>
{tab.count(unreadNotificationsCount) > 0 && (
<CountChip count={getNumberCount(tab.count(unreadNotificationsCount))} />
)}
</div>
{currentNotificationTab === tab.value && (
<div className="border absolute bottom-0 right-0 left-0 rounded-t-md border-custom-primary-100" />
)}
</div>
))}
</Header>

{/* applied filters */}
<AppliedFilters workspaceSlug={workspaceSlug.toString()} />

{/* rendering notifications */}
{loader === "init-loader" ? (
<div className="relative w-full h-full overflow-hidden">
<NotificationsLoader />
</div>
) : (
<>
{notificationIds && notificationIds.length > 0 ? (
<ContentWrapper variant={ERowVariant.HUGGING}>
<NotificationCardListRoot workspaceSlug={workspaceSlug.toString()} workspaceId={workspace?.id} />
</ContentWrapper>
) : (
<div className="relative w-full h-full flex justify-center items-center">
<NotificationEmptyState />
</div>
)}
</>
)}
</div>
</div>
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
import set from "lodash/set";
import { action, computed, makeObservable, observable, runInAction } from "mobx";
import { IUserLite, TNotification, TNotificationData } from "@plane/types";
// store
import { RootStore } from "@/plane-web/store/root.store";
// services
import workspaceNotificationService from "@/services/workspace-notification.service";
// store
import { CoreRootStore } from "../root.store";


export interface INotification extends TNotification {
// observables
Expand Down Expand Up @@ -51,7 +52,7 @@ export class Notification implements INotification {
updated_by: string | undefined = undefined;

constructor(
private store: CoreRootStore,
private store: RootStore,
private notification: TNotification
) {
makeObservable(this, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ import {
} from "@/constants/notification";
// helpers
import { convertToEpoch } from "@/helpers/date-time.helper";
// store
import { Notification, INotification } from "@/plane-web/store/notifications/notification";
import { RootStore } from "@/plane-web/store/root.store";
// services
import workspaceNotificationService from "@/services/workspace-notification.service";
// store
import { Notification, INotification } from "@/store/notifications/notification";
import { CoreRootStore } from "@/store/root.store";

type TNotificationLoader = ENotificationLoader | undefined;
type TNotificationQueryParamType = ENotificationQueryParamType;
Expand Down Expand Up @@ -84,7 +84,7 @@ export class WorkspaceNotificationStore implements IWorkspaceNotificationStore {
read: false,
};

constructor(private store: CoreRootStore) {
constructor(private store: RootStore) {
makeObservable(this, {
// observables
loader: observable.ref,
Expand Down
9 changes: 8 additions & 1 deletion web/ce/store/root.store.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
// store
import { CoreRootStore } from "@/store/root.store";
import { IWorkspaceNotificationStore, WorkspaceNotificationStore } from "./notifications/workspace-notifications.store";
import { ITimelineStore, TimeLineStore } from "./timeline";

export class RootStore extends CoreRootStore {
timelineStore: ITimelineStore;
workspaceNotification: IWorkspaceNotificationStore;

constructor() {
super();

this.workspaceNotification = new WorkspaceNotificationStore(this)
this.timelineStore = new TimeLineStore(this);
}

resetOnSignOut() {
super.resetOnSignOut();
this.workspaceNotification = new WorkspaceNotificationStore(this);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ export * from "./header";

export * from "./filters";

export * from "./notification-card";
// export * from "./notification-card";
Copy link
Contributor

Choose a reason for hiding this comment

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

@mathalav55 please check

Loading