Skip to content

Conversation

@slaveeks
Copy link
Member

@slaveeks slaveeks commented Nov 5, 2025

do not find project for checking is workspace is demo

Copy link
Contributor

Copilot AI left a 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 workspaceId from the dailyEventsPortion resolver in project.js to event objects
  • Updates the visitedBy resolver in event.js to prefer the passed workspaceId over 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,
Copy link

Copilot AI Dec 7, 2025

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

or

workspaceId: project.workspaceId ? project.workspaceId.toString() : project.workspaceId
Suggested change
workspaceId: project.workspaceId && project.workspaceId.toString ? project.workspaceId.toString() : project.workspaceId,
workspaceId: project.workspaceId?.toString() ?? project.workspaceId,

Copilot uses AI. Check for mistakes.
Comment on lines +49 to 54
const resolvedWorkspaceId = workspaceId || (await (async () => {
const project = await factories.projectsFactory.findById(projectId);

return project ? project.workspaceId.toString() : undefined;
})());

Copy link

Copilot AI Dec 7, 2025

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();
}
Suggested change
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;
}

Copilot uses AI. Check for mistakes.
const resolvedWorkspaceId = workspaceId || (await (async () => {
const project = await factories.projectsFactory.findById(projectId);

return project ? project.workspaceId.toString() : undefined;
Copy link

Copilot AI Dec 7, 2025

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();
Suggested change
return project ? project.workspaceId.toString() : undefined;
return project?.workspaceId?.toString();

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants