Skip to content

perf: refactor and replacing viewer.teams.getMinimal with viewer.teams.get#16552

Merged
zomars merged 21 commits intomainfrom
refactor/use-get-teams
Oct 10, 2024
Merged

perf: refactor and replacing viewer.teams.getMinimal with viewer.teams.get#16552
zomars merged 21 commits intomainfrom
refactor/use-get-teams

Conversation

@Udit-takkar
Copy link
Copy Markdown
Contributor

@Udit-takkar Udit-takkar commented Sep 9, 2024

What does this PR do?

  • Fixes #XXXX (GitHub issue number)

  • Fixes CAL-XXXX (Linear issue number - should be visible at the bottom of the GitHub issue description)

  • Removing teams.getMinimal and teams.lazyLoadMembers introduced in this PR refactor: improve team members page performance #16155 .

  • Refactoring listMembers endpoint to support fetching all the members

  • Also Optimized OOO modal and Filter Container

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.
  • 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?

  • Go to /teams/[id]/members page
  • Go to /settings/my-account/out-of-office
  • Test People filters in /bookings/upcoming

@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Sep 9, 2024
@vercel
Copy link
Copy Markdown

vercel Bot commented Sep 10, 2024

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 Oct 10, 2024 2:58pm
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Oct 10, 2024 2:58pm

@Udit-takkar Udit-takkar force-pushed the refactor/use-get-teams branch from 665c9ad to dd6f575 Compare September 13, 2024 18:50
@keithwillcode keithwillcode added this to the v4.6 milestone Sep 15, 2024
@Udit-takkar Udit-takkar changed the title refactor: replace viewer.teams.getMinimal with viewer.teams.get perf: refactor and replacing viewer.teams.getMinimal with viewer.teams.get Sep 17, 2024
@Udit-takkar Udit-takkar marked this pull request as ready for review September 17, 2024 09:06
@graphite-app graphite-app Bot requested review from a team September 17, 2024 09:06
input: TListMembersInputSchema;
};

export const legacyListMembers = async ({ ctx, input }: ListMembersOptions) => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

legacyListMembers is the original listMembers function

const [searchText, setSearchText] = useState("");

const members = trpc.viewer.teams.listMembers.useQuery({});
const members = trpc.viewer.teams.legacyListMembers.useQuery({});
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I still need legacyListMembers because we need unique members from all the teams making sure a members of multiple team doesn't show up more than once but cursor and distinct property wasn't working properly


  const teamMembers = await prisma.membership.findMany({
    where: whereCondition,
    select: {
      id: true,
      role: true,
      accepted: true,
      teamId: true,
      user: { select: userSelect },
    },
    cursor: cursor ? { id: cursor } : undefined,
    take: limit + 1,
    orderBy: { id: "asc" },
    distinct: ["userId"],
  });

Copy link
Copy Markdown
Contributor Author

@Udit-takkar Udit-takkar Oct 10, 2024

Choose a reason for hiding this comment

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

TODO: remove legacyListMembers here and add a SQL query in a follow up PR

@Udit-takkar Udit-takkar requested a review from emrysal October 10, 2024 16:25
Copy link
Copy Markdown
Contributor

@zomars zomars left a comment

Choose a reason for hiding this comment

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

Awesome work @Udit-takkar LGTM 🙏🏽

@zomars zomars merged commit 50f6aa5 into main Oct 10, 2024
@zomars zomars deleted the refactor/use-get-teams branch October 10, 2024 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO performance area: performance, page load, slow, slow endpoints, loading screen, unresponsive ready-for-e2e 💻 refactor teams area: teams, round robin, collective, managed event-types

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants