[Fix bug] Daily Plan | Same tasks are shown for different users #3481
Conversation
|
Warning Rate limit exceeded@CREDO23 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 22 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis pull request introduces user-specific task filtering across multiple components in the daily plan and task management system. The changes add an optional Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
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: 1
🧹 Nitpick comments (5)
apps/web/lib/features/task/daily-plan/future-tasks.tsx (1)
33-47: Efficiently filter tasks based on user membership.
The newuseEffectthat filtersfuturePlansby checkingtask.membersis correct for ensuring that each user only sees their own tasks. Ensure thattask.membersanduser.idare always defined to avoid potential runtime errors. As a good-to-have, consider short-circuiting early if!userto skip extra operations.apps/web/lib/features/task/daily-plan/past-tasks.tsx (2)
34-34: Avoid castingdatetoany.Casting
datetoanymay mask type errors. Consider refining the type or adding an overload tofilterDailyPlanif needed.- setPastPlans(filterDailyPlan(date as any, pastPlans)); + setPastPlans(filterDailyPlan(date, pastPlans));
37-50: Consider streamlining the user-based filtering logic.Chaining
mapandfilteris fine, but be mindful of potential performance overhead if the dataset is large. A single pass could achieve the same in a more optimal manner. Nonetheless, this approach is perfectly acceptable for smaller data volumes.apps/web/lib/features/user-profile-plans.tsx (2)
178-195: Efficient user membership filtering recommended.The logic correctly filters tasks for the specified user by checking the
task.membersarray. However, consider pre-checkingif (!user) return tasksto avoid mapping over all tasks unnecessarily for users who are not logged in. This could be beneficial in large data sets.
331-345: Filtering tasks by user membership.The useEffect hook properly filters tasks for the given user. For performance at scale, consider caching or memoizing the result to avoid repeated data transformations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/lib/features/task/daily-plan/future-tasks.tsx(2 hunks)apps/web/lib/features/task/daily-plan/outstanding-all.tsx(1 hunks)apps/web/lib/features/task/daily-plan/outstanding-date.tsx(1 hunks)apps/web/lib/features/task/daily-plan/past-tasks.tsx(1 hunks)apps/web/lib/features/task/daily-plan/task-estimated-count.tsx(2 hunks)apps/web/lib/features/team/user-team-card/index.tsx(1 hunks)apps/web/lib/features/user-profile-plans.tsx(6 hunks)apps/web/lib/features/user-profile-tasks.tsx(2 hunks)
🔇 Additional comments (31)
apps/web/lib/features/task/daily-plan/future-tasks.tsx (2)
15-15: Import of new interface looks good.
The addition of the IUser interface is consistent with the need to pass the optional user parameter for filtered tasks.
20-20: Ensure the optional user prop behaves as intended.
Declaring the user parameter as optional with user?: IUser is appropriate. Be mindful that subsequent code correctly handles the case when user is undefined or null.
apps/web/lib/features/task/daily-plan/past-tasks.tsx (7)
13-13: Good introduction of the IUser import.
This import properly sets up type definitions for the new user-based filtering logic.
18-26: New user prop in component signature is clear.
Allowing an optional user prop is a clean solution for user-based filtering. However, ensure that any consumer of <PastTasks> properly handles the case when user is undefined.
30-30: State initialization looks good.
Using _pastPlans from the hook to set the initial component state is consistent with existing patterns. No issues here.
51-51: Check for potential infinite re-render in useEffect.
The effect depends on pastPlans and calls setPastPlans, which might produce a new array reference each time. Confirm that the filtering step is idempotent or that the equality check prevents unnecessary renders.
55-55: Conditional rendering for empty pastPlans looks good.
The fallback to <EmptyPlans> clarifies the UI experience when there are no tasks.
56-56: Drag-and-drop context is well-integrated.
Passing setPastPlans to the handleDragAndDrop callback is consistent with the rest of the daily plan flows.
62-62: Mapping through pastPlans is straightforward.
Generating an <AccordionItem> for each plan and rendering tasks is an idiomatic approach that maintains readability.
apps/web/lib/features/task/daily-plan/task-estimated-count.tsx (2)
2-2: Adding IUser import looks correct.
No issues noticed with importing IUser alongside IDailyPlan.
50-63: Validate task filtering logic & consider potential performance improvements.
The logic correctly filters tasks by user membership. However, if any step in the pipeline (e.g., plan.tasks, plan.tasks?.filter(...)) is large, this could become costly. If applicable, consider optimizing by short-circuiting or partial loading of tasks.
apps/web/lib/features/task/daily-plan/outstanding-all.tsx (4)
11-11: IUser import is consistent with the new user-based filtering.
No issues with the new import.
16-16: Optional user prop enhances flexibility.
Including user?: IUser is a good approach for optional filtering.
18-18: Prop destructuring is straightforward.
Well done on passing user as a prop.
23-28: Implement fallback or defensive checks on user tasks.
The filtering is correct, but you might consider an additional null check to handle unexpected data or missing members gracefully.
apps/web/lib/features/task/daily-plan/outstanding-date.tsx (4)
12-13: useEffect and IUser import introduced.
Looks good. The useEffect is presumably used for re-filtering tasks when user changes.
17-17: Optional user prop addition.
Using the same pattern as in the other files for user-based filtering.
19-23: Initialize local plans state from outstandingPlans.
No immediate concerns; just ensure plans is always in sync with outstandingPlans if future logic calls for updates.
42-49: Rendering logic with plans?.length > 0
No issues found. Logic for handling empty vs. filtered state is good.
apps/web/lib/features/team/user-team-card/index.tsx (1)
129-129: Passing user to UserProfileTask
Good approach to ensure user-specific tasks are displayed. Verify that member?.employee?.user is not undefined.
apps/web/lib/features/user-profile-plans.tsx (7)
29-29: Import usage approved.
Bringing in the IUser interface here is consistent with the new feature requiring user-based filtering.
58-60: New interface definition looks good.
Defining IUserProfilePlansProps with an optional user?: IUser; is a clean approach to handling user-based filtering.
62-62: Function signature updated properly.
Explicitly accepting props implementing IUserProfilePlansProps ensures type safety and clarity in usage.
65-65: Destructuring the user object.
Assigning user from props is straightforward. Ensure any function calls within this component take into account when user may be undefined.
85-86: Unified approach to outstanding tasks.
Passing the user prop to <OutstandingAll> and <OutstandingFilterDate> ensures consistent user filtering in outstanding tasks.
89-92: Ensuring user-based filtering across tabs.
All daily plan tab components now accept user for filtering tasks. This aligns with the objective to show only tasks belonging to the correct user.
303-311: AllPlans function signature updated.
Including an optional user prop aligns this function with the rest of the user-specific filtering architecture. Great job maintaining consistent types.
apps/web/lib/features/user-profile-tasks.tsx (4)
11-11: Import referencing also looks good.
Importing IUser allows passing the user object for more granular task filtering.
17-17: Optional user prop.
Declaring user?: IUser; in Props ensures backward compatibility for components that do not require user-based filtering.
25-25: Enhanced function signature.
Adding user to UserProfileTask parameters aligns with the new feature, enabling user-bound task filtering logic.
98-98: Passing user object to the daily plan.
Connecting user ensures the daily plan displays the correct tasks, addressing the PR’s main objective of removing cross-user task duplication.
evereq
deleted the
3461-bug-daily-plan--same-tasks-are-shown-for-different-users
branch
* show relevant tasks for each member in the same team | team member card view * fix deepSan * add coderabbit ai suggest
Description
Fixes #3461
Type of Change
Checklist
Previous screenshots
Notion
Current screenshots
Loom
Summary by CodeRabbit
New Features
Bug Fixes
Documentation