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

feat: virtual queues tab in insights #18260

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

CarinaWolli
Copy link
Member

What does this PR do?

This is a temporary solution to display virtual queues within insights for all members. In the future, we will automatically list the queues a user is part of. For now, we are reusing the implementation from Routing form testing. Instead of showing all the detailed routing data, we are only displaying the matching hosts along with the Round Robin data.
Screenshot 2024-12-18 at 7 05 42 PM

https://www.loom.com/share/23d6e794218f4388849f87c6506dbd75

@keithwillcode keithwillcode added consumer core area: core, team members only labels Dec 19, 2024
Copy link

vercel bot commented Dec 19, 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 Dec 19, 2024 7:45pm
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Dec 19, 2024 7:45pm

@CarinaWolli CarinaWolli marked this pull request as ready for review December 19, 2024 00:54
@graphite-app graphite-app bot requested a review from a team December 19, 2024 00:54
@dosubot dosubot bot added insights area: insights, analytics ✨ feature New feature or request labels Dec 19, 2024
Copy link

graphite-app bot commented Dec 19, 2024

Graphite Automations

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

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

export default function InsightsVirtualQueuesPage() {
const { t } = useLocale();
const { data: routingForms, isLoading: isRoutingFormsLoading } =
trpc.viewer.insights.getUserRelevantTeamRoutingForms.useQuery();
Copy link
Member Author

Choose a reason for hiding this comment

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

This only returns routing forms of teams the user is a member of + that have at least one route that redirects to a weighted round robin event type where the user is a host of

@CarinaWolli CarinaWolli added the High priority Created by Linear-GitHub Sync label Dec 19, 2024
@CarinaWolli CarinaWolli added this to the v4.9 milestone Dec 19, 2024

for (const route of routes) {
if ("action" in route) {
const eventType = await prisma.eventType.findFirst({
Copy link
Contributor

Choose a reason for hiding this comment

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

We should refactor this to get this data in bulk. We shouldn’t have prisma calls inside of nested loops.

Copy link
Member

Choose a reason for hiding this comment

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

Hey @CarinaWolli i pushed 722d4c7 to use rawSQL instead of this nasty nested loop. I've tested it and it works as expected, if you have a better approach please just revert this commit!

)
)
SELECT DISTINCT f.*,
jsonb_build_object(
Copy link
Member

Choose a reason for hiding this comment

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

We could have just not build a json object here and returned it as normal fields then loop over and map as we expect. But this was easier and saves us iterating over the data for the second time

SELECT DISTINCT f.*,
jsonb_build_object(
'parentId', t."parentId",
'parent', CASE WHEN p.id IS NOT NULL THEN jsonb_build_object('slug', p.slug) ELSE NULL END,
Copy link
Member

Choose a reason for hiding this comment

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

Set parent.slug if we have a parent if not just set the parent field to null

) as "user"
FROM "App_RoutingForms_Form" f
INNER JOIN json_array_elements_recursive r ON f.id = r.id
INNER JOIN "EventType" e ON (r.route->>'action')::jsonb->>'eventTypeId' = e.id::text
Copy link
Member

Choose a reason for hiding this comment

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

Bonus of PGSQL we can easily query nested json without needing to do any JS magic

Comment on lines +21 to +30
WITH RECURSIVE json_array_elements_recursive AS (
SELECT f.id, f."teamId",
jsonb_array_elements(f.routes::jsonb) as route
FROM "App_RoutingForms_Form" f
WHERE f."teamId" IN (
SELECT "teamId"
FROM "Membership"
WHERE "userId" = ${userId}
)
)
Copy link
Member

Choose a reason for hiding this comment

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

Recursive is always pretty scary but TLDR: We get all routing forms -> filter all forms to ones we are a member of -> expand routes into a single row so we can query it below using sql.

Example:

{
  "id": 1,
  "teamId": 123,
  "routes": [
    { "action": { "eventTypeId": 456 } },
    { "action": { "eventTypeId": 789 } }
  ]
}

Output:

id | teamId | route
1  | 123    | { "action": { "eventTypeId": 456 } }
1  | 123    | { "action": { "eventTypeId": 789 } }

Allows us to join later in the query on event type to get an output as follows

form.id | route.eventTypeId | eventType.id | eventType.schedulingType
form1   | 123              | 123          | roundRobin
form1   | 456              | 456          | collective

},
},
});
type FormWithRelations = App_RoutingForms_Form & {
Copy link
Member

Choose a reason for hiding this comment

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

This is the same response type we were getting from the prisma query

changed "virtual queue" to "Router Position" in nav
Copy link
Member

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

Impressive! @CarinaWolli @sean-brydon LGTM

@zomars
Copy link
Member

zomars commented Dec 19, 2024

Seems like we're having API V2 failures. Any insights here? @ThyMinimalDev @hariombalhara

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consumer core area: core, team members only ✨ feature New feature or request High priority Created by Linear-GitHub Sync insights area: insights, analytics ready-for-e2e
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants