Skip to content

[WEB-1986] fix: remove the user favourites when archived a particular entity #5388

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 8 commits into from
Aug 21, 2024
Merged

[WEB-1986] fix: remove the user favourites when archived a particular entity #5388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2de3c38
chore: pages custom error codes
NarayanBavisetti Jun 26, 2024
9203e3d
fix: archive project favorites
NarayanBavisetti Aug 20, 2024
cf756d9
fix: project archive issue
gakshita Aug 20, 2024
451e990
Merge branch 'chore/project-archive' of https://github.com/makeplane/…
gakshita Aug 20, 2024
dfc5df1
fix: delete issue + dropdown z-index fix
gakshita Aug 20, 2024
4e23845
Merge branch 'preview' of github.com:makeplane/plane into fix-fav-arc…
NarayanBavisetti Aug 20, 2024
696db60
fix: import issue
gakshita Aug 20, 2024
dbd6426
Merge branch 'fix-fav-archive' of https://github.com/makeplane/plane …
gakshita Aug 20, 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 web/core/components/dropdowns/member/member-options.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export const MemberOptions = observer((props: Props) => {
return createPortal(
<Combobox.Options data-prevent-outside-click static>
<div
className="my-1 w-48 rounded border-[0.5px] border-custom-border-300 bg-custom-background-100 px-2 py-2.5 text-xs shadow-custom-shadow-rg focus:outline-none z-20"
className="my-1 w-48 rounded border-[0.5px] border-custom-border-300 bg-custom-background-100 px-2 py-2.5 text-xs shadow-custom-shadow-rg focus:outline-none z-[19]"
ref={setPopperElement}
style={{
...styles.popper,
Expand Down
13 changes: 8 additions & 5 deletions web/core/components/workspace/sidebar/favorites/favorite-folder.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { combine } from "@atlaskit/pragmatic-drag-and-drop/combine";
import { draggable, dropTargetForElements } from "@atlaskit/pragmatic-drag-and-drop/element/adapter";

import { attachClosestEdge, extractClosestEdge } from "@atlaskit/pragmatic-drag-and-drop-hitbox/closest-edge";
import uniqBy from "lodash/uniqBy";
import { useParams } from "next/navigation";
import { PenSquare, Star, MoreHorizontal, ChevronRight, GripVertical } from "lucide-react";
import { Disclosure, Transition } from "@headlessui/react";
Expand Down Expand Up @@ -37,7 +38,7 @@ export const FavoriteFolder: React.FC<Props> = (props) => {
const { sidebarCollapsed: isSidebarCollapsed } = useAppTheme();

const { isMobile } = usePlatformOS();
const { moveFavorite, getGroupedFavorites, favoriteMap, moveFavoriteFolder } = useFavorite();
const { moveFavorite, getGroupedFavorites, groupedFavorites, moveFavoriteFolder } = useFavorite();
const { workspaceSlug } = useParams();
// states
const [isMenuActive, setIsMenuActive] = useState(false);
Expand Down Expand Up @@ -110,7 +111,9 @@ export const FavoriteFolder: React.FC<Props> = (props) => {
const edge = extractClosestEdge(destinationData) || undefined;
const payload = {
id: favorite.id,
sequence: Math.round(getDestinationStateSequence(favoriteMap, destinationData.id as string, edge) || 0),
sequence: Math.round(
getDestinationStateSequence(groupedFavorites, destinationData.id as string, edge) || 0
),
};

handleOnDropFolder(payload);
Expand Down Expand Up @@ -146,7 +149,7 @@ export const FavoriteFolder: React.FC<Props> = (props) => {
if (source.data.is_folder) return;
if (sourceId === destinationId) return;
if (!sourceId || !destinationId) return;
if (favoriteMap[sourceId].parent === destinationId) return;
if (groupedFavorites[sourceId].parent === destinationId) return;
handleOnDrop(sourceId, destinationId);
},
})
Expand Down Expand Up @@ -313,14 +316,14 @@ export const FavoriteFolder: React.FC<Props> = (props) => {
"px-2": !isSidebarCollapsed,
})}
>
{favorite.children.map((child) => (
{uniqBy(favorite.children, "id").map((child) => (
<FavoriteRoot
key={child.id}
workspaceSlug={workspaceSlug.toString()}
favorite={child}
handleRemoveFromFavorites={handleRemoveFromFavorites}
handleRemoveFromFavoritesFolder={handleRemoveFromFavoritesFolder}
favoriteMap={favoriteMap}
favoriteMap={groupedFavorites}
/>
))}
</Disclosure.Panel>
Expand Down
12 changes: 6 additions & 6 deletions web/core/components/workspace/sidebar/favorites/favorites-menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import React, { useEffect, useRef, useState } from "react";
import { combine } from "@atlaskit/pragmatic-drag-and-drop/combine";
import { dropTargetForElements } from "@atlaskit/pragmatic-drag-and-drop/element/adapter";
import { orderBy, uniqBy } from "lodash";
import orderBy from "lodash/orderBy";
import { observer } from "mobx-react";
import { useParams } from "next/navigation";
import { ChevronRight, FolderPlus } from "lucide-react";
Expand Down Expand Up @@ -33,7 +33,7 @@ export const SidebarFavoritesMenu = observer(() => {

// store hooks
const { sidebarCollapsed } = useAppTheme();
const { favoriteIds, favoriteMap, deleteFavorite, removeFromFavoriteFolder } = useFavorite();
const { favoriteIds, groupedFavorites, deleteFavorite, removeFromFavoriteFolder } = useFavorite();
const { workspaceSlug } = useParams();

const { isMobile } = usePlatformOS();
Expand Down Expand Up @@ -108,7 +108,7 @@ export const SidebarFavoritesMenu = observer(() => {
setIsDragging(false);
const sourceId = source?.data?.id as string | undefined;
console.log({ sourceId });
if (!sourceId || !favoriteMap[sourceId].parent) return;
if (!sourceId || !groupedFavorites[sourceId].parent) return;
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 20, 2024

Choose a reason for hiding this comment

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

Ensure groupedFavorites access is safe.

The code assumes that groupedFavorites[sourceId] exists. Consider adding a check to ensure sourceId is valid and groupedFavorites contains it to prevent runtime errors.

- if (!sourceId || !groupedFavorites[sourceId].parent) return;
+ if (!sourceId || !groupedFavorites[sourceId] || !groupedFavorites[sourceId].parent) return;
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
if (!sourceId || !groupedFavorites[sourceId].parent) return;
if (!sourceId || !groupedFavorites[sourceId] || !groupedFavorites[sourceId].parent) return;

handleRemoveFromFavoritesFolder(sourceId);
},
})
Expand Down Expand Up @@ -170,14 +170,14 @@ export const SidebarFavoritesMenu = observer(() => {
static
>
{createNewFolder && <NewFavoriteFolder setCreateNewFolder={setCreateNewFolder} actionType="create" />}
{Object.keys(favoriteMap).length === 0 ? (
{Object.keys(groupedFavorites).length === 0 ? (
<>
{!sidebarCollapsed && (
<span className="text-custom-text-400 text-xs text-center font-medium py-1">No favorites yet</span>
)}
</>
) : (
uniqBy(orderBy(Object.values(favoriteMap), "sequence", "desc"), "id")
orderBy(Object.values(groupedFavorites), "sequence", "desc")
.filter((fav) => !fav.parent)
.map((fav, index) => (
<Tooltip
Expand All @@ -201,7 +201,7 @@ export const SidebarFavoritesMenu = observer(() => {
favorite={fav}
handleRemoveFromFavorites={handleRemoveFromFavorites}
handleRemoveFromFavoritesFolder={handleRemoveFromFavoritesFolder}
favoriteMap={favoriteMap}
favoriteMap={groupedFavorites}
/>
)}
</Tooltip>
Expand Down
1 change: 1 addition & 0 deletions web/core/store/cycle.store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,7 @@ export class CycleStore implements ICycleStore {
.then((response) => {
runInAction(() => {
set(this.cycleMap, [cycleId, "archived_at"], response.archived_at);
if (this.rootStore.favorite.entityMap[cycleId]) this.rootStore.favorite.removeFavoriteFromStore(cycleId);
});
})
.catch((error) => {
Expand Down
128 changes: 52 additions & 76 deletions web/core/store/favorite.store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export interface IFavoriteStore {
};
// computed actions
existingFolders: string[];
groupedFavorites: { [favoriteId: string]: IFavorite };
// actions
fetchFavorite: (workspaceSlug: string) => Promise<IFavorite[]>;
// CRUD actions
Expand Down Expand Up @@ -57,6 +58,7 @@ export class FavoriteStore implements IFavoriteStore {
favoriteIds: observable,
//computed
existingFolders: computed,
groupedFavorites: computed,
// action
fetchFavorite: action,
// CRUD actions
Expand All @@ -80,6 +82,23 @@ export class FavoriteStore implements IFavoriteStore {
return Object.values(this.favoriteMap).map((fav) => fav.name);
}

get groupedFavorites() {
const data: { [favoriteId: string]: IFavorite } = JSON.parse(JSON.stringify(this.favoriteMap));

Object.values(data).forEach((fav) => {
if (fav.parent && data[fav.parent]) {
if (data[fav.parent].children) {
if (!data[fav.parent].children.some((f) => f.id === fav.id)) {
data[fav.parent].children.push(fav);
}
} else {
data[fav.parent].children = [fav];
}
}
});
return data;
}
Comment on lines +85 to +100
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 20, 2024

Choose a reason for hiding this comment

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

Optimize groupedFavorites computation.

The computation of groupedFavorites could be optimized by reducing the number of operations within the loop. Consider using a map to store parent-child relationships more efficiently.

get groupedFavorites() {
  const data: { [favoriteId: string]: IFavorite } = JSON.parse(JSON.stringify(this.favoriteMap));

  const parentMap: { [parentId: string]: IFavorite[] } = {};

  Object.values(data).forEach((fav) => {
    if (fav.parent) {
      if (!parentMap[fav.parent]) {
        parentMap[fav.parent] = [];
      }
      parentMap[fav.parent].push(fav);
    }
  });

  Object.keys(parentMap).forEach((parentId) => {
    data[parentId].children = parentMap[parentId];
  });

  return data;
}


/**
* Creates a favorite in the workspace and adds it to the store
* @param workspaceSlug
Expand Down Expand Up @@ -151,22 +170,8 @@ export class FavoriteStore implements IFavoriteStore {
*/
moveFavorite = async (workspaceSlug: string, favoriteId: string, data: Partial<IFavorite>) => {
const oldParent = this.favoriteMap[favoriteId].parent;
const favorite = this.favoriteMap[favoriteId];
try {
runInAction(() => {
// add the favorite to the new parent
if (!data.parent) return;
set(this.favoriteMap, [data.parent, "children"], [favorite, ...this.favoriteMap[data.parent].children]);

// remove the favorite from the old parent
if (oldParent) {
set(
this.favoriteMap,
[oldParent, "children"],
this.favoriteMap[oldParent].children.filter((child) => child.id !== favoriteId)
);
}

// add parent of the favorite
set(this.favoriteMap, [favoriteId, "parent"], data.parent);
});
Expand All @@ -177,21 +182,6 @@ export class FavoriteStore implements IFavoriteStore {
// revert the changes
runInAction(() => {
if (!data.parent) return;
// remove the favorite from the new parent
set(
this.favoriteMap,
[data.parent, "children"],
this.favoriteMap[data.parent].children.filter((child) => child.id !== favoriteId)
);

// add the favorite back to the old parent
if (oldParent) {
set(
this.favoriteMap,
[oldParent, "children"],
[...this.favoriteMap[oldParent].children, this.favoriteMap[favoriteId]]
);
}

// revert the parent
set(this.favoriteMap, [favoriteId, "parent"], oldParent);
Expand Down Expand Up @@ -223,28 +213,13 @@ export class FavoriteStore implements IFavoriteStore {
runInAction(() => {
//remove parent
set(this.favoriteMap, [favoriteId, "parent"], null);

//remove children from parent
if (parent) {
set(
this.favoriteMap,
[parent, "children"],
this.favoriteMap[parent].children.filter((child) => child.id !== favoriteId)
);
}
});
await this.favoriteService.updateFavorite(workspaceSlug, favoriteId, data);
} catch (error) {
console.error("Failed to move favorite");
runInAction(() => {
set(this.favoriteMap, [favoriteId, "parent"], parent);
if (parent) {
set(
this.favoriteMap,
[parent, "children"],
[...this.favoriteMap[parent].children, this.favoriteMap[favoriteId]]
);
}

throw error;
});
throw error;
Expand All @@ -254,15 +229,26 @@ export class FavoriteStore implements IFavoriteStore {
removeFavoriteEntityFromStore = (entity_identifier: string, entity_type: string) => {
switch (entity_type) {
case "view":
return (this.viewStore.viewMap[entity_identifier].is_favorite = false);
return (
this.viewStore.viewMap[entity_identifier] && (this.viewStore.viewMap[entity_identifier].is_favorite = false)
);
case "module":
return (this.moduleStore.moduleMap[entity_identifier].is_favorite = false);
return (
this.moduleStore.moduleMap[entity_identifier] &&
(this.moduleStore.moduleMap[entity_identifier].is_favorite = false)
);
case "page":
return (this.pageStore.data[entity_identifier].is_favorite = false);
return this.pageStore.data[entity_identifier] && (this.pageStore.data[entity_identifier].is_favorite = false);
case "cycle":
return (this.cycleStore.cycleMap[entity_identifier].is_favorite = false);
return (
this.cycleStore.cycleMap[entity_identifier] &&
(this.cycleStore.cycleMap[entity_identifier].is_favorite = false)
);
case "project":
return (this.projectStore.projectMap[entity_identifier].is_favorite = false);
return (
this.projectStore.projectMap[entity_identifier] &&
(this.projectStore.projectMap[entity_identifier].is_favorite = false)
);
Comment on lines +233 to +251
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 20, 2024

Choose a reason for hiding this comment

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

Avoid assignment in expressions.

Assignments within expressions can be confusing and error-prone. Refactor to separate the assignment from the return statement for clarity.

- return (
-   this.viewStore.viewMap[entity_identifier] && (this.viewStore.viewMap[entity_identifier].is_favorite = false)
- );
+ if (this.viewStore.viewMap[entity_identifier]) {
+   this.viewStore.viewMap[entity_identifier].is_favorite = false;
+ }
+ return;

Apply similar changes to other cases in this switch statement.

Committable suggestion was skipped due to low confidence.

Tools
Biome

[error] 233-233: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

[error] 238-238: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

[error] 241-241: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

[error] 245-245: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

[error] 250-250: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

default:
return;
}
Expand All @@ -276,29 +262,21 @@ export class FavoriteStore implements IFavoriteStore {
*/
deleteFavorite = async (workspaceSlug: string, favoriteId: string) => {
const parent = this.favoriteMap[favoriteId].parent;
const children = this.favoriteMap[favoriteId].children;
const children = this.groupedFavorites[favoriteId].children;
const entity_identifier = this.favoriteMap[favoriteId].entity_identifier;
const initialState = this.favoriteMap[favoriteId];

try {
await this.favoriteService.deleteFavorite(workspaceSlug, favoriteId);
runInAction(() => {
if (parent) {
set(
this.favoriteMap,
[parent, "children"],
this.favoriteMap[parent].children.filter((child) => child.id !== favoriteId)
);
}
delete this.favoriteMap[favoriteId];
entity_identifier && delete this.entityMap[entity_identifier];
this.favoriteIds = this.favoriteIds.filter((id) => id !== favoriteId);
});
await this.favoriteService.deleteFavorite(workspaceSlug, favoriteId);
runInAction(() => {
entity_identifier && this.removeFavoriteEntityFromStore(entity_identifier, initialState.entity_type);
if (children) {
children.forEach((child) => {
console.log(child.entity_type);
if (!child.entity_identifier) return;
this.removeFavoriteEntityFromStore(child.entity_identifier, child.entity_type);
});
Expand Down Expand Up @@ -326,9 +304,6 @@ export class FavoriteStore implements IFavoriteStore {
const initialState = this.entityMap[entityId];
try {
const favoriteId = this.entityMap[entityId].id;
runInAction(() => {
delete this.entityMap[entityId];
});
await this.deleteFavorite(workspaceSlug, favoriteId);
} catch (error) {
console.error("Failed to remove favorite entity from favorite store", error);
Expand All @@ -341,19 +316,22 @@ export class FavoriteStore implements IFavoriteStore {

removeFavoriteFromStore = (entity_identifier: string) => {
try {
const favoriteId = this.entityMap[entity_identifier].id;
const favorite = this.favoriteMap[favoriteId];
const parent = favorite.parent;

const favoriteId = this.entityMap[entity_identifier]?.id;
const oldData = this.favoriteMap[favoriteId];
const projectData = Object.values(this.favoriteMap).filter(
(fav) => fav.project_id === entity_identifier && fav.entity_type !== "project"
);
runInAction(() => {
if (parent) {
set(
this.favoriteMap,
[parent, "children"],
this.favoriteMap[parent].children.filter((child) => child.id !== favoriteId)
);
}
projectData &&
projectData.forEach(async (fav) => {
this.removeFavoriteFromStore(fav.entity_identifier!);
this.removeFavoriteEntityFromStore(fav.entity_identifier!, fav.entity_type);
});
Comment on lines +325 to +329
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 20, 2024

Choose a reason for hiding this comment

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

Use optional chaining for safety.

The code can be made safer by using optional chaining to avoid potential runtime errors if projectData is undefined.

- projectData &&
-   projectData.forEach(async (fav) => {
+ projectData?.forEach(async (fav) => {
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
projectData &&
projectData.forEach(async (fav) => {
this.removeFavoriteFromStore(fav.entity_identifier!);
this.removeFavoriteEntityFromStore(fav.entity_identifier!, fav.entity_type);
});
projectData?.forEach(async (fav) => {
this.removeFavoriteFromStore(fav.entity_identifier!);
this.removeFavoriteEntityFromStore(fav.entity_identifier!, fav.entity_type);
});
Tools
Biome

[error] 317-321: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


if (!favoriteId) return;
delete this.favoriteMap[favoriteId];
this.removeFavoriteEntityFromStore(entity_identifier!, oldData.entity_type);

delete this.entityMap[entity_identifier];
this.favoriteIds = this.favoriteIds.filter((id) => id !== favoriteId);
});
Expand All @@ -373,8 +351,6 @@ export class FavoriteStore implements IFavoriteStore {
try {
const response = await this.favoriteService.getGroupedFavorites(workspaceSlug, favoriteId);
runInAction(() => {
// add children to the favorite
set(this.favoriteMap, [favoriteId, "children"], response);
// add the favorites to the map
response.forEach((favorite) => {
set(this.favoriteMap, [favorite.id], favorite);
Expand Down
1 change: 1 addition & 0 deletions web/core/store/module.store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@ export class ModulesStore implements IModuleStore {
.then((response) => {
runInAction(() => {
set(this.moduleMap, [moduleId, "archived_at"], response.archived_at);
if (this.rootStore.favorite.entityMap[moduleId]) this.rootStore.favorite.removeFavoriteFromStore(moduleId);
});
})
.catch((error) => {
Expand Down
1 change: 1 addition & 0 deletions web/core/store/pages/page.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,7 @@ export class Page implements IPage {
runInAction(() => {
this.archived_at = response.archived_at;
});
if (this.rootStore.favorite.entityMap[this.id]) this.rootStore.favorite.removeFavoriteFromStore(this.id);
};

/**
Expand Down
1 change: 1 addition & 0 deletions web/core/store/project/project.store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ export class ProjectStore implements IProjectStore {
.then((response) => {
runInAction(() => {
set(this.projectMap, [projectId, "archived_at"], response.archived_at);
this.rootStore.favorite.removeFavoriteFromStore(projectId);
});
})
.catch((error) => {
Expand Down