Skip to content

Comments

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

Merged
pushya22 merged 8 commits intopreviewfrom
fix-fav-archive
Aug 21, 2024
Merged

[WEB-1986] fix: remove the user favourites when archived a particular entity#5388
pushya22 merged 8 commits intopreviewfrom
fix-fav-archive

Conversation

@gakshita
Copy link
Collaborator

@gakshita gakshita commented Aug 20, 2024

fix:

This pull request implements the functionality to automatically remove user favourites when a project is archived.
Issue Link: WEB-1986

Summary by CodeRabbit

  • New Features

    • User favorites are now automatically deleted when cycles, modules, or projects are archived, ensuring data integrity and relevance.
    • Enhanced handling of user favorites in the FavoriteFolder and SidebarFavoritesMenu components, improving duplicate management and rendering logic.
  • Bug Fixes

    • Improved error handling by utilizing a centralized error code system, enhancing maintainability and clarity in error responses.
  • Refactor

    • Simplified several methods related to favorites management in the FavoriteStore, ProjectStore, and CycleStore, optimizing performance and reducing complexity.
    • Enhanced the error handling logic in various components to standardize responses and improve clarity.
    • Updated the SidebarFavoritesMenu to streamline the management and display of favorites.
  • Chores

    • Added a new error codes file for better organization and management of application error codes.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 20, 2024

Walkthrough

This update enhances data integrity by automatically deleting user favorites when cycles, modules, projects, and pages are archived. It also introduces a structured error handling system that centralizes error codes for improved maintainability. Overall, these modifications streamline the management of user favorites and bolster the application's robustness.

Changes

Files Change Summary
apiserver/plane/api/views/cycle.py, module.py, project.py, archive.py Added logic to delete user favorites upon archiving cycles, modules, and projects, enhancing data integrity and relevance in user preferences.
apiserver/plane/app/views/issue/archive.py, bulk_operations.py, page/base.py Refactored error handling to utilize centralized error codes, enhancing clarity and maintainability in error responses.
apiserver/plane/utils/error_codes.py Introduced a new dictionary for error codes, categorizing them for better management and readability.
web/core/components/workspace/sidebar/favorites/favorite-folder.tsx, favorites-menu.tsx Modified favorite management logic, renamed variables, and improved handling of duplicates for an enhanced user experience.
web/core/store/favorite.store.ts, project/project.store.ts Enhanced FavoriteStore with a hierarchical structure for favorites and updated project archiving logic to remove archived projects from favorites.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CycleModule
    participant UserFavorite
    User->>CycleModule: Archive Cycle
    CycleModule->>UserFavorite: Delete associated favorites
    UserFavorite->>UserFavorite: Confirm deletion
    CycleModule->>User: Confirm Cycle archived
Loading
sequenceDiagram
    participant User
    participant ModuleModule
    participant UserFavorite
    User->>ModuleModule: Archive Module
    ModuleModule->>UserFavorite: Delete associated favorites
    UserFavorite->>UserFavorite: Confirm deletion
    ModuleModule->>User: Confirm Module archived
Loading

🐇✨
In the fields where favorites bloom,
Archiving cycles clears the gloom.
With tidy spaces, joy we find,
As user preferences unwind.
Code refactored, bright and clear,
A happy rabbit hops in cheer! 🐇🌼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Outside diff range, codebase verification and nitpick comments (4)
web/core/components/workspace/sidebar/favorites/favorites-menu.tsx (2)

Line range hint 173-204: Simplify conditional rendering.

The conditional rendering of groupedFavorites can be simplified by directly checking its length. This improves readability.

- {Object.keys(groupedFavorites).length === 0 ? (
+ {Object.keys(groupedFavorites).length ? (

36-36: Update variable names consistently across the codebase.

The variable favoriteMap is still being used in several files, including web/core/store/favorite.store.ts, web/core/components/workspace/sidebar/favorites/favorites.helpers.ts, and web/core/components/workspace/sidebar/favorites/favorite-items/root.tsx. Ensure that all instances are updated to groupedFavorites to maintain consistency and prevent potential issues.

  • web/core/store/favorite.store.ts
  • web/core/components/workspace/sidebar/favorites/favorites.helpers.ts
  • web/core/components/workspace/sidebar/favorites/favorite-items/root.tsx
Analysis chain

Update variable names consistently.

The variable favoriteMap has been updated to groupedFavorites. Ensure that all references to favoriteMap are updated throughout the codebase to prevent any potential issues.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to `favoriteMap` are updated to `groupedFavorites`.

# Test: Search for occurrences of `favoriteMap`. Expect: No results.
rg --type tsx 'favoriteMap'

Length of output: 55


Script:

#!/bin/bash
# Description: Verify all references to `favoriteMap` are updated to `groupedFavorites`.

# Test: Search for occurrences of `favoriteMap`. Expect: No results.
rg 'favoriteMap'

Length of output: 4469

web/core/store/favorite.store.ts (1)

85-100: Optimize deep copy operation.

The deep copy operation using JSON.parse(JSON.stringify(...)) can be inefficient for large datasets. Consider using a more efficient method if performance becomes an issue.

web/core/components/workspace/sidebar/favorites/favorite-folder.tsx (1)

41-41: Instances of favoriteMap Still Exist

The renaming from favoriteMap to groupedFavorites is incomplete. There are multiple instances of favoriteMap still present in the codebase. Please ensure that all occurrences are updated to groupedFavorites to prevent any potential errors.

  • File: web/core/store/favorite.store.ts
  • File: web/core/components/workspace/sidebar/favorites/favorites-menu.tsx
  • File: web/core/components/workspace/sidebar/favorites/favorites.helpers.ts
  • File: web/core/components/workspace/sidebar/favorites/favorite-items/root.tsx
Analysis chain

Renaming favoriteMap to groupedFavorites.

The renaming of favoriteMap to groupedFavorites is consistent throughout the file. Ensure that all references are updated to prevent errors.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of `favoriteMap` have been renamed to `groupedFavorites`.

# Test: Search for any remaining instances of `favoriteMap`. Expect: No results.
rg --type typescript 'favoriteMap'

Length of output: 69


Script:

#!/bin/bash
# Description: Verify all instances of `favoriteMap` have been renamed to `groupedFavorites`.

# Test: Search for any remaining instances of `favoriteMap`. Expect: No results.
rg --type ts 'favoriteMap'

Length of output: 4479

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6748065 and 451e990.

Files selected for processing (14)
  • apiserver/plane/api/views/cycle.py (1 hunks)
  • apiserver/plane/api/views/module.py (1 hunks)
  • apiserver/plane/api/views/project.py (1 hunks)
  • apiserver/plane/app/views/cycle/archive.py (1 hunks)
  • apiserver/plane/app/views/issue/archive.py (2 hunks)
  • apiserver/plane/app/views/issue/bulk_operations.py (4 hunks)
  • apiserver/plane/app/views/module/archive.py (1 hunks)
  • apiserver/plane/app/views/page/base.py (4 hunks)
  • apiserver/plane/app/views/project/base.py (1 hunks)
  • apiserver/plane/utils/error_codes.py (1 hunks)
  • web/core/components/workspace/sidebar/favorites/favorite-folder.tsx (5 hunks)
  • web/core/components/workspace/sidebar/favorites/favorites-menu.tsx (5 hunks)
  • web/core/store/favorite.store.ts (11 hunks)
  • web/core/store/project/project.store.ts (1 hunks)
Files skipped from review due to trivial changes (1)
  • apiserver/plane/utils/error_codes.py
Additional context used
Biome
web/core/store/favorite.store.ts

[error] 240-240: 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] 317-321: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

Additional comments not posted (20)
apiserver/plane/app/views/issue/bulk_operations.py (4)

24-24: Good practice: Centralized error codes.

Importing ERROR_CODES centralizes error management, enhancing maintainability and readability. This is a positive change.


169-171: Ensure all error codes are defined.

The error code INVALID_ISSUE_TARGET_DATE is used here. Verify that it is defined in ERROR_CODES to prevent runtime errors.

Verification successful

Error code INVALID_ISSUE_TARGET_DATE is defined. The error code INVALID_ISSUE_TARGET_DATE is correctly defined in apiserver/plane/utils/error_codes.py with the value 4102. There are no issues related to its definition.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that `INVALID_ISSUE_TARGET_DATE` is defined in `ERROR_CODES`.

# Test: Search for `INVALID_ISSUE_TARGET_DATE` in error_codes. Expect: At least one result.
rg 'INVALID_ISSUE_TARGET_DATE' --type py

Length of output: 352


76-78: Ensure all error codes are defined.

The error code INVALID_ISSUE_DATES is used here. Verify that it is defined in ERROR_CODES to prevent runtime errors.

Verification successful

Error code INVALID_ISSUE_DATES is defined. The error code INVALID_ISSUE_DATES is correctly defined in ERROR_CODES with a value of 4100, ensuring no runtime errors related to undefined error codes.

  • Location: apiserver/plane/utils/error_codes.py
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that `INVALID_ISSUE_DATES` is defined in `ERROR_CODES`.

# Test: Search for `INVALID_ISSUE_DATES` in error_codes. Expect: At least one result.
rg 'INVALID_ISSUE_DATES' --type py

Length of output: 344


133-135: Ensure all error codes are defined.

The error code INVALID_ISSUE_START_DATE is used here. Verify that it is defined in ERROR_CODES to prevent runtime errors.

Verification successful

Error code INVALID_ISSUE_START_DATE is defined.

The error code INVALID_ISSUE_START_DATE is correctly defined in ERROR_CODES within apiserver/plane/utils/error_codes.py. No further action is required.

  • Definition found in apiserver/plane/utils/error_codes.py.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that `INVALID_ISSUE_START_DATE` is defined in `ERROR_CODES`.

# Test: Search for `INVALID_ISSUE_START_DATE` in error_codes. Expect: At least one result.
rg 'INVALID_ISSUE_START_DATE' --type py

Length of output: 348

web/core/store/favorite.store.ts (1)

21-21: Ensure groupedFavorites is correctly implemented.

The groupedFavorites property has been added to the interface. Ensure that its implementation is consistent across the codebase.

Verification successful

Implementation of groupedFavorites is consistent and correct.

The groupedFavorites property is consistently used across multiple components as a computed property. Its implementation and usage align with its intended design, ensuring proper management of favorites.

  • File: web/core/store/favorite.store.ts

    • Declared as a computed property.
    • Accessed using get groupedFavorites().
  • Components:

    • favorites-menu.tsx and favorite-folder.tsx:
      • Correctly access groupedFavorites from the store.
      • Utilize it for rendering and logic, consistent with its design.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify `groupedFavorites` is implemented consistently across the codebase.

# Test: Search for `groupedFavorites` usage. Expect: Consistent implementation.
rg --type ts 'groupedFavorites'

Length of output: 1653

apiserver/plane/app/views/issue/archive.py (2)

50-50: Good practice: Centralized error codes.

Importing ERROR_CODES centralizes error management, enhancing maintainability and readability. This is a positive change.


349-351: Ensure all error codes are defined.

The error code INVALID_ARCHIVE_STATE_GROUP is used here. Verify that it is defined in ERROR_CODES to prevent runtime errors.

Verification successful

Error code INVALID_ARCHIVE_STATE_GROUP is defined.

The error code INVALID_ARCHIVE_STATE_GROUP is correctly defined in the ERROR_CODES dictionary, preventing potential runtime errors. No further action is required.

  • Definition found in apiserver/plane/utils/error_codes.py.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that `INVALID_ARCHIVE_STATE_GROUP` is defined in `ERROR_CODES`.

# Test: Search for `INVALID_ARCHIVE_STATE_GROUP` in error_codes. Expect: At least one result.
rg 'INVALID_ARCHIVE_STATE_GROUP' --type py

Length of output: 336

apiserver/plane/api/views/project.py (1)

380-383: Ensure deletion operation aligns with business logic.

The deletion of user favorites associated with the project is correctly placed after the project is archived. Verify that this operation aligns with the overall business logic and does not inadvertently affect other functionalities.

web/core/components/workspace/sidebar/favorites/favorite-folder.tsx (2)

8-8: Use of uniqBy to filter duplicates.

The addition of uniqBy effectively filters out duplicate favorites based on the id property, which enhances data integrity and user experience.


319-326: Ensure groupedFavorites is correctly used.

The use of groupedFavorites in the mapping of child favorites ensures that the updated data structure is consistently applied. Verify that this change does not affect the logic elsewhere in the component.

Verification successful

groupedFavorites is consistently used across components.

The groupedFavorites variable is used consistently in the codebase, primarily for managing favorite items and passing them as props. There are no indications of logic errors or misuse affecting other components.

  • Files where groupedFavorites is used:
    • web/core/store/favorite.store.ts
    • web/core/components/workspace/sidebar/favorites/favorites-menu.tsx
    • web/core/components/workspace/sidebar/favorites/favorite-folder.tsx
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `groupedFavorites` in related components.

# Test: Search for all uses of `groupedFavorites`. Expect: Consistent application across components.
rg --type typescript 'groupedFavorites'

Length of output: 74


Script:

#!/bin/bash
# Description: Verify the usage of `groupedFavorites` in the codebase.

# Test: Search for all occurrences of `groupedFavorites` in the codebase.
rg 'groupedFavorites'

Length of output: 1643

web/core/store/project/project.store.ts (1)

421-421: Remove project from favorites when archived.

The addition of removeFavoriteFromStore ensures that a project is removed from favorites when archived. This change aligns with the intended functionality and maintains data integrity.

apiserver/plane/app/views/page/base.py (4)

489-493: Improved error handling for missing pages.

The addition of a 404 response for missing pages improves error handling by providing clearer feedback to the client.


528-532: Structured error response for locked pages.

The use of ERROR_CODES for locked pages provides a standardized error response, enhancing clarity and maintainability.


537-541: Structured error response for archived pages.

The use of ERROR_CODES for archived pages ensures consistent and clear error handling.


308-313: Ensure deletion of user favorites is correctly implemented.

The deletion of user favorites associated with a page is correctly placed before archiving. Verify that this operation aligns with the business logic and does not introduce unintended side effects.

apiserver/plane/app/views/module/archive.py (1)

578-583: LGTM! But verify the impact of the UserFavorite deletion logic.

The code changes are approved.

However, ensure that the deletion of UserFavorite entries does not have unintended side effects elsewhere in the application.

apiserver/plane/app/views/cycle/archive.py (1)

610-615: LGTM! But verify the impact of the UserFavorite deletion logic.

The code changes are approved.

However, ensure that the deletion of UserFavorite entries does not have unintended side effects elsewhere in the application.

Verification successful

Deletion of UserFavorite entries is a common practice across the codebase.

The UserFavorite.objects.filter(...).delete() pattern is consistently used in various modules and views for different entity types, indicating that this is a standard approach for handling user favorites. This suggests that the impact of such deletions is likely well-managed within the application.

  • Occurrences found in multiple files such as favorite.py, workspace/favorite.py, module/base.py, project/base.py, view/base.py, page/base.py, cycle/base.py, cycle/archive.py, and several others.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `UserFavorite` deletion logic.

# Test: Search for the `UserFavorite` deletion logic. Expect: Occurrences where the logic is used or referenced.
rg --type python -A 5 $'UserFavorite.objects.filter'

Length of output: 82


Script:

#!/bin/bash
# Description: Verify the usage of `UserFavorite` deletion logic.

# Test: Search for the `UserFavorite` deletion logic in Python files.
rg --glob '*.py' -A 5 $'UserFavorite.objects.filter'

Length of output: 12052

apiserver/plane/api/views/module.py (1)

637-642: LGTM! But verify the impact of the UserFavorite deletion logic.

The code changes are approved.

However, ensure that the deletion of UserFavorite entries does not have unintended side effects elsewhere in the application.

apiserver/plane/app/views/project/base.py (1)

496-499: LGTM! But verify the impact of the UserFavorite deletion logic.

The code changes are approved.

However, ensure that the deletion of UserFavorite entries does not have unintended side effects elsewhere in the application.

Verification successful

Verification Successful: Consistent Use of UserFavorite Deletion Logic

The deletion of UserFavorite entries is a common operation across various modules, indicating that the change is consistent with existing patterns in the codebase. No unintended side effects were found.

  • Locations include modules, cycles, projects, views, and workspaces.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `UserFavorite` deletion logic.

# Test: Search for the `UserFavorite` deletion logic. Expect: Occurrences where the logic is used or referenced.
rg --type python -A 5 $'UserFavorite.objects.filter'

Length of output: 82


Script:

#!/bin/bash
# Description: Verify the usage of `UserFavorite` deletion logic.

# Test: Search for the `UserFavorite` deletion logic. Expect: Occurrences where the logic is used or referenced.
rg 'UserFavorite.objects.filter' --glob '*.py' -A 5

Length of output: 12052

apiserver/plane/api/views/cycle.py (1)

547-552: Verify the necessity of deleting user favorites.

Ensure that removing user favorites when a cycle is archived aligns with the intended functionality. Confirm that this operation is necessary and doesn't inadvertently remove favorites that should remain for other purposes.

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";
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove unused import.

The import statement for orderBy from lodash is present but not used in the file. Consider removing it to clean up the code.

-import { orderBy } from "lodash";
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
import { orderBy } from "lodash";

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

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;

return (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]!.is_favorite = false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid assignment in expressions.

The assignment within the return statement can be confusing. Consider refactoring to improve clarity.

- return (this.projectStore.projectMap[entity_identifier]!.is_favorite = false);
+ this.projectStore.projectMap[entity_identifier]!.is_favorite = false;
+ 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
return (this.projectStore.projectMap[entity_identifier]!.is_favorite = false);
this.projectStore.projectMap[entity_identifier]!.is_favorite = false;
return;
Tools
Biome

[error] 240-240: 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)

Comment on lines +317 to +321
projectData &&
projectData.forEach(async (fav) => {
this.removeFavoriteFromStore(fav.entity_identifier!);
this.removeFavoriteEntityFromStore(fav.entity_identifier!, fav.entity_type);
});
Copy link
Contributor

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)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Outside diff range, codebase verification and nitpick comments (1)
web/core/store/favorite.store.ts (1)

Ensure safe access to groupedFavorites

The direct access to groupedFavorites[favoriteId].children in favorite.store.ts could lead to runtime errors if groupedFavorites[favoriteId] is undefined. Consider adding a check to ensure groupedFavorites[favoriteId] is defined before accessing its properties.

  • Location:
    • web/core/store/favorite.store.ts: Access to groupedFavorites[favoriteId].children without a safety check.
Analysis chain

Line range hint 265-283: Ensure groupedFavorites usage is safe.

The groupedFavorites property is used to access children. Ensure that it is correctly populated and used safely to avoid runtime errors.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify safe usage of `groupedFavorites`.

# Test: Search for `groupedFavorites` usage. Expect: Safe access and handling of potential undefined values.
rg --type typescript 'this.groupedFavorites'

Length of output: 79


Script:

#!/bin/bash
# Description: Verify safe usage of `groupedFavorites`.

# Test: Search for `groupedFavorites` usage in TypeScript files. Expect: Safe access and handling of potential undefined values.
rg --type-add 'ts:*.ts' --type ts 'this.groupedFavorites'

Length of output: 155

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 451e990 and dfc5df1.

Files selected for processing (5)
  • web/core/components/dropdowns/member/member-options.tsx (1 hunks)
  • web/core/store/cycle.store.ts (1 hunks)
  • web/core/store/favorite.store.ts (11 hunks)
  • web/core/store/module.store.ts (1 hunks)
  • web/core/store/pages/page.ts (1 hunks)
Files skipped from review due to trivial changes (1)
  • web/core/components/dropdowns/member/member-options.tsx
Additional context used
Biome
web/core/store/favorite.store.ts

[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)


[error] 325-329: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

Additional comments not posted (6)
web/core/store/favorite.store.ts (3)

325-329: 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) => {
Tools
Biome

[error] 325-329: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


61-61: Ensure groupedFavorites is properly computed.

The groupedFavorites is marked as a computed property. Ensure it is correctly computed and used without side effects.

Verification successful

groupedFavorites is correctly computed and used without side effects.

The groupedFavorites property is defined as a computed property and is used appropriately across the codebase. It is computed by transforming favoriteMap into a structured object, and its usage in components is read-only, focusing on rendering and UI logic.

  • Definition: The getter method for groupedFavorites processes favoriteMap to create a structured object without side effects.
  • Usage: In components like favorites-menu.tsx and favorite-folder.tsx, it is used for rendering and conditional logic, ensuring no side effects occur.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify computation of `groupedFavorites`.

# Test: Search for `groupedFavorites` computation. Expect: Proper computation without side effects.
rg --type typescript 'get groupedFavorites'

Length of output: 80


Script:

#!/bin/bash
# Description: Verify computation of `groupedFavorites`.

# Search for the definition of `groupedFavorites` as a computed property in TypeScript files.
rg 'groupedFavorites' --type-add 'ts:*.ts' --type ts -A 5

# Search for the usage of `groupedFavorites` in the codebase to ensure it is used without side effects.
rg 'groupedFavorites' --type-add 'ts:*.ts' --type ts -A 5

Length of output: 15423


21-21: Ensure groupedFavorites is correctly initialized.

The groupedFavorites property is introduced in the interface and class. Verify that it is correctly initialized and used throughout the codebase.

web/core/store/pages/page.ts (1)

446-446: Ensure removal of page from favorites is correct.

The conditional statement checks if the page is in the favorites before removing it. Verify that this logic integrates well with the existing favorite management system.

Verification successful

Integration of removeFavoriteFromStore is correct.

The removeFavoriteFromStore method is well-integrated with the favorite management system, handling the removal of favorites appropriately by updating relevant data structures. The conditional check ensures the method is invoked only when the entity is present in the favorites. No issues found.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify removal of page from favorites when archived.

# Test: Search for `removeFavoriteFromStore` usage in the context of pages. Expect: Proper integration with the favorite management system.
rg --type typescript 'removeFavoriteFromStore.*this\.id'

Length of output: 93


Script:

#!/bin/bash
# Description: Verify removal of page from favorites when archived.

# Test: Search for `removeFavoriteFromStore` usage in the context of pages. Expect: Proper integration with the favorite management system.
rg 'removeFavoriteFromStore.*this\.id'

Length of output: 179


Script:

#!/bin/bash
# Description: Find the implementation of `removeFavoriteFromStore` to verify its integration with the favorite management system.

# Search for the implementation of `removeFavoriteFromStore`.
ast-grep --lang typescript --pattern 'removeFavoriteFromStore($_)' 

Length of output: 68


Script:

#!/bin/bash
# Description: Search for the definition of `removeFavoriteFromStore` to verify its integration with the favorite management system.

# Search for the definition of `removeFavoriteFromStore`.
rg 'removeFavoriteFromStore' -A 10

Length of output: 8653

web/core/store/module.store.ts (1)

560-560: Ensure removal of module from favorites is correct.

The conditional statement checks if the module is in the favorites before removing it. Verify that this logic integrates well with the existing favorite management system.

Verification successful

Removal of module from favorites is correctly integrated.

The logic to remove a module from favorites is appropriately safeguarded by checking its presence in the entityMap before invoking removeFavoriteFromStore. This ensures proper integration with the favorite management system.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify removal of module from favorites when archived.

# Test: Search for `removeFavoriteFromStore` usage in the context of modules. Expect: Proper integration with the favorite management system.
rg --type typescript 'removeFavoriteFromStore.*moduleId'

Length of output: 93


Script:

#!/bin/bash
# Description: Verify removal of module from favorites when archived.

# Test: Search for `removeFavoriteFromStore` usage in the context of modules. Expect: Proper integration with the favorite management system.
rg 'removeFavoriteFromStore.*moduleId' --glob '*.ts'

Length of output: 350

web/core/store/cycle.store.ts (1)

627-627: Ensure removal of cycle from favorites is correct.

The conditional statement checks if the cycle is in the favorites before removing it. Verify that this logic integrates well with the existing favorite management system.

Verification successful

Verified: Removal of cycle from favorites is correctly integrated.

The removeFavoriteFromStore method is used with a conditional check to ensure a cycle is in the favorites before removal. This logic is consistent and appears to be well-integrated with the favorite management system.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify removal of cycle from favorites when archived.

# Test: Search for `removeFavoriteFromStore` usage in the context of cycles. Expect: Proper integration with the favorite management system.
rg --type typescript 'removeFavoriteFromStore.*cycleId'

Length of output: 92


Script:

#!/bin/bash
# Description: Verify removal of cycle from favorites when archived.

# Test: Search for `removeFavoriteFromStore` usage in TypeScript files. Expect: Proper integration with the favorite management system.
rg 'removeFavoriteFromStore.*cycleId' --glob '*.ts' -A 5

Length of output: 849

Comment on lines +85 to +100
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;
}
Copy link
Contributor

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;
}

Comment on lines +233 to +251
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)
);
Copy link
Contributor

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)

@NarayanBavisetti NarayanBavisetti changed the base branch from preview to chore/project-archive August 20, 2024 13:32
Base automatically changed from chore/project-archive to preview August 20, 2024 14:10
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between dfc5df1 and dbd6426.

Files selected for processing (1)
  • web/core/components/workspace/sidebar/favorites/favorites-menu.tsx (5 hunks)
Files skipped from review as they are similar to previous changes (1)
  • web/core/components/workspace/sidebar/favorites/favorites-menu.tsx

@pushya22 pushya22 merged commit 5fc99c9 into preview Aug 21, 2024
@SatishGandham SatishGandham deleted the fix-fav-archive branch August 21, 2024 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants