-
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?
Conversation
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.
Pull request overview
This PR implements a performance optimization to reduce unnecessary database queries by passing workspaceId from parent resolvers to nested resolvers, avoiding extra project lookups when checking for demo workspaces.
- Propagates
workspaceIdfrom thedailyEventsPortionresolver inproject.jsto event objects - Updates the
visitedByresolver inevent.jsto prefer the passedworkspaceIdover fetching it from the database - Version bump from 1.2.15 to 1.2.16
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/resolvers/project.js | Adds workspaceId to event objects in dailyEventsPortion to pass context to nested resolvers |
| src/resolvers/event.js | Updates visitedBy resolver to use passed workspaceId parameter, falling back to project lookup only when needed |
| package.json | Version bump to 1.2.16 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ...item, | ||
| event: { | ||
| ...item.event, | ||
| workspaceId: project.workspaceId && project.workspaceId.toString ? project.workspaceId.toString() : project.workspaceId, |
Copilot
AI
Dec 7, 2025
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 condition project.workspaceId.toString checks for the existence of the toString method but doesn't check if project.workspaceId itself is null or undefined. This could throw a TypeError if project.workspaceId is null/undefined.
Consider changing to:
workspaceId: project.workspaceId?.toString() ?? project.workspaceIdor
workspaceId: project.workspaceId ? project.workspaceId.toString() : project.workspaceId| workspaceId: project.workspaceId && project.workspaceId.toString ? project.workspaceId.toString() : project.workspaceId, | |
| workspaceId: project.workspaceId?.toString() ?? project.workspaceId, |
| const resolvedWorkspaceId = workspaceId || (await (async () => { | ||
| const project = await factories.projectsFactory.findById(projectId); | ||
|
|
||
| return project ? project.workspaceId.toString() : undefined; | ||
| })()); | ||
|
|
Copilot
AI
Dec 7, 2025
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 immediately invoked async function expression (IIFE) is unnecessarily complex. Consider simplifying this to improve code readability:
let resolvedWorkspaceId = workspaceId;
if (!resolvedWorkspaceId) {
const project = await factories.projectsFactory.findById(projectId);
resolvedWorkspaceId = project?.workspaceId.toString();
}| 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; | |
| } |
| const resolvedWorkspaceId = workspaceId || (await (async () => { | ||
| const project = await factories.projectsFactory.findById(projectId); | ||
|
|
||
| return project ? project.workspaceId.toString() : undefined; |
Copilot
AI
Dec 7, 2025
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 project exists, but doesn't verify that project.workspaceId exists before calling toString(). If project.workspaceId is null or undefined, this will throw a TypeError.
Consider using optional chaining:
return project?.workspaceId?.toString();| return project ? project.workspaceId.toString() : undefined; | |
| return project?.workspaceId?.toString(); |
do not find project for checking is workspace is demo