feat: Manage Team ooo or crud and fetch past ooo records #16456

vijayraghav-io · 2024-09-02T07:37:17Z

What does this PR do?

Fixes [CAL-4527] OOO: Show your own history and give Admins CRUD and see all team members history #16333
Fixes CAL-

Approach to fetch past records:- Infinite scrolling with pagination is implemented to fetch additional records (10 at a time) when the user scrolls to the bottom of the table.
Also search by username and email is implemented to fetch past records of a member quickly.

https://www.loom.com/share/fe88b4739846419493fe26836daedf88?sid=96acca39-eaad-4fcc-89e9-c2153532e080

Mandatory Tasks (DO NOT REMOVE)

I have self-reviewed the code (A decent size PR without self-review might be rejected).
- N/A - I have added a Docs issue here if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
I confirm automated tests are in place that prove my fix is effective or that my feature works.

vercel · 2024-09-02T07:37:22Z

@vijayraghav-io is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

graphite-app · 2024-09-02T07:40:10Z

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (09/02/24)

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

"Add community label" took an action on this PR • (09/02/24)

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

"Add foundation team as reviewer" took an action on this PR • (09/23/24)

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

anikdhabal · 2024-09-02T07:55:03Z

packages/features/settings/OutOfOfficeEntriesListForTeam.tsx

+interface OutOfOfficeEntry {
+  id: number;
+  uuid: string;
+  start: Date;
+  end: Date;
+  toUserId: number | null;
+  toUser: {
+    username: string;
+  } | null;
+  reason: {
+    id: number;
+    emoji: string;
+    reason: string;
+    userId: number;
+  } | null;
+  notes: string | null;
+  user: { id: number; avatarUrl: string; username: string; email: string } | null;
+}


This is defined twice, can you pls reuse it another component?

yeah, it can be reused, will update

anikdhabal · 2024-09-02T07:57:02Z

packages/features/settings/CreateOrEditOutOfOfficeModal.tsx

+    listMembers
+      ?.filter((member) => (oooType === "mine" ? me?.data?.id !== member.id : oooType === "team"))


It’s better to use enum instead of hardcoded values like "mine" and "team."

Amit91848 · 2024-09-02T07:58:49Z

apps/web/pages/settings/my-account/out-of-office/index.tsx

+          oooType={oooType}
+          setTeamOOOEntriesUpdated={setTeamOOOEntriesUpdated}
+          setMyOOOEntriesUpdated={setMyOOOEntriesUpdated}
+        />
+      )}
+      {oooType === "team" ? (
+        <OutOfOfficeEntriesListForTeam
+          editOutOfOfficeEntry={editOutOfOfficeEntry}
+          teamOOOEntriesUpdated={teamOOOEntriesUpdated}
+        />
+      ) : (
+        <OutOfOfficeEntriesList
+          editOutOfOfficeEntry={editOutOfOfficeEntry}
+          myOOOEntriesUpdated={myOOOEntriesUpdated}


Why are there separate components for Team and Mine view?

As far as I can tell both are same with same functionality and code?

they are similar, but differ in columns displayed. Wanted to avoid having many conditions within same file for team and mine and keep it separated , s

handled in single component

Amit91848 · 2024-09-02T08:09:34Z

Unable to scroll Redirect To Member when editing an entry, can only scroll it while adding a new that too I have to close it and open it for it to work, can you also replicate it?

Recording.2024-09-02.133726.mp4

Amit91848 · 2024-09-02T08:12:38Z

packages/trpc/server/routers/loggedInViewer/outOfOffice.handler.ts

+        : false,
+    },
+    orderBy: {
+      end: "desc",


Imo it is better to sort by start date rather than end.

anikdhabal · 2024-09-02T08:13:38Z

packages/trpc/server/routers/loggedInViewer/outOfOffice.handler.ts

+  let deletingOooOfUserId = ctx.user.id;
+  let deletingOooOfEmail = ctx.user.email;
+  let deletingOooOfUserName = ctx.user.username;


I don't like the variable names in the whole file; they need to be improved

i was trying to be more descriptive for purpose of new variables, updated, hope it is better now.

Amit91848

Great stuff as usual @vijayraghav-io , but this needs some more improvement.

For orgs I can add redirection from one team to another

Recording.2024-09-02.134712.mp4

anikdhabal · 2024-09-02T08:22:21Z

packages/trpc/server/routers/loggedInViewer/outOfOffice.handler.ts

+    const members = await prisma.membership.findMany({
+      where: {
+        teamId: {
+          in: teams.map((team) => team.teamId),
+        },
+        userId: {
+          not: ctx.user.id,
+        },
+      },
+      select: {
+        userId: true,
+      },
+    });
+    if (members.length === 0) {
+      throw new TRPCError({ code: "NOT_FOUND", message: "no_team_members" });
+    }
+    fetchOOOEntriesForIds = members.map((member) => member.userId);
+  }


This is a very performance-reducing operation. Need to be improved

Amit91848 · 2024-09-02T08:23:19Z

packages/trpc/server/routers/loggedInViewer/outOfOffice.handler.ts

-      start: "asc",
+  });
+
+  const outOfOfficeEntries = await prisma.outOfOfficeEntry.findMany({


Can you not add _count in select for this, instead of having separate queries for count and actual data?

The first query gets the total count of matching records, while the second query fetches only limited records (say 10). I tried _count can't be used here.

anikdhabal · 2024-09-02T08:33:19Z

packages/trpc/server/routers/loggedInViewer/outOfOffice.handler.ts

+import type {
+  TOutOfOfficeDelete,
+  TOutOfOfficeEntriesListSchema,
+  TOutOfOfficeInputSchema,
+} from "./outOfOffice.schema";
+
+// function getTeam() checks if there is a team where 'adminUserId' is admin or owner
+// and 'memberUserId' is a member.
+// If exists returns the first found such team.
+const getTeam = async (adminUserId: number, memberUserId: number) => {
+  return await prisma.team.findFirst({
+    where: {
+      AND: [
+        {
+          members: {
+            some: {
+              userId: adminUserId,
+              role: {
+                in: [MembershipRole.ADMIN, MembershipRole.OWNER],
+              },
+            },
+          },
+        },
+        {
+          members: {
+            some: {
+              userId: memberUserId,
+            },
+          },
+        },
+      ],
+    },


Too many changes in the backend inside a single file. We definitely need to add tests

Added Tests

anikdhabal · 2024-09-02T08:39:56Z

packages/features/settings/OutOfOfficeEntriesListForTeam.tsx

+export const OutOfOfficeEntriesListForTeam = ({
+  editOutOfOfficeEntry,
+  teamOOOEntriesUpdated,
+}: {
+  editOutOfOfficeEntry: (entry: BookingRedirectForm) => void;
+  teamOOOEntriesUpdated: number;
+}) => {
+  const { t } = useLocale();
+  const tableContainerRef = useRef<HTMLDivElement>(null);
+  const [debouncedSearchTerm, setDebouncedSearchTerm] = useState("");
+  const [deletedEntry, setDeletedEntry] = useState(0);
+  const { data, isPending, fetchNextPage, isFetching, refetch } =
+    trpc.viewer.outOfOfficeEntriesList.useInfiniteQuery(
+      {
+        limit: 10,
+        fetchTeamMembersEntries: true,
+        searchTerm: debouncedSearchTerm,
+      },
+      {
+        getNextPageParam: (lastPage) => lastPage.nextCursor,
+        placeholderData: keepPreviousData,
+      }
+    );
+
+  useEffect(() => {
+    refetch();
+  }, [teamOOOEntriesUpdated, deletedEntry, refetch]);
+
+  const totalDBRowCount = data?.pages?.[0]?.meta?.totalRowCount ?? 0;
+  //Flatten the array of arrays from the useInfiniteQuery hook
+  const flatData = useMemo(
+    () => data?.pages?.flatMap((page) => page.rows) ?? [],
+    [data]
+  ) as OutOfOfficeEntry[];
+  const totalFetched = flatData.length;
+


Hey, this is similar to the other component. I know there are a few different cases that don't match, but instead of doing the same things twice in two different components, can you please try to consolidate them into a single component? If things become too difficult and there are too many visible conditional changes, it's okay to define 2 components. However, in such cases, define things once and use them in another component. For example, EmptyScreen is defined twice, etc.

handled in single component

anikdhabal

Great work @vijayraghav-io , don't test it locally but codewise I've some doubt.

vijayraghav-io · 2024-09-02T08:44:21Z

Thankyou! 🙏 @Amit91848 @anikdhabal , sure will address the comments.

vijayraghav-io · 2024-09-04T18:46:09Z

Unable to scroll Redirect To Member when editing an entry, can only scroll it while adding a new that too I have to close it and open it for it to work, can you also replicate it?

I checked multiple times, not able to recreate this for me. Able to search the list of members for redirect also.

CarinaWolli

I didn't review the code but tested it locally, found the following issues:

We need rounded corners in both my and team OOO:
Non-admins/owners should not see the Team OOO tab at all
Can we show past OOO in different tab called 'previous'?
As described here: [CAL-4527] OOO: Show your own history and give Admins CRUD and see all team members history #16333 (comment)

CarinaWolli · 2024-09-05T14:51:50Z

packages/features/settings/OutOfOfficeEntriesList.tsx

+  }
+  columns.push({
+    id: "outOfOffice",
+    header: `OutOfOffice (${totalDBRowCount})`,


Should be 'Out of office' with a translation

…6333_teamOOOCrud

vercel · 2024-12-10T15:38:01Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments

Name	Status	Preview	Comments	Updated (UTC)
cal	⬜️ Ignored (Inspect)	Visit Preview		Dec 11, 2024 1:20pm
calcom-web-canary	⬜️ Ignored (Inspect)	Visit Preview		Dec 11, 2024 1:20pm

vijayraghav-io · 2024-12-10T17:08:42Z

@vijayraghav-io unit tests are failing, please fix it 🙏

@anikdhabal , @Praashh resolved conflicts, updated tests due to changes from merges.
Also verified and tested again team OOO flow.

…6333_teamOOOCrud

anikdhabal · 2024-12-29T07:29:50Z

@vijayraghav-io great works man. But I am thinking of completely transferring all the E2E tests related to OOO into unit tests. Could you go through all the OOO E2E tests and add them to your current test file along with the other tests?

vijayraghav-io · 2024-12-29T14:28:11Z

@vijayraghav-io great works man. But I am thinking of completely transferring all the E2E tests related to OOO into unit tests. Could you go through all the OOO E2E tests and add them to your current test file along with the other tests?

Thank you! @anikdhabal .
IMO replacing E2E tests with unit tests may reduce the current scope of testing for OOO.
As these are End to End tests testing both UI and backend functionalities.
Also the flakiness for OOO tests is reduced very much now, observing the E2E results from latest PRs merged after #18367

anikdhabal · 2024-12-29T14:42:55Z

IMO replacing E2E tests with unit tests may reduce the current scope of testing for OOO.
As these are End to End tests testing both UI and backend functionalities.
Also the flakiness for OOO tests is reduced very much now, observing the E2E results from latest PRs merged after #18367

If this is the case, then why are we adding tests to the handler here? Can't we add e2e tests instead to verify both the UI and functionalities for this PR? I think it would make sense to create a single test file for OOO and add e2e tests for this. Let's make the change then

I've noticed that recent PRs failed due to OOO tests. Also, I have a query regarding that PR #18367, and we generally avoid any time dependency in e2e tests.

vijayraghav-io · 2024-12-29T16:22:28Z

I've noticed that recent PRs failed due to OOO tests. Also, I have a query regarding that PR #18367, and we generally avoid any time dependency in e2e tests.

Can you please recheck again, recent PRs don't have flakiness due to OOO tests.
Yeah E2E tests for Team OOO can be added.

vijayraghav-io · 2024-12-29T16:43:32Z

These are some of E2E results after OOO flakiness fix PR (#18367) was merged.

This is the latest after above PR is reverted (OOO flakiness reappeared).

vijayraghav-io · 2024-12-29T18:47:30Z

@anikdhabal , updated OOO E2E tests to remove flakiness without time dependency in this new PR #18412

…6333_teamOOOCrud

vijayraghav-io · 2025-01-16T08:22:37Z

@anikdhabal , resolved merge conflicts.

Also, I tried to create E2E tests for Team OOO instead of current unit tests, but currently there is some playwright limitations for this. So, I think currently we can go with unit tests for Team OOO and once we have the solution from playwright we can create E2E tests.
The limitation is : For Team OOO, the Create OOO Dialog is taller (because of member selection for For Member and To Member, and also have made it to be minimum possible) , hence playwright can't reach the create button and redirect member selection as playwright starts with default 100% browser zoom. It will be possible if chromium browser is zoomed out. Currently playwright does not support browser zoom out through its APIs. There are some issues already raised in playwright repo, refs - microsoft/playwright#2497 , microsoft/playwright#32352

PFB the Loom showing the issue with a team OOO E2E test, here i have manually zoomed out chromium browser while playwright is trying to reach down. (Here test is passing because of manul zoom out during test)

https://www.loom.com/share/10a0e9474d534f1b9a99b0564a8b4e98?sid=16eab3c0-6281-4de3-b0af-63443c17dc04

Hope, with this all concerns are addressed.

feature for team ooo crud and past records

e3ccfc6

graphite-app bot requested a review from a team September 2, 2024 07:37

graphite-app bot added the community Created by Linear-GitHub Sync label Sep 2, 2024

github-actions bot added enterprise area: enterprise, audit log, organisation, SAML, SSO Medium priority Created by Linear-GitHub Sync organizations area: organizations, orgs teams area: teams, round robin, collective, managed event-types ✨ feature New feature or request labels Sep 2, 2024

dosubot bot added the ui area: UI, frontend, button, form, input label Sep 2, 2024

dosubot bot added this to the v4.5 milestone Sep 2, 2024

anikdhabal reviewed Sep 2, 2024

View reviewed changes

Amit91848 reviewed Sep 2, 2024

View reviewed changes

anikdhabal reviewed Sep 2, 2024

View reviewed changes

Amit91848 suggested changes Sep 2, 2024

View reviewed changes

anikdhabal reviewed Sep 2, 2024

View reviewed changes

Amit91848 reviewed Sep 2, 2024

View reviewed changes

anikdhabal reviewed Sep 2, 2024

View reviewed changes

anikdhabal requested changes Sep 2, 2024

View reviewed changes

added tests for ooo handler, combined list components for mine,team

c4cc362

nit: removed commented code

010b8f9

CarinaWolli requested changes Sep 5, 2024

View reviewed changes

added current-previous tab , not to show TeamOOO tab for nonadmin

2d4e07b

vijayraghav-io added 4 commits December 10, 2024 15:50

updates after merge

1736b59

Merge branch 'main' of https://github.com/calcom/cal.com into feat_#1…

baaaa6f

…6333_teamOOOCrud

updates after merge due tp changes in datatable and others

8957676

Merge branch 'main' of https://github.com/calcom/cal.com into feat_#1…

47a109e

…6333_teamOOOCrud

vercel bot deployed to Preview – api December 10, 2024 15:46 View deployment

update import due to file splitting and new name

34abf88

vercel bot deployed to Preview – api December 10, 2024 16:23 View deployment

Merge branch 'main' of https://github.com/calcom/cal.com into feat_#1…

b06b8db

…6333_teamOOOCrud

vercel bot deployed to Preview – api December 11, 2024 06:47 View deployment

Merge branch 'main' of https://github.com/calcom/cal.com into feat_#1…

5973e78

…6333_teamOOOCrud

vercel bot deployed to Preview – api December 11, 2024 13:28 View deployment

keithwillcode modified the milestones: v4.8, v4.9 Dec 15, 2024

dosubot bot modified the milestone: v4.9 Dec 15, 2024

Merge branch 'main' into feat_#16333_teamOOOCrud

ce58fad

vijayraghav-io added 3 commits December 30, 2024 21:46

Merge branch 'main' of https://github.com/calcom/cal.com into feat_#1…

11b0821

…6333_teamOOOCrud

Merge branch 'main' of https://github.com/calcom/cal.com into feat_#1…

7a1186c

…6333_teamOOOCrud

Merge branch 'main' of https://github.com/calcom/cal.com into feat_#1…

5063c3b

…6333_teamOOOCrud

dosubot bot modified the milestones: v4.9, v4.10 Jan 16, 2025

		listMembers
		?.filter((member) => (oooType === "mine" ? me?.data?.id !== member.id : oooType === "team"))

feat: Manage Team ooo or crud and fetch past ooo records #16456

Are you sure you want to change the base?

feat: Manage Team ooo or crud and fetch past ooo records #16456

Conversation

vijayraghav-io commented Sep 2, 2024 • edited Loading

What does this PR do?

Mandatory Tasks (DO NOT REMOVE)

vercel bot commented Sep 2, 2024

graphite-app bot commented Sep 2, 2024 • edited Loading

Graphite Automations

anikdhabal Sep 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Amit91848 Sep 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Amit91848 commented Sep 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Amit91848 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anikdhabal left a comment

Choose a reason for hiding this comment

vijayraghav-io commented Sep 2, 2024 • edited Loading

vijayraghav-io commented Sep 4, 2024

CarinaWolli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Dec 10, 2024 • edited Loading

vijayraghav-io commented Dec 10, 2024 • edited Loading

anikdhabal commented Dec 29, 2024

vijayraghav-io commented Dec 29, 2024

anikdhabal commented Dec 29, 2024 • edited Loading

vijayraghav-io commented Dec 29, 2024

vijayraghav-io commented Dec 29, 2024

vijayraghav-io commented Dec 29, 2024 • edited Loading

vijayraghav-io commented Jan 16, 2025 • edited Loading

vijayraghav-io commented Sep 2, 2024 •

edited

Loading

graphite-app bot commented Sep 2, 2024 •

edited

Loading

anikdhabal Sep 2, 2024 •

edited

Loading

Amit91848 Sep 2, 2024 •

edited

Loading

vijayraghav-io commented Sep 2, 2024 •

edited

Loading

vercel bot commented Dec 10, 2024 •

edited

Loading

vijayraghav-io commented Dec 10, 2024 •

edited

Loading

anikdhabal commented Dec 29, 2024 •

edited

Loading

vijayraghav-io commented Dec 29, 2024 •

edited

Loading

vijayraghav-io commented Jan 16, 2025 •

edited

Loading