From 39816e9bb49e83fd7ea134243f59c7500ee36bc1 Mon Sep 17 00:00:00 2001 From: Philippe Rolet Date: Fri, 2 Feb 2024 14:47:57 +0100 Subject: [PATCH] Refactor getMembers / recent authors (#3560) * add userIds param to getMembers * adapt recent_authors to not fetch all members * review --- front/lib/api/assistant/recent_authors.ts | 36 +++++++++---------- front/lib/api/data_sources.ts | 4 ++- front/lib/api/workspace.ts | 21 +++++++++-- front/pages/api/stripe/webhook.ts | 8 +++-- .../assistant/agent_configurations/index.ts | 14 +++----- 5 files changed, 47 insertions(+), 36 deletions(-) diff --git a/front/lib/api/assistant/recent_authors.ts b/front/lib/api/assistant/recent_authors.ts index e9c94c72d421..1edcd420d4ce 100644 --- a/front/lib/api/assistant/recent_authors.ts +++ b/front/lib/api/assistant/recent_authors.ts @@ -5,6 +5,7 @@ import type { } from "@dust-tt/types"; import { Sequelize } from "sequelize"; +import { getMembers } from "@app/lib/api/workspace"; import type { Authenticator } from "@app/lib/auth"; import { AgentConfiguration } from "@app/lib/models"; import { safeRedisClient } from "@app/lib/redis"; @@ -105,35 +106,30 @@ async function populateAuthorIdsFromDb({ } function renderAuthors( - authorIds: readonly string[], - members: UserType[], + authors: UserType[], currentUserId?: number ): readonly string[] { return ( - authorIds - .map((id) => parseInt(id, 10)) - .map((authorId) => { + authors + .map((author) => { // If authorId is the current requester, return "Me". - if (authorId === currentUserId) { + if (author.id === currentUserId) { return "Me"; } - return members.find((m) => m.id === authorId)?.fullName ?? null; + return author.fullName; }) // Filter out `null` authors. .filter((name): name is string => name !== null) ); } -export async function getAgentRecentAuthors( - { - agent, - auth, - }: { - agent: LightAgentConfigurationType; - auth: Authenticator; - }, - members: UserType[] -): Promise { +export async function getAgentRecentAuthors({ + agent, + auth, +}: { + agent: LightAgentConfigurationType; + auth: Authenticator; +}): Promise { const { sId: agentId, versionAuthorId } = agent; const owner = auth.workspace(); @@ -162,9 +158,11 @@ export async function getAgentRecentAuthors( // Populate from the database and store in Redis if the entry is not already present. recentAuthorIds = await populateAuthorIdsFromDb({ agentId, workspaceId }); } - + const authors = await getMembers(auth, { + userIds: recentAuthorIds.map((id) => parseInt(id, 10)), + }); // Consider moving this logic to the FE if we need to fetch members in different places. - return renderAuthors(recentAuthorIds, members, currentUserId); + return renderAuthors(authors, currentUserId); } export async function agentConfigurationWasUpdatedBy({ diff --git a/front/lib/api/data_sources.ts b/front/lib/api/data_sources.ts index ac87135d18a2..4459eff1d8ea 100644 --- a/front/lib/api/data_sources.ts +++ b/front/lib/api/data_sources.ts @@ -251,7 +251,9 @@ async function warnPostDeletion( switch (dataSourceProvider) { case "github": // get admin emails - const adminEmails = (await getMembers(auth, "admin")).map((u) => u.email); + const adminEmails = (await getMembers(auth, { role: "admin" })).map( + (u) => u.email + ); // send email to admins for (const email of adminEmails) await sendGithubDeletionEmail(email); break; diff --git a/front/lib/api/workspace.ts b/front/lib/api/workspace.ts index 45b25648d85a..abfb75b55ee6 100644 --- a/front/lib/api/workspace.ts +++ b/front/lib/api/workspace.ts @@ -106,15 +106,30 @@ export async function setInternalWorkspaceSegmentation( */ export async function getMembers( auth: Authenticator, - role?: RoleType + { + role, + userIds, + }: { + role?: RoleType; + userIds?: ModelId[]; + } = {} ): Promise { const owner = auth.workspace(); if (!owner) { return []; } - const whereClause = role - ? { workspaceId: owner.id, role } + + const whereClause: { + workspaceId: ModelId; + userId?: ModelId[]; + role?: RoleType; + } = userIds + ? { workspaceId: owner.id, userId: userIds } : { workspaceId: owner.id }; + if (role) { + whereClause.role = role; + } + const memberships = await Membership.findAll({ where: whereClause, }); diff --git a/front/pages/api/stripe/webhook.ts b/front/pages/api/stripe/webhook.ts index bca3fad340a9..8a9eadabe9d1 100644 --- a/front/pages/api/stripe/webhook.ts +++ b/front/pages/api/stripe/webhook.ts @@ -401,7 +401,7 @@ async function handler( "Couldn't get owner or subscription from `auth`." ); } - const adminEmails = (await getMembers(auth, "admin")).map( + const adminEmails = (await getMembers(auth, { role: "admin" })).map( (u) => u.email ); const customerEmail = invoice.customer_email; @@ -464,7 +464,7 @@ async function handler( ); // then email admins - const adminEmails = (await getMembers(auth, "admin")).map( + const adminEmails = (await getMembers(auth, { role: "admin" })).map( (u) => u.email ); if (adminEmails.length === 0) { @@ -675,7 +675,9 @@ async function checkStaticDatasourcesSize(auth: Authenticator) { } await sendOpsDowngradeTooMuchDataEmail(workspace.sId, datasourcesTooBig); // for all admins - const adminEmails = (await getMembers(auth, "admin")).map((u) => u.email); + const adminEmails = (await getMembers(auth, { role: "admin" })).map( + (u) => u.email + ); for (const adminEmail of adminEmails) await sendAdminDowngradeTooMuchDataEmail(adminEmail, datasourcesTooBig); } diff --git a/front/pages/api/w/[wId]/assistant/agent_configurations/index.ts b/front/pages/api/w/[wId]/assistant/agent_configurations/index.ts index c1a339a3ef1a..e7abbf34f962 100644 --- a/front/pages/api/w/[wId]/assistant/agent_configurations/index.ts +++ b/front/pages/api/w/[wId]/assistant/agent_configurations/index.ts @@ -23,7 +23,6 @@ import { getAgentConfigurations, } from "@app/lib/api/assistant/configuration"; import { getAgentRecentAuthors } from "@app/lib/api/assistant/recent_authors"; -import { getMembers } from "@app/lib/api/workspace"; import { Authenticator, getSession } from "@app/lib/auth"; import { safeRedisClient } from "@app/lib/redis"; import { apiError, withLogging } from "@app/logger/withlogging"; @@ -130,8 +129,6 @@ async function handler( } if (withAuthors === "true") { - const members = await getMembers(auth); - agentConfigurations = await Promise.all( agentConfigurations.map( async ( @@ -139,13 +136,10 @@ async function handler( ): Promise => { return { ...agentConfiguration, - lastAuthors: await getAgentRecentAuthors( - { - agent: agentConfiguration, - auth, - }, - members - ), + lastAuthors: await getAgentRecentAuthors({ + agent: agentConfiguration, + auth, + }), }; } )