-
-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add missing dataloaders and optimize some #311
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
Changes from all commits
7aa2735
cb80ce4
76bddc5
58ae23a
42c83e6
1d1ec79
5b1372e
0e94aaa
fd14a90
2ba2630
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,10 @@ | ||
| import { SQL, eq, inArray } from "drizzle-orm"; | ||
| import { SQL, inArray } from "drizzle-orm"; | ||
|
|
||
| import { authHelpers } from "~/authz/helpers"; | ||
| import { builder } from "~/builder"; | ||
| import { | ||
| ScheduleStatus, | ||
| SelectTicketSchema, | ||
| selectCommunitySchema, | ||
| selectGalleriesSchema, | ||
| selectImagesSchema, | ||
|
|
@@ -16,7 +17,6 @@ import { | |
| selectUsersSchema, | ||
| ticketStatusEnum, | ||
| ticketVisibilityEnum, | ||
| ticketsSchema, | ||
| usersSchema, | ||
| } from "~/datasources/db/schema"; | ||
| import { lower } from "~/datasources/db/shared"; | ||
|
|
@@ -342,76 +342,91 @@ export const EventLoadable = builder.loadableObject(EventRef, { | |
| return tags.map((t) => selectTagsSchema.parse(t)); | ||
| }, | ||
| }), | ||
| tickets: t.field({ | ||
| tickets: t.loadableList({ | ||
| description: | ||
| "List of tickets for sale or redemption for this event. (If you are looking for a user's tickets, use the usersTickets field)", | ||
| type: [TicketRef], | ||
| type: TicketRef, | ||
| args: { | ||
| input: t.arg({ | ||
| type: EventsTicketTemplateSearchInput, | ||
| required: false, | ||
| }), | ||
| }, | ||
| resolve: async (root, { input }, { DB, USER }) => { | ||
| const wheres: SQL[] = []; | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Estos wheres no se utilizaban |
||
|
|
||
| wheres.push(eq(ticketsSchema.eventId, root.id)); | ||
|
|
||
| // If the user is an admin, they can see all tickets, otherwise, only | ||
| // active tickets are shown. | ||
| let statusCheck: (typeof ticketStatusEnum)[number][] = ["active"]; | ||
| let visibilityCheck: (typeof ticketVisibilityEnum)[number][] = [ | ||
| "public", | ||
| ]; | ||
|
|
||
| if (USER) { | ||
| if (USER.isSuperAdmin) { | ||
| statusCheck = ["active", "inactive"]; | ||
|
|
||
| visibilityCheck = ["public", "private", "unlisted"]; | ||
| } else { | ||
| const isAdmin = await authHelpers.isAdminOfEventCommunity({ | ||
| userId: USER.id, | ||
| eventId: root.id, | ||
| DB, | ||
| }); | ||
|
|
||
| if (isAdmin) { | ||
| byPath: true, | ||
| load: async (eventsIds: string[], context, args) => { | ||
| const { DB, USER } = context; | ||
| const { input } = args; | ||
|
|
||
| const idToTicketsMap = new Map< | ||
| string, | ||
| SelectTicketSchema[] | undefined | ||
| >(); | ||
|
|
||
| const ticketsPromises = eventsIds.map(async (eventId) => { | ||
| // If the user is an admin, they can see all tickets, otherwise, only | ||
| // active tickets are shown. | ||
| let statusCheck: (typeof ticketStatusEnum)[number][] = ["active"]; | ||
| let visibilityCheck: (typeof ticketVisibilityEnum)[number][] = [ | ||
| "public", | ||
| ]; | ||
|
|
||
| if (USER) { | ||
| if (USER.isSuperAdmin) { | ||
| statusCheck = ["active", "inactive"]; | ||
|
|
||
| visibilityCheck = ["public", "private", "unlisted"]; | ||
| } else { | ||
| const isAdmin = await authHelpers.isAdminOfEventCommunity({ | ||
| userId: USER.id, | ||
| eventId: eventId, | ||
| DB, | ||
| }); | ||
|
|
||
| if (isAdmin) { | ||
| statusCheck = ["active", "inactive"]; | ||
|
|
||
| visibilityCheck = ["public", "private", "unlisted"]; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const coupon = input?.coupon?.length | ||
| ? await DB.query.couponsSchema.findFirst({ | ||
| where: (c, { eq, and }) => | ||
| and( | ||
| eq(c.eventId, root.id), | ||
| eq(c.isActive, true), | ||
| eq(lower(c.code), input.coupon?.toLowerCase() ?? ""), | ||
| ), | ||
| columns: { | ||
| id: true, | ||
| }, | ||
| }) | ||
| : null; | ||
|
|
||
| const tickets = await ticketsFetcher.searchTickets({ | ||
| DB, | ||
| search: { | ||
| status: statusCheck, | ||
| visibility: visibilityCheck, | ||
| eventIds: [root.id], | ||
| tags: input?.tags ? input.tags : undefined, | ||
| couponId: coupon?.id, | ||
| }, | ||
| sort: [["createdAt", "asc"]], | ||
| const coupon = input?.coupon?.length | ||
| ? await DB.query.couponsSchema.findFirst({ | ||
| where: (c, { eq, and }) => | ||
| and( | ||
| eq(c.eventId, eventId), | ||
| eq(c.isActive, true), | ||
| eq(lower(c.code), input.coupon?.toLowerCase() ?? ""), | ||
| ), | ||
| columns: { | ||
| id: true, | ||
| }, | ||
| }) | ||
| : null; | ||
|
|
||
| const tickets = await ticketsFetcher.searchTickets({ | ||
| DB, | ||
| search: { | ||
| status: statusCheck, | ||
| visibility: visibilityCheck, | ||
| eventIds: [eventId], | ||
| tags: input?.tags ? input.tags : undefined, | ||
| couponId: coupon?.id, | ||
| }, | ||
| sort: [["createdAt", "asc"]], | ||
| }); | ||
|
|
||
| idToTicketsMap.set( | ||
| eventId, | ||
| tickets.map((t) => selectTicketSchema.parse(t)), | ||
| ); | ||
| }); | ||
|
|
||
| return tickets.map((t) => selectTicketSchema.parse(t)); | ||
| await Promise.all(ticketsPromises); | ||
|
|
||
| return eventsIds.map((id) => idToTicketsMap.get(id) || []); | ||
| }, | ||
| resolve: (root) => root.id, | ||
| }), | ||
| schedules: t.field({ | ||
| type: [ScheduleRef], | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,7 +59,7 @@ builder.mutationField("payForPurchaseOrder", (t) => | |
| default_redirect_url: PURCHASE_CALLBACK_URL, | ||
| purchaseOrderId, | ||
| }); | ||
| const { purchaseOrder, ticketsIds } = await handlePaymentLinkGeneration({ | ||
| const { purchaseOrder } = await handlePaymentLinkGeneration({ | ||
| DB, | ||
| USER, | ||
| purchaseOrderId, | ||
|
|
@@ -72,10 +72,7 @@ builder.mutationField("payForPurchaseOrder", (t) => | |
| }); | ||
|
|
||
| // 4. We return the payment link. | ||
| return { | ||
| purchaseOrder, | ||
| ticketsIds, | ||
| }; | ||
| return purchaseOrder; | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No se usan realmente los ticketsIds en el resolver. |
||
| }, | ||
| }), | ||
| ); | ||
|
|
@@ -141,23 +138,11 @@ builder.mutationField("checkPurchaseOrderStatus", (t) => | |
| transactionalEmailService: RPC_SERVICE_EMAIL, | ||
| }); | ||
|
|
||
| const tickets = await DB.query.userTicketsSchema.findMany({ | ||
| where: (po, { eq }) => eq(po.purchaseOrderId, purchaseOrderId), | ||
| columns: { | ||
| id: true, | ||
| }, | ||
| }); | ||
|
|
||
| if (!purchaseOrder) { | ||
| throw new Error("Purchase order not found"); | ||
| } | ||
|
|
||
| const ticketsIds = tickets.map((t) => t.id); | ||
|
|
||
| return { | ||
| purchaseOrder: selectPurchaseOrdersSchema.parse(purchaseOrder), | ||
| ticketsIds, | ||
| }; | ||
| return selectPurchaseOrdersSchema.parse(purchaseOrder); | ||
| }, | ||
| }), | ||
| ); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Por qué pasar de
resolveaload?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
En la linea 345, se pasó de
t.fieldat.loadableList.Este cambio fue necesario para implementar el método
load, que aprovecha el patrón de DataLoader, agrupando múltiples solicitudes en una sola operación a la DB para optimizar el rendimiento.Ahora,
resolvese encarga de devolver los IDs de los eventos, que posteriormente se procesan enload(usandoeventIds).Este patrón es ideal cuando se devuelven múltiples eventos a una consulta, ya que evita el problema de N+1 reduciendo la cantidad de operaciones individuales ejecutadas en la DB.
Ejemplo de uso típico de la query:
Lo que haría el nuevo código:
loadableListrecibe todos los IDs de eventos.Evitar el N+1 en este resolver implicaría más cambios en la lógica y no un simple Promise.all.
Lo ideal sería 1 sola query a la BD que permita traer todos los tickets relacionados a los eventsIds en cuestión pero no quise meterme ahí aún.
El cambio actual apunta a preparar el camino para una futura revisión de esta query, mantiene el PR pequeño y permite centrarse también en los otros DataLoaders que se incluyen en el PR.
Más info: