-
Notifications
You must be signed in to change notification settings - Fork 939
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
test: unit test for display services #1832
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
Thank you for following the naming conventions for pull request titles! 🙏 |
packages/lib/display/service.ts
Create Issue // In the Prisma schema file
status String? @default(null)
Create Issue // In the catch block of the getDisplay function
if (error instanceof Prisma.PrismaClientKnownRequestError) {
throw new DatabaseError(`Error fetching display: ${error.message}`);
}
Create Issue // In the createDisplay function
if (userId) {
person = await prisma.person.upsert({
where: { userId: userId },
update: {},
create: { userId: userId, environmentId: environmentId }
});
}
|
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.
@Dhruwang thanks a lot for the tests! 💪 They look great, but your updates to the display service aren't aligned with the goals of the services:
packages/lib/display/service.ts
Outdated
@@ -315,7 +316,7 @@ export const getDisplaysByPersonId = async (personId: string, page?: number): Pr | |||
revalidate: SERVICES_REVALIDATION_INTERVAL, | |||
} | |||
)(); | |||
return displays.map((display) => formatDateFields(display, ZDisplay)); | |||
return displays.map((display) => formatDateFields({ ...display, status: null }, ZDisplay)); |
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.
same as above
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.
@Dhruwang thank you for the changes :-) you missed two but I changed them now :-)
What does this PR do?
Fixes 1566
Type of change
How should this be tested?
run test
Checklist
Required
pnpm build
console.logs
git pull origin main
Appreciated