-
Notifications
You must be signed in to change notification settings - Fork 2
feat(perf): removed project queries from AllWorkspacesWithProjects #574
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
base: master
Are you sure you want to change the base?
Changes from all commits
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 | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -41,13 +41,18 @@ module.exports = { | |||||||||||||||||||||
| * @param factories - factories for working with models | ||||||||||||||||||||||
| * @return {Promise<UserModel[]> | null} | ||||||||||||||||||||||
| */ | ||||||||||||||||||||||
| async visitedBy({ visitedBy, projectId }, _args, { factories, user }) { | ||||||||||||||||||||||
| async visitedBy({ visitedBy, projectId, workspaceId }, _args, { factories, user }) { | ||||||||||||||||||||||
| /** | ||||||||||||||||||||||
| * Crutch for Demo Workspace | ||||||||||||||||||||||
| * Prefer workspaceId from parent if present to avoid extra project lookup | ||||||||||||||||||||||
| */ | ||||||||||||||||||||||
| const project = await factories.projectsFactory.findById(projectId); | ||||||||||||||||||||||
| const resolvedWorkspaceId = workspaceId || (await (async () => { | ||||||||||||||||||||||
| const project = await factories.projectsFactory.findById(projectId); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return project ? project.workspaceId.toString() : undefined; | ||||||||||||||||||||||
| })()); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
Comment on lines
+49
to
54
|
||||||||||||||||||||||
| const resolvedWorkspaceId = workspaceId || (await (async () => { | |
| const project = await factories.projectsFactory.findById(projectId); | |
| return project ? project.workspaceId.toString() : undefined; | |
| })()); | |
| let resolvedWorkspaceId = workspaceId; | |
| if (!resolvedWorkspaceId) { | |
| const project = await factories.projectsFactory.findById(projectId); | |
| resolvedWorkspaceId = project ? project.workspaceId.toString() : undefined; | |
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -460,6 +460,20 @@ module.exports = { | |||||
|
|
||||||
| const dailyEventsPortion = await factory.findDailyEventsPortion(limit, nextCursor, sort, filters, search); | ||||||
|
|
||||||
| /** | ||||||
| * Pass workspaceId down to events so nested resolvers (like Event.visitedBy) | ||||||
| * can avoid extra project lookups. | ||||||
| */ | ||||||
| if (dailyEventsPortion && Array.isArray(dailyEventsPortion.dailyEvents)) { | ||||||
| dailyEventsPortion.dailyEvents = dailyEventsPortion.dailyEvents.map((item) => ({ | ||||||
| ...item, | ||||||
| event: { | ||||||
| ...item.event, | ||||||
| workspaceId: project.workspaceId && project.workspaceId.toString ? project.workspaceId.toString() : project.workspaceId, | ||||||
|
||||||
| workspaceId: project.workspaceId && project.workspaceId.toString ? project.workspaceId.toString() : project.workspaceId, | |
| workspaceId: project.workspaceId?.toString() ?? project.workspaceId, |
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.
The null check only verifies that
projectexists, but doesn't verify thatproject.workspaceIdexists before callingtoString(). Ifproject.workspaceIdis null or undefined, this will throw a TypeError.Consider using optional chaining: