Skip to content

[WEB-2382]chore:notification files restructuring #6181

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
pushya22 merged 9 commits into from
Dec 11, 2024
Merged

[WEB-2382]chore:notification files restructuring #6181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c9a8e8f
chore: adjusted increment/decrement for unread count
vamsikrishnamathala Dec 9, 2024
bf57d9e
chore: improved param handling for unread notification count function
vamsikrishnamathala Dec 9, 2024
8d181d7
Merge branch 'preview' of github.com:makeplane/plane into chore-notif…
vamsikrishnamathala Dec 10, 2024
bc37bf8
chore:file restructuring
vamsikrishnamathala Dec 10, 2024
9b8bed4
fix:notification types
vamsikrishnamathala Dec 10, 2024
64c3dfd
chore:file restructuring
vamsikrishnamathala Dec 10, 2024
5f36d99
chore:modified notfication types
vamsikrishnamathala Dec 10, 2024
2768eaa
chore: modified types for notification
vamsikrishnamathala Dec 11, 2024
17d3169
chore:removed redundant checks for id
vamsikrishnamathala Dec 11, 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
2 changes: 1 addition & 1 deletion packages/types/src/workspace-notifications.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export type TNotificationData = {
};

export type TNotification = {
id: string | undefined;
id: string;
title: string | undefined;
data: TNotificationData | undefined;
entity_identifier: string | undefined;
Expand Down
2 changes: 1 addition & 1 deletion web/app/[workspaceSlug]/(projects)/notifications/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"use client";

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

export default function ProjectInboxIssuesLayout({ children }: { children: React.ReactNode }) {
return (
Expand Down
2 changes: 1 addition & 1 deletion web/ce/components/workspace-notifications/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export * from './root'
export * from "./notification-card/root";
0 ...ations/sidebar/notification-card/root.tsx → ...-notifications/notification-card/root.tsx
View file
Open in desktop
File renamed without changes.
1 change: 1 addition & 0 deletions web/core/components/workspace-notifications/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export * from "./notification-app-sidebar-option";
export * from "./sidebar";
export * from "./root";
3 changes: 2 additions & 1 deletion ...mponents/workspace-notifications/root.tsx → ...mponents/workspace-notifications/root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
NotificationEmptyState,
NotificationSidebarHeader,
AppliedFilters,
NotificationCardListRoot,
} from "@/components/workspace-notifications";
// constants
import { NOTIFICATION_TABS, TNotificationTab } from "@/constants/notification";
Expand All @@ -21,6 +20,8 @@ import { getNumberCount } from "@/helpers/string.helper";
// hooks
import { useWorkspace, useWorkspaceNotifications } from "@/hooks/store";

import { NotificationCardListRoot } from "@/plane-web/components/workspace-notifications";

Comment on lines +23 to +24
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 10, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Multiple components still using old import path need attention

The verification reveals that while NotificationCardListRoot has been moved to the new path, several other components in the file are still using the old import path @/components/workspace-notifications:

  • NotificationsLoader
  • NotificationEmptyState
  • NotificationSidebarHeader
  • AppliedFilters

Additionally, there are multiple other files in the codebase still importing components from the old path, suggesting this migration is incomplete.

🔗 Analysis chain

Import path change aligns with component restructuring.

The NotificationCardListRoot component has been moved to @/plane-web/components, which aligns with the ongoing restructuring effort. Based on the retrieved learning, this is part of a gradual migration where some components remain in core.

Let's verify if there are other notification components that might need similar updates:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other notification components that might need path updates

# Search for notification component imports
rg -t typescript -t javascript "from\s+['\"]@/components/workspace-notifications['\"]"

# Search for potential notification components still in the old location
fd -t f . "components/workspace-notifications" | grep -v "plane-web"

Length of output: 316

Script:

#!/bin/bash
# Let's try a different approach to find notification-related imports and components

# Search for imports containing notification-related paths
rg "from.*notification" --type=tsx --type=ts

# Find notification-related files
fd "notification" -e tsx -e ts

Length of output: 706

Script:

#!/bin/bash
# Let's check the content of these files to find any imports we need to verify

# Check imports in notification-related files
rg -l "@/components/workspace-notifications" web/core/components/workspace-notifications/notification-app-sidebar-option.tsx

# Let's also check the content of the root.tsx file to understand the full context
cat web/core/components/workspace-notifications/root.tsx

Length of output: 4420

Script:

#!/bin/bash
# Let's check the imports from @/components/workspace-notifications in the root.tsx file

# Search for the components that are still being imported from the old path
ast-grep --pattern 'import { $_ } from "@/components/workspace-notifications"'

# Also check if these components exist in the new location
fd -e tsx "NotificationsLoader|NotificationEmptyState|NotificationSidebarHeader|AppliedFilters" web/plane-web/components/workspace-notifications

Length of output: 1967

export const NotificationsSidebarRoot: FC = observer(() => {
const { workspaceSlug } = useParams();
// hooks
Expand Down
1 change: 0 additions & 1 deletion web/core/components/workspace-notifications/sidebar/notification-card/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
export * from "./root";
export * from "./item";
export * from "./options";
18 changes: 2 additions & 16 deletions web/core/store/notifications/notification.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export interface INotification extends TNotification {

export class Notification implements INotification {
// observables
id: string | undefined = undefined;
id: string;
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 10, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor: Remove redundant ID null checks

Since id is now required, the null checks in methods like updateNotification, markNotificationAsRead, etc., are redundant.

Here's how to clean up the redundant checks:

- if (!this.id) return undefined;

This change should be applied to all methods that have this check.

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

title: string | undefined = undefined;
data: TNotificationData | undefined = undefined;
entity_identifier: string | undefined = undefined;
Expand Down Expand Up @@ -54,6 +54,7 @@ export class Notification implements INotification {
private store: CoreRootStore,
private notification: TNotification
) {
this.id = this.notification.id;
makeObservable(this, {
// observables
id: observable.ref,
Expand Down Expand Up @@ -90,7 +91,6 @@ export class Notification implements INotification {
snoozeNotification: action,
unSnoozeNotification: action,
});
this.id = this.notification.id;
this.title = this.notification.title;
this.data = this.notification.data;
this.entity_identifier = this.notification.entity_identifier;
Expand Down Expand Up @@ -169,8 +169,6 @@ export class Notification implements INotification {
workspaceSlug: string,
payload: Partial<TNotification>
): Promise<TNotification | undefined> => {
if (!this.id) return undefined;

try {
const notification = await workspaceNotificationService.updateNotificationById(workspaceSlug, this.id, payload);
if (notification) {
Expand All @@ -188,8 +186,6 @@ export class Notification implements INotification {
* @returns { TNotification | undefined }
*/
markNotificationAsRead = async (workspaceSlug: string): Promise<TNotification | undefined> => {
if (!this.id) return undefined;

const currentNotificationReadAt = this.read_at;
try {
const payload: Partial<TNotification> = {
Expand All @@ -215,8 +211,6 @@ export class Notification implements INotification {
* @returns { TNotification | undefined }
*/
markNotificationAsUnRead = async (workspaceSlug: string): Promise<TNotification | undefined> => {
if (!this.id) return undefined;

const currentNotificationReadAt = this.read_at;
try {
const payload: Partial<TNotification> = {
Expand All @@ -242,8 +236,6 @@ export class Notification implements INotification {
* @returns { TNotification | undefined }
*/
archiveNotification = async (workspaceSlug: string): Promise<TNotification | undefined> => {
if (!this.id) return undefined;

const currentNotificationArchivedAt = this.archived_at;
try {
const payload: Partial<TNotification> = {
Expand All @@ -267,8 +259,6 @@ export class Notification implements INotification {
* @returns { TNotification | undefined }
*/
unArchiveNotification = async (workspaceSlug: string): Promise<TNotification | undefined> => {
if (!this.id) return undefined;

const currentNotificationArchivedAt = this.archived_at;
try {
const payload: Partial<TNotification> = {
Expand All @@ -293,8 +283,6 @@ export class Notification implements INotification {
* @returns { TNotification | undefined }
*/
snoozeNotification = async (workspaceSlug: string, snoozeTill: Date): Promise<TNotification | undefined> => {
if (!this.id) return undefined;

const currentNotificationSnoozeTill = this.snoozed_till;
try {
const payload: Partial<TNotification> = {
Expand All @@ -315,8 +303,6 @@ export class Notification implements INotification {
* @returns { TNotification | undefined }
*/
unSnoozeNotification = async (workspaceSlug: string): Promise<TNotification | undefined> => {
if (!this.id) return undefined;

const currentNotificationSnoozeTill = this.snoozed_till;
try {
const payload: Partial<TNotification> = {
Expand Down
Loading