Skip to content

Comments

fix: personal events in 'All' filter#23258

Closed
pallava-joshi wants to merge 3 commits intocalcom:mainfrom
pallava-joshi:fix/insights-event-type-filter
Closed

fix: personal events in 'All' filter#23258
pallava-joshi wants to merge 3 commits intocalcom:mainfrom
pallava-joshi:fix/insights-event-type-filter

Conversation

@pallava-joshi
Copy link
Contributor

What does this PR do?

This PR fixes a bug on the Insights page where the "Event Type" filter would not show a user's personal event types when the "All" scope (for an organization) was selected.

The root cause was a backend database query that only fetched event types associated with a teamId, omitting personal events where teamId is null.

The fix updates the Prisma query to use an OR condition, ensuring it fetches both team-owned and user-owned event types for a complete and accurate filter list.

Fixes #23257

Visual Demo (For contributors especially)

A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).

Video Demo (if applicable):

Screencast.From.2025-08-21.19-35-08.mp4

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • N/A
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

Prerequisites: Log in as a user who:

  • Is an Owner or Admin of an organization.
  • Has at least one personal event type (not associated with any team).
  • The organization has at least one team with its own event types.

Steps to Reproduce:

  • Navigate to the /insights page.
  • In the top filter bar, ensure the scope is set to "All" (your organization).
  • Click the + button to add a new filter.
  • Select Event Type from the dropdown.

Expected Result:

  • The dropdown list for the Event Type filter should now contain both the event types from all teams in your organization and your personal event types.

@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Aug 21, 2025
@vercel
Copy link

vercel bot commented Aug 21, 2025

@pallava-joshi is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 21, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The PR modifies getEventTypeList in packages/features/insights/server/trpc-router.ts. For the admin/org-wide (isAll) path, it replaces a teamId IN filter with an OR condition: teamId in [organizationId, ...childrenTeamIds] OR (userId equals current user AND teamId is null). Control flow for applying userId/teamId filters is reordered to prefer the isAll branch; otherwise, it falls back to teamId (when provided and not isAll) or userId. Other logic remains unchanged.

Assessment against linked issues

Objective Addressed Explanation
Include personal event types in Event Type filter for admin/org-wide view (#23257)
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ 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.
    • 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.
  • 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 the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

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.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • 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.

@graphite-app graphite-app bot requested a review from a team August 21, 2025 14:15
@github-actions github-actions bot added consumer event-types area: event types, event-types 🐛 bug Something isn't working labels Aug 21, 2025
@dosubot dosubot bot added the insights area: insights, analytics label Aug 21, 2025
@graphite-app
Copy link

graphite-app bot commented Aug 21, 2025

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (08/21/25)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add community label" took an action on this PR • (08/21/25)

1 label was added to this PR based on Keith Williams's automation.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
packages/features/insights/server/trpc-router.ts (2)

1096-1102: Confirm behavior when teamId is not provided in “All” scope

This block depends on childrenTeamIds, which is only populated earlier when isAll && teamId && orgAdmin. If the client ever calls this with isAll=true but without teamId (and with some userId present), orgTeamIds will omit child teams, potentially under-fetching event types. If the UI always passes the org’s teamId in “All” scope, we’re fine—otherwise consider an inline fallback.

Optional fallback within this block (only triggers when childrenTeamIds is empty):

-    const orgTeamIds = [user.organizationId, ...childrenTeamIds];
+    const childIds =
+      childrenTeamIds.length > 0
+        ? childrenTeamIds
+        : (
+            await prisma.team.findMany({
+              where: { parentId: user.organizationId },
+              select: { id: true },
+            })
+          ).map((t) => t.id);
+    const orgTeamIds = [user.organizationId, ...childIds];

Please confirm the frontend always passes the org teamId for “All”, or adopt the fallback to be defensive.


1096-1112: Add targeted tests to prevent regressions

Recommend adding tests that assert:

  • Org admin + isAll: includes team-owned event types across org + current user’s personal event types; excludes other users’ personal event types.
  • Non-admin + isAll: still denied or constrained per existing authorization rules (no leakage).
  • isAll without teamId (if possible in UI): either assert UI always sends teamId or cover the fallback behavior.

I can draft test cases for getEventTypeList using a seeded org with two teams and two users if helpful.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between cafecad and a2c5888.

📒 Files selected for processing (1)
  • packages/features/insights/server/trpc-router.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/features/insights/server/trpc-router.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • packages/features/insights/server/trpc-router.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • packages/features/insights/server/trpc-router.ts
🧠 Learnings (1)
📚 Learning: 2025-08-21T13:44:06.784Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.784Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.

Applied to files:

  • packages/features/insights/server/trpc-router.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (2)
packages/features/insights/server/trpc-router.ts (2)

1096-1109: Correct fix: include personal event types under org-wide (“All”) scope

The OR condition correctly adds the current user’s personal event types (teamId null) alongside org/team event types when the requester is an org admin. Selection set complies with our “select-only” Prisma guideline.


1111-1113: Clarify intent of userId filter (personal-only vs. all owned event types)

Else branch sets only userId, which will include both personal (teamId null) and team-owned event types where the user is the owner. If the intent of this path is “personal only,” add teamId: null. If the intent is “user’s event types across teams,” current logic is correct. Please confirm desired semantics.

-    eventTypeWhereConditional["userId"] = userId;
+    eventTypeWhereConditional["userId"] = userId;
+    // Uncomment if the intent is "personal-only" event types:
+    // eventTypeWhereConditional["teamId"] = null;

Copy link
Contributor

@kart1ka kart1ka left a comment

Choose a reason for hiding this comment

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

It does not work when "Your account" is selected.
image

@kart1ka kart1ka marked this pull request as draft August 21, 2025 16:35
@pallava-joshi
Copy link
Contributor Author

@kart1ka it’s working on my machine - can you please double-check if you also have an event under the Personal tab?
If not, lmk which account you’re logged in with so I can try to reproduce.

image

@pallava-joshi pallava-joshi requested a review from kart1ka August 21, 2025 17:39
@kart1ka
Copy link
Contributor

kart1ka commented Aug 21, 2025

@pallava-joshi The issue is in an org account.

@pallava-joshi
Copy link
Contributor Author

@Devanshusharma2005 can you check on your machine - I tried with these 3 accounts:

They both have a few events under their Personal and Team event types. On my end, everything appears to be working correctly. The member is able to see team events, and the filtering functions as expected for both roles.

image image

Copy link
Contributor

@kart1ka kart1ka left a comment

Choose a reason for hiding this comment

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

"Your Accounts" show both team events and personal/user events, which is wrong. Only user events should be shown. See the video attached. Only "All" should show both team and user events.

https://cap.link/7gbe7kew6att25a

Please also write tests to make sure everything works as expected.

@kart1ka
Copy link
Contributor

kart1ka commented Aug 25, 2025

Closing in favour of #23343

@kart1ka kart1ka closed this Aug 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working community Created by Linear-GitHub Sync consumer event-types area: event types, event-types insights area: insights, analytics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Event Type filter is missing personal event types

2 participants