Skip to content
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

[Fix bug] Daily Plan | Same tasks are shown for different users #3481

Merged

Conversation

CREDO23
Copy link
Contributor

@CREDO23 CREDO23 commented Dec 24, 2024

Description

Fixes #3461

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

Previous screenshots

Notion

Current screenshots

Loom

Summary by CodeRabbit

  • New Features

    • Added user-specific filtering for tasks across multiple components, enhancing task visibility based on user association.
    • Updated user-related properties in various components to support user context in task management.
  • Bug Fixes

    • Improved task filtering logic to ensure only relevant tasks are displayed based on user membership.
  • Documentation

    • Updated interfaces and method signatures to reflect the inclusion of the user parameter in various components.

@CREDO23 CREDO23 self-assigned this Dec 24, 2024
@CREDO23 CREDO23 linked an issue Dec 24, 2024 that may be closed by this pull request
Copy link
Contributor

coderabbitai bot commented Dec 24, 2024

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between ddc01b2 and f3e8659.

📒 Files selected for processing (2)
  • apps/web/lib/features/task/daily-plan/outstanding-all.tsx (1 hunks)
  • apps/web/lib/features/task/daily-plan/outstanding-date.tsx (1 hunks)

Walkthrough

This pull request introduces user-specific task filtering across multiple components in the daily plan and task management system. The changes add an optional user parameter to various components like FutureTasks, OutstandingAll, OutstandingFilterDate, PastTasks, and UserProfileTask. These modifications enable filtering of tasks based on a specific user's ID, ensuring that users only see tasks they are associated with, addressing the issue of tasks being duplicated across team members.

Changes

File Change Summary
apps/web/lib/features/task/daily-plan/future-tasks.tsx Added optional user parameter to filter future tasks by user
apps/web/lib/features/task/daily-plan/outstanding-all.tsx Updated to filter tasks based on optional user parameter
apps/web/lib/features/task/daily-plan/outstanding-date.tsx Added user filtering logic to OutstandingFilterDate component
apps/web/lib/features/task/daily-plan/past-tasks.tsx Introduced user-specific task filtering for past tasks
apps/web/lib/features/task/daily-plan/task-estimated-count.tsx Modified getTotalTasks to support user-based task counting
apps/web/lib/features/team/user-team-card/index.tsx Updated UserProfileTask to include user prop
apps/web/lib/features/user-profile-plans.tsx Added user filtering across various task components
apps/web/lib/features/user-profile-tasks.tsx Added optional user parameter to UserProfileTask

Assessment against linked issues

Objective Addressed Explanation
Tasks should be unique to each user
Only show tasks relevant to the user

Possibly related PRs

Suggested reviewers

  • evereq
  • Cedric921

Poem

🐰 Hopping through code with glee,
Tasks now filtered, user-specific and free!
No more duplicates, each view so clear,
Rabbit's magic makes data appear!
Filtering tasks with a wiggle and wink! 🌟


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

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

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

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

CodeRabbit Commands (Invoked using PR comments)

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

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

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

Documentation and Community

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 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 new useEffect that filters futurePlans by checking task.members is correct for ensuring that each user only sees their own tasks. Ensure that task.members and user.id are always defined to avoid potential runtime errors. As a good-to-have, consider short-circuiting early if !user to skip extra operations.

apps/web/lib/features/task/daily-plan/past-tasks.tsx (2)

34-34: Avoid casting date to any.

Casting date to any may mask type errors. Consider refining the type or adding an overload to filterDailyPlan if needed.

- setPastPlans(filterDailyPlan(date as any, pastPlans));
+ setPastPlans(filterDailyPlan(date, pastPlans));

37-50: Consider streamlining the user-based filtering logic.

Chaining map and filter is 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.members array. However, consider pre-checking if (!user) return tasks to 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

📥 Commits

Reviewing files that changed from the base of the PR and between ee35122 and ddc01b2.

📒 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.

apps/web/lib/features/task/daily-plan/outstanding-date.tsx Outdated Show resolved Hide resolved
@Cedric921 Cedric921 added this to the v0.2.0 milestone Dec 26, 2024
@evereq evereq merged commit 05eaa04 into develop Dec 26, 2024
13 checks passed
@evereq evereq deleted the 3461-bug-daily-plan--same-tasks-are-shown-for-different-users branch December 26, 2024 07:23
Innocent-Akim pushed a commit that referenced this pull request Dec 26, 2024
* show relevant tasks for each member in the same team | team member card view

* fix deepSan

* add coderabbit ai suggest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug - Daily Plan | Same tasks are shown for different users
3 participants