-
Notifications
You must be signed in to change notification settings - Fork 59
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.
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
[Performance optimisation] Unnecessary API calls #3689
[Performance optimisation] Unnecessary API calls #3689
Conversation
fix deepscan errors add greptile-apps suggestions add greptile-apps suggestions
…s types, task sizes, ...
WalkthroughThis update refactors multiple modules across the application. Key changes include the removal of redundant role-fetching calls from the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant KanbanComponent
participant TaskStatusHook
participant API
User->>KanbanComponent: Click "Toggle Column"
KanbanComponent->>TaskStatusHook: Call toggleColumn(column, status)
TaskStatusHook->>API: Send async editTaskStatus request
API-->>TaskStatusHook: Return success/failure response
TaskStatusHook->>KanbanComponent: Return updated taskStatuses
KanbanComponent->>User: Update UI with new status
sequenceDiagram
participant Component
participant DailyPlanHook
participant API
Component->>DailyPlanHook: Call handleFirstLoad()
DailyPlanHook->>API: Call loadAllDayPlans (async)
API-->>DailyPlanHook: Return day plans data
DailyPlanHook->>Component: Update state with daily plans
Poem
Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
apps/web/lib/features/task/task-filters.tsxOops! Something went wrong! :( ESLint: 8.46.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. apps/web/lib/settings/task-statuses-form.tsxOops! Something went wrong! :( ESLint: 8.46.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. apps/web/lib/settings/task-sizes-form.tsxOops! Something went wrong! :( ESLint: 8.46.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR significantly optimizes API calls and improves performance across the ever-teams application. Here's a focused summary of the key changes:
- Reduced polling intervals from 5s to 1min for organization teams and 1min to 5min for other data, significantly decreasing server load
- Renamed task status variables consistently across files (taskStatus → taskStatuses) to match updated hook implementations
- Added deep equality checks using lodash's isEqual before state updates to prevent unnecessary re-renders
- Removed redundant getRoles() calls from multiple components since roles are now fetched through the useRoles hook
- Implemented optimistic UI updates after mutations to reduce refetch needs
The changes appear well-structured but should be carefully tested to ensure the increased polling intervals don't negatively impact real-time collaboration features.
💡 (3/5) Reply to the bot's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!
49 file(s) reviewed, 32 comment(s)
Edit PR Review Bot Settings | Greptile
There was a problem hiding this 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 comments (1)
apps/web/app/hooks/features/useKanban.ts (1)
28-64
: 🛠️ Refactor suggestionConsider adding error handling in the loading sequence.
While thisuseEffect
correctly checksgetTaskStatusesLoading
andtasksFetching
before proceeding, there's no fallback logic for error states. If either request fails, you may end up with partial or empty data. It might improve user experience to handle an error state (e.g., show a warning or retry mechanism).
🧹 Nitpick comments (13)
apps/web/app/[locale]/projects/components/project-views/index.tsx (2)
1-7
: Consistent import statement formattingChanged double quotes to single quotes in import statements for better consistency with the codebase style.
21-21
: Minor formatting improvementRemoved extra space before the colon in the type definition, improving code style consistency.
apps/web/lib/settings/table-action-popover.tsx (1)
64-64
: Use optional chaining for safer accessThe static analysis tool correctly suggests using optional chaining here to avoid potential runtime errors if
handleEdit
is undefined.- onClick={() => { - handleEdit && handleEdit(member); - }} + onClick={() => { + handleEdit?.(member); + }}🧰 Tools
🪛 Biome (1.9.4)
[error] 64-64: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/web/app/hooks/features/useTaskPriorities.ts (2)
47-64
: Consider switching to async/await for consistency.
While this uses a.then
callback, you might switch completely to async/await for easier flow control and more robust error handling.
100-100
: Refetching after editing.
Re-fetching the task priorities right after editing is useful for ensuring the UI reflects the updated data. If performance becomes a concern with rapid edits, consider batching requests or a single refresh step.apps/web/app/hooks/features/useRoles.ts (1)
27-38
: Creating roles with proper error handling.
This async approach withconsole.error
is straightforward. Consider eventually adding user-facing error notifications if needed.apps/web/lib/features/permission/permission-dropdown.tsx (1)
31-42
: Creating a new role and updating local state.
After callingcreateRole
, updating your localroles
array ensures immediate visual feedback. For concurrency safety, consider using a functional update:setRoles((prev) => [...prev, res.data])
.apps/web/lib/app/init-state.tsx (1)
148-148
: Use optional chaining.
You can replacefuncRef.current && funcRef.current()
with the simpler and saferfuncRef.current?.()
:- funcRef.current && funcRef.current(); + funcRef.current?.();🧰 Tools
🪛 Biome (1.9.4)
[error] 148-148: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/web/app/hooks/features/useKanban.ts (4)
74-85
: Re-evaluate dependencies to avoid potential excessive re-renders.
The dependency array is quite large, containing multiple states. Each minor change re-runs this effect and re-calculates the entire Kanban data. Consider memoizing some filters or consolidating states to mitigate performance overhead.
90-111
: Add a fallback for unsuccessful updates intoggleColumn
.
Currently, your code checksif (updatedStatus) { ... }
but does not handle the case whentaskStatusHook.editTaskStatus
fails or returns an unexpected response. You may want to add a fallback or error message to let users know the edit failed.
127-149
: Consider batching multiple reorder operations.
If there’s a scenario where a user moves multiple columns in quick succession, you may send multiple reorder requests. Batching them could reduce API calls and improve performance under heavy interactions.
160-160
: Rename returned property for clarity.
columns: taskStatusHook.taskStatuses
might introduce confusion. Consider renaming this property totaskStatuses
orstatusColumns
to reflect the data more accurately.apps/web/app/hooks/features/useTaskStatus.ts (1)
55-129
: Robust error handling in creation, deletion, and editing of statuses.
Each method includes clear try/catch blocks and logs errors. Consider unifying user-facing error messages for a more consistent UI/UX.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (49)
apps/web/app/[locale]/permissions/component.tsx
(3 hunks)apps/web/app/[locale]/projects/components/filters-card-modal.tsx
(3 hunks)apps/web/app/[locale]/projects/components/project-views/grid-view/grid-item.tsx
(1 hunks)apps/web/app/[locale]/projects/components/project-views/index.tsx
(2 hunks)apps/web/app/[locale]/projects/components/project-views/list-view/data-table.tsx
(1 hunks)apps/web/app/[locale]/team/[teamId]/[profileLink]/page.tsx
(1 hunks)apps/web/app/hooks/features/useDailyPlan.ts
(6 hunks)apps/web/app/hooks/features/useKanban.ts
(4 hunks)apps/web/app/hooks/features/useOrganizationProjects.ts
(7 hunks)apps/web/app/hooks/features/useOrganizationTeams.ts
(6 hunks)apps/web/app/hooks/features/usePublicOrganizationTeams.ts
(3 hunks)apps/web/app/hooks/features/useRefetchData.ts
(1 hunks)apps/web/app/hooks/features/useRoles.ts
(1 hunks)apps/web/app/hooks/features/useTaskInput.ts
(1 hunks)apps/web/app/hooks/features/useTaskLabels.ts
(8 hunks)apps/web/app/hooks/features/useTaskPriorities.ts
(7 hunks)apps/web/app/hooks/features/useTaskRelatedIssueType.ts
(7 hunks)apps/web/app/hooks/features/useTaskSizes.ts
(1 hunks)apps/web/app/hooks/features/useTaskStatus.ts
(1 hunks)apps/web/app/hooks/features/useTeamTasks.ts
(2 hunks)apps/web/app/hooks/features/useTimer.ts
(3 hunks)apps/web/app/interfaces/ITask.ts
(1 hunks)apps/web/app/services/client/api/task-sizes.ts
(1 hunks)apps/web/app/services/client/api/task-status.ts
(3 hunks)apps/web/app/stores/task-status.ts
(1 hunks)apps/web/components/pages/kanban/edit-status-modal.tsx
(3 hunks)apps/web/components/pages/kanban/menu-kanban-card.tsx
(2 hunks)apps/web/components/pages/kanban/sort-tasks-status-settings.tsx
(1 hunks)apps/web/components/pages/main/team-member.tsx
(1 hunks)apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx
(2 hunks)apps/web/components/shared/teams/teams-dropdown.tsx
(2 hunks)apps/web/lib/app/init-state.tsx
(1 hunks)apps/web/lib/features/daily-plan/add-task-estimation-hours-modal.tsx
(3 hunks)apps/web/lib/features/permission/permission-dropdown.tsx
(5 hunks)apps/web/lib/features/project/add-or-edit-project/index.tsx
(2 hunks)apps/web/lib/features/project/add-or-edit-project/steps/review-summary.tsx
(2 hunks)apps/web/lib/features/project/add-or-edit-project/steps/team-and-relations-form.tsx
(2 hunks)apps/web/lib/features/project/archive-project-modal.tsx
(1 hunks)apps/web/lib/features/task-status/delete-status-confirmation-modal.tsx
(2 hunks)apps/web/lib/features/task/task-all-status-type.tsx
(4 hunks)apps/web/lib/features/task/task-input-kanban.tsx
(1 hunks)apps/web/lib/features/task/task-status.tsx
(3 hunks)apps/web/lib/features/team-members-kanban-view.tsx
(2 hunks)apps/web/lib/features/team-members.tsx
(1 hunks)apps/web/lib/features/team/team-outstanding-notifications.tsx
(2 hunks)apps/web/lib/settings/table-action-popover.tsx
(8 hunks)apps/web/lib/settings/task-sizes-form.tsx
(6 hunks)apps/web/lib/settings/task-statuses-form.tsx
(8 hunks)apps/web/lib/settings/team-setting-form.tsx
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
apps/web/lib/features/task-status/delete-status-confirmation-modal.tsx (1)
Learnt from: CREDO23
PR: ever-co/ever-teams#3309
File: apps/web/lib/features/task-status/delete-status-confirmation-modal.tsx:31-32
Timestamp: 2025-03-21T13:46:21.274Z
Learning: In the task item, the `status` field holds the name of the status, not the status ID.
apps/web/lib/settings/task-statuses-form.tsx (1)
Learnt from: CREDO23
PR: ever-co/ever-teams#3309
File: apps/web/lib/features/task-status/delete-status-confirmation-modal.tsx:31-32
Timestamp: 2025-03-21T13:46:21.274Z
Learning: In the task item, the `status` field holds the name of the status, not the status ID.
🧬 Code Definitions (25)
apps/web/lib/features/team-members-kanban-view.tsx (1)
apps/web/app/hooks/features/useTaskStatus.ts (1)
useTaskStatus
(18-164)
apps/web/app/hooks/features/useRefetchData.ts (1)
apps/web/app/hooks/features/useTaskStatus.ts (1)
useTaskStatus
(18-164)
apps/web/lib/features/project/add-or-edit-project/steps/team-and-relations-form.tsx (1)
apps/web/app/hooks/features/useRoles.ts (1)
useRoles
(9-100)
apps/web/components/pages/kanban/edit-status-modal.tsx (1)
apps/web/app/hooks/features/useTaskStatus.ts (1)
useTaskStatus
(18-164)
apps/web/components/pages/kanban/menu-kanban-card.tsx (1)
apps/web/app/hooks/features/useTaskStatus.ts (1)
useTaskStatus
(18-164)
apps/web/app/hooks/features/useTaskInput.ts (1)
apps/web/app/hooks/features/useTaskStatus.ts (1)
useTaskStatus
(18-164)
apps/web/lib/features/project/add-or-edit-project/steps/review-summary.tsx (1)
apps/web/app/hooks/features/useRoles.ts (1)
useRoles
(9-100)
apps/web/lib/features/team/team-outstanding-notifications.tsx (1)
apps/web/app/hooks/features/useDailyPlan.ts (1)
useDailyPlan
(33-503)
apps/web/app/hooks/features/usePublicOrganizationTeams.ts (3)
apps/web/app/hooks/features/useOrganizationTeams.ts (1)
useOrganizationTeams
(282-538)apps/web/app/hooks/features/useTeamTasks.ts (1)
useTeamTasks
(79-548)apps/web/app/hooks/features/useTaskStatus.ts (1)
useTaskStatus
(18-164)
apps/web/lib/features/task-status/delete-status-confirmation-modal.tsx (1)
apps/web/app/hooks/features/useTaskStatus.ts (1)
useTaskStatus
(18-164)
apps/web/app/[locale]/projects/components/project-views/grid-view/grid-item.tsx (1)
apps/web/app/hooks/features/useTaskStatus.ts (1)
useTaskStatus
(18-164)
apps/web/lib/features/task/task-status.tsx (1)
apps/web/app/hooks/features/useTaskStatus.ts (1)
useTaskStatus
(18-164)
apps/web/app/[locale]/projects/components/filters-card-modal.tsx (1)
apps/web/app/hooks/features/useTaskStatus.ts (1)
useTaskStatus
(18-164)
apps/web/app/[locale]/projects/components/project-views/list-view/data-table.tsx (1)
apps/web/app/hooks/features/useTaskStatus.ts (1)
useTaskStatus
(18-164)
apps/web/lib/settings/table-action-popover.tsx (1)
apps/web/app/hooks/features/useRoles.ts (1)
useRoles
(9-100)
apps/web/app/services/client/api/task-status.ts (1)
apps/web/app/interfaces/ITask.ts (1)
ITaskStatusOrder
(208-211)
apps/web/lib/features/daily-plan/add-task-estimation-hours-modal.tsx (1)
apps/web/app/hooks/features/useTaskStatus.ts (1)
useTaskStatus
(18-164)
apps/web/lib/features/permission/permission-dropdown.tsx (1)
apps/web/app/hooks/features/useRoles.ts (1)
useRoles
(9-100)
apps/web/app/hooks/features/useTaskStatus.ts (3)
apps/web/app/stores/task-status.ts (1)
taskStatusesState
(4-4)apps/web/app/services/client/api/task-status.ts (5)
getTaskStatusesAPI
(33-43)createTaskStatusAPI
(11-15)deleteTaskStatusAPI
(29-31)editTaskStatusAPI
(17-21)editTaskStatusOrderAPI
(23-28)apps/web/app/interfaces/ITask.ts (1)
ITaskStatusOrder
(208-211)
apps/web/app/hooks/features/useTaskSizes.ts (1)
apps/web/app/services/client/api/task-sizes.ts (4)
getTaskSizes
(25-32)createTaskSizeAPI
(5-11)deleteTaskSizeAPI
(21-23)editTaskSizeAPI
(13-19)
apps/web/app/hooks/features/useTeamTasks.ts (1)
apps/web/app/hooks/features/useTaskStatus.ts (1)
useTaskStatus
(18-164)
apps/web/app/[locale]/permissions/component.tsx (1)
apps/web/app/hooks/features/useRoles.ts (1)
useRoles
(9-100)
apps/web/lib/app/init-state.tsx (7)
apps/web/app/hooks/features/useOrganizationProjects.ts (1)
useOrganizationProjects
(18-159)apps/web/app/hooks/features/useRoles.ts (1)
useRoles
(9-100)apps/web/app/hooks/features/useTaskStatus.ts (1)
useTaskStatus
(18-164)apps/web/app/hooks/features/useTaskSizes.ts (1)
useTaskSizes
(12-102)apps/web/app/hooks/features/useTaskLabels.ts (1)
useTaskLabels
(22-147)apps/web/app/hooks/features/useTaskRelatedIssueType.ts (1)
useTaskRelatedIssueType
(23-139)apps/web/app/hooks/features/useDailyPlan.ts (1)
useDailyPlan
(33-503)
apps/web/app/hooks/features/useOrganizationTeams.ts (1)
apps/web/app/hooks/useQuery.ts (1)
useQuery
(15-41)
apps/web/app/hooks/features/useDailyPlan.ts (2)
apps/web/app/services/client/api/daily-plan.ts (2)
getDayPlansByEmployeeAPI
(57-76)getAllDayPlansAPI
(15-34)apps/web/app/services/server/requests/daily-plan.ts (1)
getAllDayPlans
(12-42)
🪛 Biome (1.9.4)
apps/web/lib/settings/table-action-popover.tsx
[error] 64-64: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/web/lib/app/init-state.tsx
[error] 148-148: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: deploy
🔇 Additional comments (169)
apps/web/app/stores/task-status.ts (2)
4-4
: Consistent renaming of task status state atomThe renaming from
taskStatusListState
totaskStatusesState
improves naming consistency by using the plural form to better represent a collection of task statuses. This aligns with the broader renaming convention changes mentioned in the summary.
11-13
: Updated references to match the renamed state atomThe variable name has been appropriately updated from
taskStatus
totaskStatuses
to reflect the state atom rename, while maintaining the same functionality. The logic for retrieving the active task status remains unchanged and works correctly.apps/web/lib/features/team-members-kanban-view.tsx (2)
40-40
: Good renaming for consistency with the updated hook interfaceThe change from destructuring
taskStatus
asts
to simply destructuringtaskStatuses
directly is a good improvement that aligns with the updateduseTaskStatus
hook interface. This renaming provides better clarity and is part of the PR's effort to optimize API calls and improve code consistency.
85-85
: Correctly updated variable referenceThe reference to the renamed variable has been properly updated here. The functionality remains the same - finding a task status by name to get its ID - but now uses the more appropriately named
taskStatuses
variable.apps/web/lib/features/daily-plan/add-task-estimation-hours-modal.tsx (4)
555-555
: LGTM! Variable renaming for consistency.The variable is now properly renamed from
taskStatus
totaskStatuses
to be consistent with what's returned from theuseTaskStatus
hook.
595-597
: LGTM! Updated status and taskStatusId references.The task creation parameters correctly use the plural form
taskStatuses
array to access the first item's name and id, maintaining consistency with the variable renaming.
604-604
: LGTM! Updated useCallback dependency array.The dependency array has been properly updated to include the renamed
taskStatuses
variable, ensuring the callback function refreshes when the task statuses change.
802-804
: LGTM! Updated background color filter reference.The filter operation now correctly uses
status.taskStatuses
instead oftaskStatus
, maintaining consistency with the variable renaming throughout the application.apps/web/app/services/client/api/task-status.ts (3)
4-6
: Import improvement: Added explicit ITaskStatusOrder import.The addition of
ITaskStatusOrder
to the imports improves code organization by making dependencies explicit rather than relying on locally defined types. This change aligns with good practices for maintainability and consistency.
23-24
: Type refinement: Using indexed access type for better type safety.Using
ITaskStatusOrder["reorder"]
as the return type provides better type safety by ensuring the patch function's return type matches exactly what's expected from the reorder operation. This indexed access type approach prevents potential type mismatches and makes the code more maintainable.
33-33
: Naming convention improvement: Consistent API function naming.Renaming
getTaskStatusList
togetTaskStatusesAPI
achieves two important improvements:
- The plural form (
Statuses
) correctly indicates that the function returns multiple status items- The
API
suffix maintains consistency with other API functions in this fileThis change aligns with the PR's objective of standardizing naming conventions throughout the codebase.
apps/web/lib/features/task-status/delete-status-confirmation-modal.tsx (3)
27-27
: Good addition of setTaskStatuses to support local state managementAdding setTaskStatuses from the useTaskStatus hook allows for local state updates without requiring additional API calls, which aligns with the PR's optimization goals.
54-58
: Effective optimization through local state managementThis is an excellent performance improvement. Instead of refetching all task statuses after a deletion, you're updating the local state directly by filtering out the deleted status. This approach reduces unnecessary API calls, which is exactly in line with the PR objectives.
64-64
: Proper dependency array updateGood job updating the dependency array to include setTaskStatuses and status.id since they're both used within the callback function. This ensures the callback is correctly regenerated when these dependencies change.
apps/web/lib/features/team-members.tsx (1)
26-26
: Renamed variable improves consistency with hook implementationThe destructuring assignment now correctly maps
getOrganizationTeamsLoading
from the hook to the local variableteamsFetching
, which aligns with the PR's objective of improving performance by ensuring consistent handling of loading states.apps/web/lib/features/task/task-all-status-type.tsx (4)
3-7
: LGTM: Streamlined importsThe imports have been organized more efficiently, supporting the PR's goal of optimizing the codebase.
19-20
: Added Jotai state management for projectsThe addition of Jotai's
useAtomValue
hook andorganizationProjectsState
replaces direct API calls with centralized state management, which aligns perfectly with the PR's goal of reducing unnecessary API calls.
41-45
: Optimized project data retrievalThe implementation now uses Jotai's global state to access projects instead of making redundant API calls. The
useMemo
ensures that project filtering only happens when necessary, which significantly contributes to the 85% reduction in API calls mentioned in the PR objectives.
106-109
: Updated reference to use memoized project dataThe component now correctly references the memoized
taskProject
variable instead of the previously fetched project data, completing the optimization cycle for project data access.apps/web/app/interfaces/ITask.ts (1)
208-211
: Added type for task status reorderingThe new
ITaskStatusOrder
type provides proper typing for status reordering operations, which supports the PR's optimization goals by ensuring type safety when manipulating task status orders without requiring additional API calls for validation.apps/web/app/[locale]/team/[teamId]/[profileLink]/page.tsx (1)
25-25
: Renamed variable maintains consistent terminologyRenaming the extracted property from
loading
toteamsFetching
ensures consistent variable naming throughout the codebase while preserving the functionality, supporting the PR's overall optimization efforts.apps/web/components/shared/teams/teams-dropdown.tsx (2)
12-12
: Variable name updated to better reflect its purpose.The variable name has been changed from
teamsFetching
togetOrganizationTeamsLoading
, which more clearly indicates its role in tracking the loading state of the organization teams fetch operation.
34-34
: Variable reference updated to match the renamed property.The loading state check has been correctly updated to use
getOrganizationTeamsLoading
instead ofteamsFetching
, ensuring consistency with the destructured property name in line 12.apps/web/lib/features/project/add-or-edit-project/steps/review-summary.tsx (2)
2-2
: Removed unused import after eliminating useEffect.The
useEffect
import has been correctly removed since it's no longer needed after removing the effect hook that was fetching roles.
31-31
: Reduced unnecessary API calls by removing getRoles.Removing
getRoles
from the destructured properties ofuseRoles()
hook aligns with the PR objective of reducing unnecessary API calls. The component now only consumes theroles
data without triggering a new fetch.This change supports the overall PR goal of reducing API calls from ~200 to ~30 on initial load, as stated in the PR objectives.
apps/web/lib/features/project/archive-project-modal.tsx (1)
110-112
: Enhanced translation context with project name.Added the project name to the translation parameters, which improves the user experience by including the project name in the archive confirmation message.
This change provides more context to users about which project they're archiving, making the UI more informative and user-friendly.
apps/web/lib/features/task/task-input-kanban.tsx (1)
430-430
: Updated function call name to match the refactored hook method.The method call has been updated from
loadTaskStatusData()
toloadTaskStatuses()
to align with the renamed method in theuseTaskStatus
hook, maintaining consistency across the codebase.This is part of the broader refactoring effort to standardize naming conventions from singular to plural for collections (e.g., taskStatus → taskStatuses), which improves code readability and maintainability.
apps/web/lib/settings/team-setting-form.tsx (1)
27-27
: Variable renaming improves clarityThe loading variable has been renamed from
loading
togetOrganizationTeamsLoading : loading
, providing more specificity about which loading state is being tracked while maintaining backward compatibility.apps/web/lib/features/team/team-outstanding-notifications.tsx (2)
2-2
: Reduced hook dependenciesRemoved
useOrganizationTeams
from imports, which aligns with the PR's goal of reducing unnecessary API calls.
19-19
: Eliminated redundant API callThe component now directly uses the
dailyPlan
andoutstandingPlans
fromuseDailyPlan
without relying onactiveTeam
fromuseOrganizationTeams
. This removes a redundant API call and simplifies the component's dependencies.This change contributes to the PR's goal of reducing API calls by approximately 85% on initial load.
apps/web/components/pages/kanban/menu-kanban-card.tsx (2)
19-19
: Updated variable name for clarityChanged
taskStatus
totaskStatuses
to reflect that it represents a collection of statuses rather than a single status, improving code readability.
84-84
: Consistent reference to renamed variableUpdated the reference from
taskStatus[0].id
totaskStatuses[0].id
to match the variable renaming, ensuring consistent usage throughout the component.apps/web/components/pages/kanban/edit-status-modal.tsx (3)
17-17
: Good refactoring to use specific named variables from the hook.The change from using generic loading state to specific named variables (
editTaskStatusLoading
,setTaskStatuses
) improves code readability and makes it clear which specific operations are being performed.
45-59
: Great optimization of state management!This implementation now updates the local state directly with the response data instead of triggering a full refetch of task statuses. The local state update through
setTaskStatuses
is an efficient way to maintain data consistency without unnecessary API calls.This change aligns perfectly with the PR objective to reduce redundant API requests.
108-109
: Appropriate loading state management.Using the specific
editTaskStatusLoading
state for the button's disabled and loading properties correctly reflects the current operation status.apps/web/components/pages/main/team-member.tsx (2)
11-11
: Appropriate renaming for consistent API naming conventions.The change from
teamsFetching
togetOrganizationTeamsLoading
follows the standardized naming convention being applied across the codebase, making the code more maintainable and easier to understand.
16-16
: Consistent variable usage.Updating the usage of the renamed loading state variable maintains the same logical condition while using the new standardized variable name.
apps/web/app/hooks/features/useTimer.ts (3)
195-195
: Consistent naming convention update.Changing from
taskStatus
totaskStatuses
reflects the plural nature of the data being handled, following the standardized naming convention being applied across the codebase.
375-377
: Properly updated variable reference in code logic.The task status checking logic has been correctly updated to use the renamed
taskStatuses
variable, maintaining the same functionality while using the new naming convention.
415-415
: Updated dependency array with renamed variable.Properly updating the dependency array in the useCallback hook to reference the renamed variable is important to maintain correct dependency tracking and prevent potential bugs.
apps/web/app/hooks/features/useTaskInput.ts (1)
46-46
: Consistent naming convention with aliasing.The update correctly renames the imported hook variable from
taskStatus
totaskStatuses
while maintaining the same local variable nametaskStatusList
through aliasing. This preserves the existing functionality while aligning with the new naming convention.apps/web/app/hooks/features/useRefetchData.ts (1)
9-9
: Function name update improves consistency with useTaskStatus hook implementationThe renaming from
loadTaskStatusData
toloadTaskStatuses
maintains consistency with the actual function name in theuseTaskStatus
hook. This is part of the broader effort to standardize naming conventions across the codebase.Also applies to: 16-16, 21-21
apps/web/lib/features/project/add-or-edit-project/index.tsx (1)
39-39
: Roles API call reductionRemoving
getRoles
from the destructuring aligns with the PR's goal of reducing unnecessary API calls. The component now relies on the pre-populated roles state rather than fetching it again.apps/web/lib/settings/table-action-popover.tsx (2)
6-6
: Import cleanup and type definition updateRemoving the unused
useEffect
import and adding a semicolon to the type definition improves code cleanliness.Also applies to: 15-15
180-180
: Roles API call reduction in RolePopoverRemoving the
getRoles
function from the destructuring prevents an unnecessary API call when this component renders. This is consistent with the PR's performance optimization goals.apps/web/app/[locale]/permissions/component.tsx (1)
32-32
: Roles API call reduction in Permissions componentRemoving
getRoles
from the destructuring eliminates an unnecessary API call, consistent with the performance optimization goals of this PR. The component now relies on the pre-populated roles state.apps/web/lib/features/project/add-or-edit-project/steps/team-and-relations-form.tsx (2)
3-3
: Import optimized to reflect actual usageThe import statement has been optimized to remove the no-longer-used
useEffect
hook, keeping only what's actually needed by the component.
22-22
: Reduced unnecessary API calls by removing automatic roles fetchingThis change removes the unnecessary extraction of the
getRoles
function, which prevents automatic API calls on component mount. The component now only uses the roles state from the hook, which aligns with the PR objective of reducing unnecessary API calls.Looking at the
useRoles
hook, theroles
state is pre-populated from the global state, eliminating the need for explicit fetching when the component mounts.apps/web/app/hooks/features/usePublicOrganizationTeams.ts (5)
22-22
: Variable renamed for better clarity and consistencyThe destructured property has been renamed from
teamsFetching
togetOrganizationTeamsLoading
, making it more descriptive and aligned with the actual function name that performs the loading, which improves code readability.
24-24
: Standardized naming convention for task statusesThe variable has been updated from singular
setTaskStatus
to pluralsetTaskStatuses
to better represent that it manages a collection of statuses, which maintains consistency with the rest of the codebase.
102-102
: Updated function call to use standardized namingFunction call updated to use
setTaskStatuses
instead ofsetTaskStatus
, maintaining consistency with the renamed variable.
111-111
: Updated dependency array for useCallback hookThe dependency array in the
useCallback
hook has been updated to reference the renamed functionsetTaskStatuses
, ensuring that the callback is properly updated when this dependency changes.
115-115
: Updated returned property name for consistencyThe returned property has been renamed from
teamsFetching
togetOrganizationTeamsLoading
to match the earlier variable name change, maintaining consistency throughout the hook's interface.apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx (2)
1-1
: Optimized imports by removing unnecessary hooksThe imports have been streamlined to remove unused hooks like
useOrganizationProjects
anduseOrganizationTeams
, which aligns with the PR objective of reducing unnecessary API calls triggered by these hooks.The component now relies on the Jotai atom state via
useAtomValue(organizationProjectsState)
on line 287, eliminating redundant API fetches.
409-409
: Simplified mapping function by removing unused parameterThe unnecessary index parameter has been removed from the map callback function, which is a minor improvement for code cleanliness.
apps/web/app/[locale]/projects/components/filters-card-modal.tsx (3)
48-48
: Standardized naming convention for task statusesChanged from singular
taskStatus
to pluraltaskStatuses
to better represent that it's a collection, which improves code clarity and maintains consistency with other parts of the codebase.
51-52
: Updated variable references in status colors mapThe
statusColorsMap
creation now uses the renamedtaskStatuses
variable, maintaining consistency with the variable name change.
203-203
: Consistently applied the taskStatuses naming throughout the componentAll references to the task status collection have been updated to use the plural form
taskStatuses
, ensuring consistency throughout the component and improving code readability.Also applies to: 211-212, 221-222, 228-228
apps/web/app/[locale]/projects/components/project-views/grid-view/grid-item.tsx (2)
27-27
: Consistent variable naming with useTaskStatus hook.The variable name change from
taskStatus
totaskStatuses
aligns with the actual return value from theuseTaskStatus
hook, improving code consistency.
30-31
: Updated useMemo dependencies to match renamed variable.The change correctly updates both the mapping operation and the dependency array to use the renamed
taskStatuses
variable, ensuring the memoized value is recalculated appropriately.apps/web/lib/features/task/task-status.tsx (3)
127-127
: Consistent variable naming with useTaskStatus hook.The variable name change from
taskStatus
totaskStatuses
aligns with the actual return value from theuseTaskStatus
hook, improving code consistency.
155-155
: Updated find operation to use renamed variable.The change correctly updates the logic to use
taskStatuses
for finding the selected status, maintaining functionality while improving naming consistency.
273-274
: Consistent function implementation with renamed variable.The
useTaskStatusValue
function now correctly usestaskStatuses
in the return statement, maintaining the proper data flow throughout the component.apps/web/app/[locale]/projects/components/project-views/list-view/data-table.tsx (2)
74-74
: Consistent variable naming with useTaskStatus hook.The variable name change from
taskStatus
totaskStatuses
aligns with the actual return value from theuseTaskStatus
hook, improving code consistency.
76-77
: Updated useMemo dependencies to match renamed variable.The change correctly updates both the mapping operation and the dependency array to use the renamed
taskStatuses
variable, ensuring the memoized value is recalculated appropriately.apps/web/app/hooks/features/useTeamTasks.ts (2)
94-94
: Consistent variable naming with useTaskStatus hook.The variable name change from
taskStatus
totaskStatuses
aligns with the actual return value from theuseTaskStatus
hook, improving code consistency.
290-327
: Improved function parameter structure and default value assignment.The
createTask
function has been refactored to use inline parameter destructuring with appropriate types, making the code more concise and readable. The default value for thestatus
parameter now correctly referencestaskStatuses[0]?.name
instead of the previous singular form.This refactoring maintains functionality while improving code consistency and readability, which aligns with the PR's goal of optimizing the codebase.
apps/web/lib/settings/task-sizes-form.tsx (6)
62-67
: Function names standardized to singular form.The renaming from plural (e.g.,
createTaskSizes
) to singular (e.g.,createTaskSize
) improves consistency in the API naming convention throughout the application. This matches the pattern seen in other hooks and components.
93-107
: LGTM! Function calls updated to match the new singular API naming.The function call has been updated to use the renamed
createTaskSize
function, maintaining consistency with the API changes.🧰 Tools
🪛 Biome (1.9.4)
[error] 104-104: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
115-123
: LGTM! Appropriate function name update.The edit operation now correctly calls
editTaskSize
instead ofeditTaskSizes
, consistent with the naming convention changes.
129-129
: Dependencies list updated to match renamed functions.The useCallback dependencies have been properly updated to reflect the renamed functions, ensuring the callback is correctly memoized based on the latest function references.
Also applies to: 133-133
211-212
: Loading state variables correctly updated.The loading state variables have been updated to use the new singular naming convention (
createTaskSizeLoading
andeditTaskSizeLoading
), maintaining consistency with the function name changes.
253-256
: Improved delete handler with async/await pattern.Converting the delete handler to an async function and awaiting the deletion before refetching data prevents potential race conditions where refetch might happen before the deletion is complete. This optimization aligns with the PR goal of reducing unnecessary API calls.
apps/web/lib/settings/task-statuses-form.tsx (6)
68-70
: State variable naming improved for clarity and consistency.The renaming from
loading
togetTaskStatusesLoading
andtaskStatus
totaskStatuses
improves clarity by being more descriptive and following the plural convention for collections. AddingsetTaskStatuses
to the destructured values gives direct access to update the state.Also applies to: 74-75
84-84
: Dependencies correctly updated for useEffect.The useEffect dependency has been properly updated to use
taskStatuses
instead oftaskStatus
, ensuring the effect runs correctly when the task statuses change.
108-109
: Enhanced asynchronous operation handling with await.Adding the
await
keyword to API calls provides better control over the asynchronous flow, ensuring that subsequent operations only execute after the API call completes. This helps prevent potential race conditions.Also applies to: 132-133
154-154
: Variable reference updated to match renamed state.The array variable correctly uses
taskStatuses.slice()
to match the renamed state variable.
328-328
: Conditional rendering updated to use the correct state variables.The conditional checks have been updated to use
taskStatuses.length
andgetTaskStatusesLoading
instead of their previous names, ensuring proper rendering based on the latest state.Also applies to: 334-335
359-362
: Improved state management after deletion.Adding direct state updates using
setTaskStatuses
after deletion operations ensures the UI stays in sync with the backend data without requiring an additional API call to refresh the data. This optimization aligns with the PR's goal of reducing unnecessary API calls.apps/web/app/hooks/features/useTaskRelatedIssueType.ts (7)
16-16
: Streamlined imports by removing unnecessary hooks.Removing unused hooks like
useEffect
simplifies the import list and reflects the optimized implementation that avoids unnecessary API calls on component mounting.
27-27
: Improved variable naming for better clarity.Renaming
loading
togetTaskRelatedIssueTypeLoading
andqueryCall
togetTaskRelatedIssueTypeQueryCall
makes the code more descriptive and self-documenting, clarifying the purpose of each variable.
52-64
: Enhanced asynchronous data loading.Converting
loadTaskRelatedIssueTypeData
to use the async/await pattern improves code readability and error handling. The function now returns a promise that can be awaited by callers, enabling better control of asynchronous operations.
86-93
: Updated API call references for consistency.The code now uses
getTaskRelatedIssueTypeQueryCall
instead ofqueryCall
in both thedeleteTaskRelatedIssueType
andeditTaskRelatedIssueType
functions, maintaining consistency with the renamed variables.Also applies to: 105-112
98-98
: Dependencies correctly updated for useCallback hooks.The dependencies for the useCallback hooks have been properly updated to include
getTaskRelatedIssueTypeQueryCall
, ensuring that the callbacks are correctly memoized based on the latest function references.Also applies to: 117-117
120-123
: Added dedicated function for initial data loading.The new
handleFirstLoad
function encapsulates the initialization logic, waiting for data to load before triggering first load completion. This optimization helps reduce unnecessary API calls by providing a single point of entry for initial data loading.
126-126
: Updated hook return values to match implementation changes.The hook now returns
loading
asgetTaskRelatedIssueTypeLoading
andfirstLoadTaskRelatedIssueTypeData
ashandleFirstLoad
, ensuring consumers of the hook use the correct functions and state variables. This maintains a consistent API while benefiting from the optimized implementation.Also applies to: 129-129
apps/web/app/hooks/features/useOrganizationProjects.ts (8)
10-10
: Streamlined imports by removing useEffect and adding useFirstLoad.Removing useEffect and adding useFirstLoad reflects the optimization strategy of replacing auto-triggered effects with explicitly called loading functions, reducing unnecessary API calls on component mounting.
Also applies to: 16-16
19-23
: Enhanced data retrieval with cookie-based IDs.Retrieving tenant and organization IDs from cookies instead of solely relying on the user state is more efficient and reduces dependencies on user data loading. This change supports the PR's goal of optimizing data fetching.
44-55
: Improved error handling in editOrganizationProjectSetting.Added comprehensive try/catch block with more descriptive error messages and validation of required parameters before making API calls. Using
tenantId
from cookies instead ofuser?.tenantId
reduces unnecessary dependencies on user state.
62-67
: Enhanced error handling across all API operations.Added consistent try/catch blocks with proper error logging using
console.error
instead ofconsole.log
for better error visibility and debugging. These improvements enhance the robustness and maintainability of the code.Also applies to: 77-77, 89-89, 104-104
107-107
: Dependencies correctly updated for createOrganizationProject.The dependencies for the createOrganizationProject callback now include
organizationId
andtenantId
, ensuring the callback is correctly memoized based on the latest variable values.
122-135
: Optimized organization projects loading logic.The rewritten
loadOrganizationProjects
function now includes important optimizations:
- Early return if user is not available
- Skip loading if projects are already loaded
- Comprehensive error handling
These changes significantly reduce unnecessary API calls, directly supporting the PR's performance optimization goals.
137-140
: Added dedicated function for initial data loading.The new
handleFirstLoad
function combines loading data and marking the first load as complete, providing a single entry point for initialization. This approach helps manage the loading sequence more effectively and reduces redundant API calls.
156-157
: Updated hook return values to match implementation changes.The hook now returns additional functions (
setOrganizationProjects
andfirstLoadOrganizationProjectsData: handleFirstLoad
) that align with the optimized implementation, ensuring consumers of the hook have access to the necessary functions for state management and initialization.apps/web/app/hooks/features/useTaskPriorities.ts (6)
26-26
: Naming looks consistent.
The destructuredloading
,queryCall
, andloadingRef
properties are well-named, improving the readability and clarity of the code.
43-43
: Clearer naming for firstLoadData.
RenamingfirstLoadData
tofirstLoadTaskPrioritiesData
helps clarify exactly what is being loaded.
85-85
: Refetching after deletion.
Refetching the task priorities list after deleting a priority ensures the state stays up-to-date. This is a good approach; just be mindful of potential duplicate calls if multiple deletions occur in quick succession.
119-122
: Combine load logic for the initial load.
CallingloadTaskPriorities
immediately followed byfirstLoadTaskPrioritiesData
centralizes your first-load process in one place, making it easier to reason about.
125-125
: Renamed loading variable improves readability.
UsinggetTaskPrioritiesLoading
clarifies the nature of this loading state as specific to getting task priorities.
127-127
: Straightforward mapping of firstLoadTaskPrioritiesData.
LinkingfirstLoadTaskPrioritiesData
tohandleFirstLoad
nicely unifies the naming approach for initial data fetching.apps/web/app/hooks/features/useRoles.ts (11)
2-2
: Imports for role APIs.
Importing all role-related APIs in one statement keeps the codebase organized.
7-7
: Introducing useFirstLoad.
Bringing in theuseFirstLoad
hook sets up a consistent pattern for handling the first load of roles.
10-10
: Atom state for roles.
Using[roles, setRoles]
fromrolesState
provides a clear, centralized store for role data.
11-11
: Renaming to firstRolesLoad.
RenamingfirstLoadData
tofirstRolesLoad
makes it easy to read and indicates the data domain (roles).
12-15
: Distinct loading states.
Having separate loading states and query calls (getRolesQueryCall
,createRoleQueryCall
, etc.) keeps each operation’s status independent and more manageable.
17-25
: Async/await for fetching roles.
Using try/catch here is a good choice to handle errors gracefully for thegetRoles
call.
40-50
: Updating roles.
Again, the pattern with async/await and catch blocks is good for clarity.
52-62
: Deleting roles.
Similar approach to creation and updates, ensuring you handle errors consistently.
64-77
: Load roles with getRoles.
CallinggetRoles
insideloadRoles
is well-structured. Throwing an error if roles fail to load helps spot issues early in dev.
79-83
: First roles load routine.
BundlingloadRoles()
withfirstRolesLoad()
ensures the initial fetch is triggered once and consistently.
84-99
: Exporting the updated hook.
Returning roles, loading states, and all CRUD operations in one object is convenient and keeps usage simple in consuming components.apps/web/lib/features/permission/permission-dropdown.tsx (4)
20-20
: Expanded roles handling in hook destructure.
Pullingroles, createRole, updateRole, deleteRole, setRoles
fromuseRoles
centralizes role management here.
61-75
: Editing a role with updated local state.
The approach is consistent with creation. UsingsetRoles((prev) => ...)
is good practice to avoid stale references.
102-114
: Deleting a role and refreshing local state.
WrappingdeleteRole
in a callback ensures the role is removed from the roles array once the request succeeds. This is coherent with the rest of the code.
191-191
: Conditional delete call.
Usingrole.id && handleDeleteRole(role.id)
is a safe check for an ID before deleting a role.apps/web/lib/app/init-state.tsx (4)
3-22
: Additional hooks imported for initialization.
Consolidating all app-wide hooks in one place improves code organization and ensures each feature's data is loaded consistently.
30-30
: Importing useRoles.
Bringing roles logic into the global init flow ensures roles are loaded and refreshed like other data.
64-90
: One-time load for critical data.
Invoking each feature’sfirstLoad...
function in one place ensures all critical data is fetched exactly once upon startup.
92-139
: AutoRefresher with extended intervals.
Increasing most intervals to a minute or five minutes significantly reduces redundant API calls and improves performance. Great optimization for large-scale apps.apps/web/app/hooks/features/useKanban.ts (1)
115-115
: No concerns with collapse behavior.
BothisColumnCollapse
andisAllColumnCollapse
are straightforward and improve readability.Also applies to: 122-122
apps/web/app/hooks/features/useTaskStatus.ts (4)
3-10
: Imports and new APIs look consistent.
The switch to singular API methods (e.g.,editTaskStatusAPI
) and typed interfaces clarifies the codebase and aligns with best practices.
16-30
: Active team and organization ID usage.
PullingactiveTeamIdState
and cookies for organization/tenant ensures the data is consistently fetched, but confirm that any mismatch between atom value vs. cookie value is handled gracefully (e.g., if the cookie becomes stale).
36-53
: Asynchronous approach to fetching statuses is well-structured.
The use ofuseQuery
with a loading ref and early return avoids unnecessary network calls. This design effectively prevents race conditions.
131-163
:loadTaskStatuses
andhandleFirstLoad
effectively centralize initial fetch logic.
This approach cleanly wraps your loaded data flow. Just be mindful of concurrency if multiple hooks callloadTaskStatuses
in parallel.apps/web/app/hooks/features/useTaskSizes.ts (8)
4-6
: Updated singular API method calls and imports check out.
Transitioning from plural forms likecreateTaskSizesAPI
tocreateTaskSizeAPI
clarifies single-item operations.
14-14
: Atom interactions for task sizes.
UsingtaskSizesListState
ensures a consistent global state for newly fetched or modified sizes. This pattern aligns with your approach in other hooks.
26-40
: Efficient loading workflow with error handling.
TheloadTaskSizes
function safely checks the loading ref to prevent redundant calls and logs failures, improving reliability.
42-54
: Create logic is succinct and extensible.
The code mergesdata
withorganizationTeamId
, simplifying usage. Good error management is in place.
56-67
: Deletion flow is straightforward.
Returningres.data
allows the caller to act on the updated data. This pattern is consistent with the rest of the hook.
69-81
: Edit handles updates with minimal overhead.
Again, returningres.data
is consistent, and console errors provide quick debugging.
83-88
:handleFirstLoad
triggers loading then callsfirstLoadTaskSizesData
.
This pattern gracefully separates initial load logic from the normal workflow, enhancing maintainability.
89-101
: Final return object is neatly organized.
ExposingcreateTaskSizeLoading
,deleteTaskSizeLoading
, andeditTaskSizeLoading
helps consumers manage UI states effectively.apps/web/app/services/client/api/task-sizes.ts (4)
3-3
: Good addition of cookie utility imports.Adding imports for cookie utilities aligns with the PR's objective to optimize data fetching by centralizing the retrieval of tenant and organization IDs.
5-10
: Function renamed and simplified correctly.Renaming from
createTaskSizesAPI
tocreateTaskSizeAPI
and retrieving thetenantId
internally improves consistency and reduces parameter passing across the codebase.
13-18
: Good refactoring of edit function.Similar to the create function, this edit function has been properly renamed and simplified by retrieving the
tenantId
internally.
21-22
: Function renamed consistently.Changing from
deleteTaskSizesAPI
todeleteTaskSizeAPI
maintains naming consistency with other functions.apps/web/components/pages/kanban/sort-tasks-status-settings.tsx (6)
3-3
: Updated imports to include the new interface.Adding
ITaskStatusOrder
to imports supports the improved implementation of the task status reordering functionality.
10-10
: Improved import organization.Consolidated the import from
react-beautiful-dnd
into a single line, which improves code readability.
12-17
: Code formatting improved.The component signature and state initialization have been cleaned up with consistent formatting.
18-27
: Cleaner hook usage and event handler.Updated to use the new hooks
reOrderTaskStatus
andsetTaskStatuses
, and maintained the existing drag-and-drop functionality.
28-64
: Enhanced state management after reordering.The
saveOrder
function has been significantly improved:
- Now creates a properly typed
ITaskStatusOrder
object- Awaits the API call result
- Updates the state with the new order if successful
- Properly comments out old code
This implementation ensures that the UI reflects the current state of task statuses after reordering.
65-107
: UI rendering maintained with improved formatting.The component's UI structure is preserved while improving code formatting for better readability.
apps/web/app/hooks/features/useTaskLabels.ts (7)
15-15
: Added useCallback import.Adding the useCallback import is necessary for the new async functions that were added in this file.
26-26
: Improved variable naming.Renaming variables to be more descriptive:
loading
→getTaskLabelsLoading
,queryCall
→getTaskLabelsQueryCall
, etc. This naming convention makes it clearer what each variable refers to.
41-41
: Removed unnecessary state.Removal of the
taskLabelsFetchingState
atom and associated logic simplifies the code and aligns with the PR's objective to reduce unnecessary state management.
43-60
: Improved async implementation.The
loadTaskLabels
function has been converted to use async/await and properly checks the loading ref before proceeding, which prevents duplicate API calls.
73-81
: Consistent usage of refactored query function.All functions that fetch task labels data have been updated to use the renamed
getTaskLabelsQueryCall
function, maintaining consistency throughout the code.Also applies to: 95-103, 114-122
129-132
: Added handleFirstLoad function.The new
handleFirstLoad
function centralizes the loading logic, ensuring data is loaded in the correct sequence and reduces redundant API calls.
134-147
: Updated return structure.The return object now uses the more descriptive loading state name and includes the new
handleFirstLoad
function to replace the direct call tofirstLoadTaskLabelsData
.apps/web/app/hooks/features/useDailyPlan.ts (11)
4-4
: Updated imports.Changed to explicitly import
useCallback
anduseMemo
, which are needed for the new async functions and hooks.
37-40
: Improved variable naming for query hooks.Renaming variables to be more descriptive (
getDayPlansByEmployeeLoading
,getAllDayPlansQueryCall
, etc.) improves code clarity and follows a consistent naming pattern.
57-63
: Removed unnecessary effect.Commented out the unnecessary useEffect that was setting the fetching state, which aligns with the PR's objective to simplify state management and reduce unnecessary updates.
67-79
: Converted to async/await with error handling.The
getAllDayPlans
function has been properly converted to use async/await with try/catch error handling, which improves error management and code readability.
81-87
: Added dedicated loading function.The new
loadAllDayPlans
function encapsulates the logic for fetching and updating the state with all daily plans, which helps reduce code duplication.
91-103
: Improved error handling for getMyDailyPlans.Similar to other functions,
getMyDailyPlans
now uses async/await with proper error handling to improve error management and code readability.
105-111
: Added loadMyDailyPlans function.This new function centralizes the logic for fetching and updating the state with personal daily plans, which helps reduce code duplication.
115-134
: Improved getEmployeeDayPlans function.The function has been converted to async/await with comprehensive error handling, including validation of the employeeId parameter.
136-145
: Added loadEmployeeDayPlans function.This new function centralizes the logic for fetching and updating employee day plans, including proper null checks for user.employee.id.
442-447
: Created handleFirstLoad function.The new
handleFirstLoad
function properly sequences the loading of all necessary data during initial load, which reduces redundant API calls and ensures data consistency.
498-501
: Updated exports with new functions.The return object now includes the new loading functions and the
handleFirstLoad
function, providing a more comprehensive API for the hook's consumers.apps/web/app/hooks/features/useOrganizationTeams.ts (9)
5-6
: Good use of cookie getters to reduce API calls.Adding these cookie retrieval functions enables direct access to important IDs without requiring additional API calls, which aligns with the PR's goal of reducing unnecessary network requests.
283-287
: Improved naming conventions enhance code clarity.Renaming these variables with more descriptive names (
getOrganizationTeamsLoading
,getOrganizationTeamsQueryCall
,getOrganizationTeamsLoadingRef
) makes the code more explicit and reduces ambiguity, especially when multiple similar hooks are used in the same component.
295-296
: Efficient use of cookie values.Retrieving organization and tenant IDs from cookies at initialization reduces redundant function calls during component lifecycle, contributing to performance optimization.
300-300
: Loading reference updated to match the new naming pattern.Updated reference to use the renamed loading variable, maintaining consistency throughout the component.
365-378
: Improved loading check with renamed reference.The function now correctly uses the renamed loading reference, maintaining the same functionality with better naming conventions.
434-434
: Dependencies array properly updated.The dependencies array has been correctly updated to reflect the renamed variables, ensuring the useCallback behaves as expected.
436-453
: Removed redundant API call mechanism.Commenting out this useEffect section eliminates a potentially redundant API call that was occurring on component mount, which directly contributes to the PR's goal of reducing unnecessary API calls.
496-508
: Optimized first load process with conditional API calls.The new
handleFirstLoad
function improves performance by:
- Centralizing first load logic in one place
- Only making the API call to get organization team when all required IDs are available
- Preventing unnecessary state updates when loading is in progress
This is a key optimization that aligns with the PR's goal of reducing unnecessary API calls.
512-518
: Return value updated to reflect new naming and functionality.The updated return statement properly exposes the renamed loading state and the new first load handler, maintaining a consistent API for consumers of this hook.
There was a problem hiding this 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
♻️ Duplicate comments (2)
apps/web/app/hooks/features/useOrganizationProjects.ts (1)
137-140
: 🛠️ Refactor suggestionMissing await for firstOrganizationProjectsLoad.
The
firstOrganizationProjectsLoad
function is not being awaited, which could lead to timing issues if it's an async function.const handleFirstLoad = useCallback(async () => { await loadOrganizationProjects(); - firstOrganizationProjectsLoad(); + await firstOrganizationProjectsLoad(); }, [firstOrganizationProjectsLoad, loadOrganizationProjects]);apps/web/app/hooks/features/useDailyPlan.ts (1)
442-447
: 🛠️ Refactor suggestionMissing error handling in handleFirstLoad.
The
handleFirstLoad
function should include a try/catch block to handle any errors that might occur during the loading process.const handleFirstLoad = useCallback(async () => { + try { await loadAllDayPlans(); await loadMyDailyPlans(); await loadEmployeeDayPlans(); firstLoadDailyPlanData(); + } catch (error) { + console.error('Error during first load:', error); + } }, [firstLoadDailyPlanData, loadAllDayPlans, loadEmployeeDayPlans, loadMyDailyPlans]);
🧹 Nitpick comments (3)
apps/web/lib/app/init-state.tsx (1)
147-149
: Use optional chaining for added safety.The current check can be simplified using optional chaining, which is a more concise way to handle potential undefined values.
-funcRef.current && funcRef.current(); +funcRef.current?.();🧰 Tools
🪛 Biome (1.9.4)
[error] 148-148: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/web/app/hooks/features/useDailyPlan.ts (2)
67-79
: Added error messages but the empty error case needs improvement.The function now properly uses async/await with try/catch blocks for error handling. However, the error message in line 74 doesn't include any details about the error, making debugging difficult.
if (res) { return res.data; } else { - console.error('Error fetching all day plans'); + console.error('Error fetching all day plans: Response was empty or invalid'); }
91-102
: Empty error response should include more details.Similar to the previous function, the empty response error message doesn't provide enough information for debugging.
if (res) { return res.data; } else { - console.error('Error fetching my daily plans'); + console.error('Error fetching my daily plans: Response was empty or invalid'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/app/hooks/features/useDailyPlan.ts
(6 hunks)apps/web/app/hooks/features/useOrganizationProjects.ts
(7 hunks)apps/web/app/services/client/api/task-sizes.ts
(1 hunks)apps/web/lib/app/init-state.tsx
(1 hunks)apps/web/lib/features/project/add-or-edit-project/index.tsx
(2 hunks)apps/web/lib/settings/task-sizes-form.tsx
(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/lib/features/project/add-or-edit-project/index.tsx
- apps/web/lib/settings/task-sizes-form.tsx
- apps/web/app/services/client/api/task-sizes.ts
🧰 Additional context used
🧬 Code Definitions (2)
apps/web/app/hooks/features/useDailyPlan.ts (2)
apps/web/app/services/client/api/daily-plan.ts (2)
getDayPlansByEmployeeAPI
(57-76)getAllDayPlansAPI
(15-34)apps/web/app/services/server/requests/daily-plan.ts (1)
getAllDayPlans
(12-42)
apps/web/lib/app/init-state.tsx (7)
apps/web/app/hooks/features/useOrganizationProjects.ts (1)
useOrganizationProjects
(18-159)apps/web/app/hooks/features/useRoles.ts (1)
useRoles
(9-100)apps/web/app/hooks/features/useTaskStatus.ts (1)
useTaskStatus
(18-164)apps/web/app/hooks/features/useTaskSizes.ts (1)
useTaskSizes
(12-102)apps/web/app/hooks/features/useTaskLabels.ts (1)
useTaskLabels
(22-147)apps/web/app/hooks/features/useTaskRelatedIssueType.ts (1)
useTaskRelatedIssueType
(23-139)apps/web/app/hooks/features/useDailyPlan.ts (1)
useDailyPlan
(33-503)
🪛 Biome (1.9.4)
apps/web/lib/app/init-state.tsx
[error] 148-148: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: deploy
🔇 Additional comments (20)
apps/web/lib/app/init-state.tsx (6)
48-50
: Good addition of first-load functionality for organization projects and roles.The addition of
firstLoadOrganizationProjectsData
andfirstLoadRolesData
helps consolidate all initialization in one place, which is a good practice for maintaining a clear loading sequence.
93-94
: Well-defined time constants improve maintainability.Extracting time intervals into named constants (five_minutes, one_minute) makes the code more readable and maintainable. This is a good practice for managing polling intervals.
111-134
: Well-optimized refresh intervals align with PR objectives.The changes to refresh intervals (from more frequent to less frequent) are excellent optimizations:
- Team data and tasks now refresh every minute instead of more frequently
- Task-related metadata (statuses, priorities, sizes, etc.) now refresh every 5 minutes
- Day plans refresh every 5 minutes
These changes directly support the PR objective of reducing unnecessary API calls by approximately 85%.
122-122
: Consider increasing invitation refresh interval for consistency.The invitation refresh interval is set to 60 seconds (one minute) while other metadata is refreshed every 5 minutes. For consistency with the PR's optimization goals, consider increasing this interval.
-useRefreshIntervalV2(myInvitations, 60 * 1000, true /* used as loadTeamTasksData deepCheck param */); +useRefreshIntervalV2(myInvitations, five_minutes, true /* used as myInvitations deepCheck param */);
115-120
: Good removal of frequent timer status polling.Commenting out the timer status refresh that was previously set to 5 seconds and replacing it with a one-minute interval on line 104 is a positive change that helps reduce API calls significantly.
64-90
: Well-structured initialization sequence.The
useOneTimeLoad
implementation ensures that all data is loaded once at component initialization, which provides a clean approach to manage the application's initial state. The addition of new first-load functions for organization projects and roles completes the initialization sequence.apps/web/app/hooks/features/useOrganizationProjects.ts (7)
19-20
: LGTM: Direct retrieval of IDs from cookies.The code now retrieves
tenantId
andorganizationId
directly from cookies at the top of the component, which is a good practice as it centralizes these values and avoids redundant calls to cookie functions.
23-23
: LGTM: Added first load hook integration.The integration with
useFirstLoad
hook aligns with the PR's goal of optimizing API calls by centralizing the initial loading logic.
44-55
: Improved error handling but type safety issue remains.The function now includes proper error handling with a specific error message when tenantId is missing, which is good. However, the
data
parameter is still typed asany
, which could lead to type safety issues.- (id: string, data: any) => { + (id: string, data: Record<string, unknown>) => {
62-67
: LGTM: Enhanced error handling with async/await.The function has been improved to use async/await with proper error handling, making the code more readable and maintainable.
99-101
: Potential undefined addition to array.When adding a new project to the organizationProjects array, the code should check if res.data exists to prevent undefined being added to the array.
- setOrganizationProjects([...organizationProjects, res.data]); + if (res?.data) { + setOrganizationProjects([...organizationProjects, res.data]); + }
123-134
: LGTM: Added optimization checks before API call.The function now checks if user exists and if organizationProjects already has data before making the API call, which helps reduce unnecessary API calls and aligns with the PR objectives.
156-157
: LGTM: Extended API for component consumers.Exposing
setOrganizationProjects
andfirstLoadOrganizationProjectsData
provides more control to component consumers and supports the optimization goal of the PR.apps/web/app/hooks/features/useDailyPlan.ts (7)
37-39
: LGTM: Standardized API naming pattern.The variable names now follow a consistent pattern, improving code readability.
57-57
: LGTM: Simplified first load mechanism.The integration with
useFirstLoad
hook is a good approach to manage the first load state, replacing the previous implementation that used a separate state variable.
59-63
: LGTM: Removed unnecessary useEffect.Removing this useEffect aligns with the PR's goal of reducing unnecessary operations and API calls.
81-87
: LGTM: Added dedicated loading function.This new function separates data fetching from state updates, creating a cleaner separation of concerns.
115-131
: LGTM: Improved error handling with input validation.The function now validates the input parameter and has better error handling with specific error messages, which is good for debugging.
136-145
: LGTM: Added employee data loading with dependency checks.This function efficiently loads employee plans only when necessary, reducing API calls when the user isn't available.
498-501
: LGTM: Extended API for component consumers.Exposing the loading functions and providing a unified
firstLoadDailyPlanData
function gives more control to consumers and aligns with the PR's optimization goals.
@CREDO23 thanks for tons of work on this one 🚀 ! @Innocent-Akim can you help to review this before I also digg into it? Thanks in advance! |
- Add fallback empty array when accessing tasksFiltered[tab] - Add safety check in useMemo hook to handle undefined tasks - Prevents 'Cannot read properties of undefined (reading 'filter')' error
There was a problem hiding this 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
🧹 Nitpick comments (5)
apps/web/lib/settings/task-statuses-form.tsx (1)
88-126
: Suggestion: Consider using optional chaining in Promise handlingThe implementation of async/await in the onSubmit function is good, but there's a potential improvement for the Promise chain on line 101.
- })?.then(() => { + })?.then(() => {The static analysis tool correctly identified this, and you're already using optional chaining, which is appropriate given that the createTaskStatus and editTaskStatus functions may return undefined if there's an error.
🧰 Tools
🪛 Biome (1.9.4)
[error] 104-104: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/web/lib/settings/task-sizes-form.tsx (4)
81-117
: Async/await implementation should be consistentThe onSubmit function has a mix of promise chaining with
?.then()
and async/await patterns. While this works, it would be more consistent to use async/await throughout.- createTaskSize({ - name: values.name, - color: values.color, - // description: '', - organizationId: user?.employee?.organizationId, - tenantId: user?.tenantId, - icon: values.icon - // projectId: '', - })?.then(() => { - !formOnly && setCreateNew(false); - - onCreated && onCreated(); - refetch(); - reset(); - }); + const result = await createTaskSize({ + name: values.name, + color: values.color, + // description: '', + organizationId: user?.employee?.organizationId, + tenantId: user?.tenantId, + icon: values.icon + // projectId: '', + }); + + if (result) { + !formOnly && setCreateNew(false); + + onCreated && onCreated(); + refetch(); + reset(); + }Similar changes should be applied to the edit task size section as well.
🧰 Tools
🪛 Biome (1.9.4)
[error] 95-95: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
95-95
: Use optional chaining as suggested by static analysisThe static analysis tool correctly suggests using optional chaining here.
- onCreated && onCreated(); + onCreated?.();🧰 Tools
🪛 Biome (1.9.4)
[error] 95-95: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
214-214
: Use optional chaining as suggested by static analysisThe static analysis tool correctly suggests using optional chaining here.
- {taskSizes && taskSizes?.length ? ( + {taskSizes?.length ? (🧰 Tools
🪛 Biome (1.9.4)
[error] 214-214: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
31-49
: Consider moving icon list generation outside componentThe icon lists are generated on every render, which is unnecessary since they're static. Consider moving this outside the component or using useMemo to optimize performance.
+// Move outside component or wrap in useMemo +const taskStatusIconList: IIcon[] = generateIconList('task-statuses', [ + 'open', + 'in-progress', + 'ready', + 'in-review', + 'blocked', + 'completed' +]); +const taskSizesIconList: IIcon[] = generateIconList('task-sizes', [ + 'x-large' + // 'large', + // 'medium', + // 'small', + // 'tiny', +]); +const taskPrioritiesIconList: IIcon[] = generateIconList('task-priorities', ['urgent', 'high', 'medium', 'low']); +const iconList: IIcon[] = [...taskStatusIconList, ...taskSizesIconList, ...taskPrioritiesIconList]; export const TaskSizesForm = ({ formOnly = false, onCreated }: StatusForm) => { const user = useAtomValue(userState); const { register, setValue, handleSubmit, reset, getValues } = useForm(); const [createNew, setCreateNew] = useState(formOnly); const [edit, setEdit] = useState<ITaskSizesItemList | null>(null); const t = useTranslations(); - const taskStatusIconList: IIcon[] = generateIconList('task-statuses', [ - 'open', - 'in-progress', - 'ready', - 'in-review', - 'blocked', - 'completed' - ]); - const taskSizesIconList: IIcon[] = generateIconList('task-sizes', [ - 'x-large' - // 'large', - // 'medium', - // 'small', - // 'tiny', - ]); - const taskPrioritiesIconList: IIcon[] = generateIconList('task-priorities', ['urgent', 'high', 'medium', 'low']); - - const iconList: IIcon[] = [...taskStatusIconList, ...taskSizesIconList, ...taskPrioritiesIconList];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/lib/features/task/task-filters.tsx
(2 hunks)apps/web/lib/settings/task-sizes-form.tsx
(1 hunks)apps/web/lib/settings/task-statuses-form.tsx
(2 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
apps/web/lib/settings/task-sizes-form.tsx (1)
apps/web/app/hooks/features/useTaskSizes.ts (1)
useTaskSizes
(12-102)
apps/web/lib/settings/task-statuses-form.tsx (6)
apps/web/lib/settings/icon-items.tsx (1)
generateIconList
(96-104)apps/web/app/hooks/features/useTaskStatus.ts (1)
useTaskStatus
(18-164)apps/web/app/hooks/features/useRefetchData.ts (1)
useRefetchData
(8-24)apps/web/app/hooks/features/useTeamTasks.ts (1)
useTeamTasks
(79-548)apps/web/lib/settings/list-card.tsx (1)
StatusesListCard
(11-82)apps/web/lib/features/task-status/delete-status-confirmation-modal.tsx (1)
DeleteTaskStatusConfirmationModal
(25-105)
🪛 Biome (1.9.4)
apps/web/lib/settings/task-sizes-form.tsx
[error] 95-95: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 214-214: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/web/lib/settings/task-statuses-form.tsx
[error] 104-104: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: deploy
🔇 Additional comments (10)
apps/web/lib/features/task/task-filters.tsx (2)
95-95
: Good defensive programming!Adding the fallback empty array (
|| []
) ensures thattasks
is always an array even whentasksFiltered[tab]
is undefined. This prevents potential runtime errors during filtering operations and contributes to the overall goal of reducing unnecessary API calls that might be triggered by error handling.
217-217
: Consistent defensive approach - good practice.Similarly to the earlier change, adding the fallback empty array guarantees that filtering operations always work on an array. This prevents potential crashes when filtering operations are performed and ensures consistent behavior across the application.
apps/web/lib/settings/task-statuses-form.tsx (5)
56-65
: LGTM: Improved hook destructuring for better state managementThe renaming of variables from
loading
togetTaskStatusesLoading
andtaskStatus
totaskStatuses
improves clarity and consistency. The addition ofsetTaskStatuses
enables proper state updates throughout the component.
294-342
: LGTM: Updated rendering logic to use renamed variablesThe rendering logic has been properly updated to check
taskStatuses.length
instead oftaskStatus.length
and to usegetTaskStatusesLoading
for the loading spinner, maintaining consistency with the variable renaming.
313-333
: Improved state management after task status deletionThe implementation now properly updates the task status state after deletion using the new
setTaskStatuses
function. This ensures the UI stays in sync with the backend state without requiring additional API calls.This change aligns perfectly with the PR objective of reducing unnecessary API calls by maintaining local state properly.
127-131
: LGTM: Consistent variable renamingThe update from
taskStatus.slice()
totaskStatuses.slice()
maintains consistency with the variable renaming throughout the component.
67-74
: LGTM: Dependency array update in useEffectThe dependency array correctly includes
taskStatuses
instead of the previous variable name, ensuring proper reactive behavior.apps/web/lib/settings/task-sizes-form.tsx (3)
50-58
: Good refactoring for API function naming consistencyNice job on consistently renaming the functions and loading states from plural to singular form. This makes the codebase more consistent and aligns with standard naming conventions for single-item operations.
186-186
: Good use of loading states for the buttonNice implementation of using both loading states to control the button's disabled and loading states. This prevents users from submitting multiple times.
226-228
: Good implementation of async/await for delete operationThe delete operation now properly uses async/await syntax, which aligns with the PR's goal of optimizing API calls and ensuring they're only triggered when necessary.
Description :
Optimized Data Fetching
Adjusted Polling Intervals
Impact 🚀
Type of Change
Checklist
Previous screenshots
Screen.Recording.2025-03-23.at.18.08.05.mov
Current screenshots
Screen.Recording.2025-03-23.at.18.38.18.mov
Summary by CodeRabbit
New Features
Refactor
useRoles
hook has been restructured to improve state management and error handling.Bug Fixes
Style