fix: improvements for project types. #5857
Conversation
WalkthroughThe pull request introduces a significant update across several components and services, transitioning from using the Changes
Possibly related PRs
Suggested reviewers
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (7)
web/core/services/project/project.service.ts (3)
18-18: LGTM: Method signature updated correctly.The
createProjectmethod signature has been updated to usePartial<TProject>as input andPromise<TProject>as output, which is consistent with the transition fromIProjecttoTProject. This change maintains type safety and reflects the asynchronous nature of the operation.Consider adding a JSDoc comment to describe the method's purpose and parameters for better documentation:
/** * Creates a new project in the specified workspace. * @param workspaceSlug - The slug of the workspace where the project will be created. * @param data - Partial project data for creation. * @returns A promise that resolves to the created project. */ async createProject(workspaceSlug: string, data: Partial<TProject>): Promise<TProject>
54-54: LGTM: Method signature updated correctly.The
updateProjectmethod signature has been updated to usePartial<TProject>as input andPromise<TProject>as output, which is consistent with the transition fromIProjecttoTProject. This change maintains type safety and reflects the asynchronous nature of the operation.Consider adding a JSDoc comment to describe the method's purpose and parameters for better documentation:
/** * Updates an existing project in the specified workspace. * @param workspaceSlug - The slug of the workspace containing the project. * @param projectId - The ID of the project to update. * @param data - Partial project data for update. * @returns A promise that resolves to the updated project. */ async updateProject(workspaceSlug: string, projectId: string, data: Partial<TProject>): Promise<TProject>
Line range hint
1-165: Summary: Successful transition fromIProjecttoTProjectThe changes in this file consistently update the
ProjectServiceclass to use theTProjecttype instead ofIProject. This transition improves type consistency across the project service and aligns with the broader changes in the PR.Key points:
- Import statements have been updated correctly.
- Method signatures for
createProject,getProjects,getProject, andupdateProjecthave been updated to useTProject.- The use of
Partial<TProject>in input parameters allows for flexible project creation and updates.These changes enhance type safety and maintain the existing functionality of the
ProjectServiceclass. The transition appears to be complete and correct for this file.Consider adding JSDoc comments to the updated methods for improved documentation and developer experience.
web/helpers/project.helper.ts (1)
Line range hint
1-116: Overall assessment: Successful type transition with improved consistency.The changes in this file successfully transition from
IProjecttoTProjecttype, improving type consistency across the project. The logic of the functions remains unchanged, which is appropriate for this type of refactoring. The addition ofEUserPermissionsenum usage also enhances code readability and maintainability.To ensure a smooth transition:
- Verify that all occurrences of
IProjecthave been replaced withTProjectthroughout the codebase.- Update any documentation or comments that may still reference
IProject.- Run comprehensive tests to catch any type-related issues that may have been introduced.
Consider creating a migration guide or updating the project's documentation to reflect this type change, especially if
IProjectwas part of a public API.web/core/components/workspace/sidebar/projects-list.tsx (3)
21-24: LGTM! Consider grouping related imports.The addition of
EUserPermissions,EUserPermissionsLevel, andTProjectimports aligns with the PR objectives. These changes support the transition fromIProjecttoTProjectand enhance permission checks.Consider grouping related imports together for better readability. For example:
import { EUserPermissions, EUserPermissionsLevel } from "@/plane-web/constants/user-permissions"; import { TProject } from "@/plane-web/types";
Line range hint
39-43: LGTM! Consider adding a comment for clarity.The addition of
isAuthorizedUserenhances permission control, aligning with the PR objectives. This constant is used to conditionally render UI elements, improving security and user experience.Consider adding a brief comment explaining the purpose of
isAuthorizedUser:// Check if the user has admin or member permissions at the workspace level const isAuthorizedUser = allowPermissions( [EUserPermissions.ADMIN, EUserPermissions.MEMBER], EUserPermissionsLevel.WORKSPACE );
Line range hint
71-77: LGTM! Consider using type assertion for improved type safety.The transition from
IProject[]toTProject[]forjoinedProjectsListis correctly implemented, aligning with the PR objectives.For improved type safety, consider using a type assertion when pushing project details:
const joinedProjectsList: TProject[] = []; joinedProjects.map((projectId) => { const projectDetails = getProjectById(projectId); if (projectDetails) joinedProjectsList.push(projectDetails as TProject); });This ensures that
projectDetailsis explicitly treated asTProjectwhen added to the list.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- web/core/components/dropdowns/project.tsx (2 hunks)
- web/core/components/workspace/sidebar/projects-list.tsx (2 hunks)
- web/core/services/project/project.service.ts (2 hunks)
- web/core/store/project/project.store.ts (1 hunks)
- web/helpers/project.helper.ts (4 hunks)
✅ Files skipped from review due to trivial changes (1)
- web/core/store/project/project.store.ts
🧰 Additional context used
🔇 Additional comments (13)
web/core/services/project/project.service.ts (3)
8-9: LGTM: Import statement updated correctly.The import statement has been updated to use
TProjectfrom@/plane-web/types, which is consistent with the transition fromIProjecttoTProjectthroughout the file. This change supports better type consistency and maintainability.
38-38: LGTM: Return type updated correctly.The
getProjectsmethod signature has been updated to returnPromise<TProject[]>, which is consistent with the transition fromIProjecttoTProject. This change maintains type safety for the array of projects returned by the method.
46-46: LGTM: Return type updated correctly.The
getProjectmethod signature has been updated to returnPromise<TProject>, which is consistent with the transition fromIProjecttoTProject. This change maintains type safety for the single project returned by the method.web/helpers/project.helper.ts (5)
3-3: LGTM: Import statements updated to reflect type changes.The import statements have been correctly updated to accommodate the transition from
IProjecttoTProject. The addition ofEUserPermissionsimport suggests new functionality related to user permissions, which aligns with the PR objectives.Also applies to: 7-7, 10-10
23-23: LGTM: Parameter type updated for consistency.The parameter type for
joinedProjectshas been correctly updated fromIProject[]toTProject[], maintaining consistency with the new type system.
54-54: LGTM: Function updated with improved type safety and readability.The
shouldRenderProjectfunction has been successfully updated to useTProjecttype. Additionally, the use ofEUserPermissions.MEMBERenum instead of a hardcoded value improves code readability and maintainability.Also applies to: 57-57, 58-58
62-62: LGTM: Function signature updated for type consistency.The
shouldFilterProjectfunction has been correctly updated to use theTProjecttype, maintaining consistency with the new type system while preserving the existing logic.Also applies to: 68-68
99-99: LGTM: Function updated for type consistency. Verify usage across the codebase.The
orderProjectsfunction has been correctly updated to useTProject[]for parameter and return types, maintaining consistency with the new type system.To ensure that this change doesn't introduce any issues elsewhere in the codebase, please run the following verification script:
Also applies to: 101-101, 103-103, 104-104
web/core/components/dropdowns/project.tsx (4)
15-16: LGTM: Import statement for TProject added correctly.The import statement for
TProjectfrom@/plane-web/typeshas been added correctly. This change aligns with the transition fromIProjecttoTProjecttype.
Line range hint
1-238: Summary: TProject integration looks good, final verification recommended.The changes to integrate
TProjectinstead ofIProjectin theProjectDropdowncomponent look good. The import statement and prop type have been updated correctly. To ensure a smooth transition, please:
- Verify that all usages of
renderConditionwithin the component are compatible withTProject.- Confirm that
getProjectByIdand all usages ofprojectDetailsare compatible withTProject.- Run a final type-check and build process to catch any potential type-related issues.
These steps will help ensure that the refactoring from
IProjecttoTProjectis complete and consistent throughout the component and its dependencies.
30-30: LGTM: Prop type updated to use TProject.The
renderConditionprop type has been correctly updated to useTProjectinstead ofIProject. This change is consistent with the new import statement.To ensure consistency, please verify that all usages of
renderConditionwithin the component are compatible with theTProjecttype. Run the following script to check for any remaining references toIProject:
Line range hint
1-238: Verify compatibility of getProjectById and projectDetails with TProject.While the prop type has been updated to use
TProject, it's important to ensure thatgetProjectByIdfrom theuseProjecthook and all usages ofprojectDetailsthroughout the component are compatible with theTProjecttype.Please run the following script to check the type definition of
getProjectByIdand verify its compatibility withTProject:If any incompatibilities are found, please update the relevant parts of the component to ensure full compatibility with
TProject.web/core/components/workspace/sidebar/projects-list.tsx (1)
Line range hint
1-324: Overall, the changes look good and align with the PR objectives.The transition from
IProjecttoTProject, addition of user permission checks, and related import changes have been implemented correctly. The suggestions provided are minor and aimed at improving code clarity and type safety.Great job on maintaining consistency throughout the file while implementing these changes!
Summary by CodeRabbit
New Features
Bug Fixes
Documentation