Skip to content
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(schedule): PNRN-49 create schedule test #14

Merged
merged 9 commits into from
Sep 5, 2022

Conversation

junix033101
Copy link
Contributor

@junix033101 junix033101 commented Sep 3, 2022

Description

Provide a description on what was changed/added/fixed in this PR

Checklist

First, create a draft pull request. Then mark the PR as ready for review when all necessary checkbox items has been completed

  • Reviewed own code
  • Commented on code that is hard to understand
  • Implemented tests for the feature/bugfix
  • All GitHub status checks pass
  • The frontend and backend PR previews have been deployed

Backend only

  • Added test data for new entity
  • Generated the client api if there are new or deleted endpoints and/or DTOs

@vercel
Copy link

vercel bot commented Sep 3, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
pnrn-pnrn ✅ Ready (Inspect) Visit Preview Sep 5, 2022 at 9:06AM (UTC)
1 Ignored Deployment
Name Status Preview Updated
pnrn-pnrn-staging ⬜️ Ignored (Inspect) Sep 5, 2022 at 9:06AM (UTC)

@princhcanal princhcanal temporarily deployed to pnrn-pnrn-pr-14 September 4, 2022 15:18 Inactive
@princhcanal princhcanal temporarily deployed to pnrn-pnrn-pr-14 September 5, 2022 05:38 Inactive
expect(endTime).toEqual(testCreateSchedule.endTime.toISOString());
});

it('should not create schedule when availability is UNAVAILABLE', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for this test, i think we need to rediscuss the implementation details about the availabilities.

Comment on lines 110 to 118
it('should update schedule', () => {
availableSchedule.startTime = new Date('2022-9-2 3:00:00 PM');
availableSchedule.endTime = new Date('2022-9-2 5:00:00 PM');

return requestWithStaff
.put(scheduleRoute + '/' + availableSchedule.id)
.send(availableSchedule)
.expect(HttpStatus.OK);
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ing ani ra ang PUT @junix033101. basically, same siya sa POST except mag edit raka sa existing data. so pwede raka mu gamit ug data gikan sa test-data parehas aning availableSchedule na ako gi gamit

Comment on lines +32 to +36
} catch (e) {
if (e instanceof NotFoundError) {
throw new NotFoundException();
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the reason a 500 error was thrown before was because prisma does not have the same error handling as nest so we have to explicitly throw a NotFoundException (NestJS error) when prisma throws a NotFoundError

@princhcanal princhcanal temporarily deployed to pnrn-pnrn-pr-14 September 5, 2022 09:05 Inactive
@junix033101 junix033101 marked this pull request as ready for review September 5, 2022 09:21
@junix033101 junix033101 requested a review from a team as a code owner September 5, 2022 09:22
Copy link
Member

@princhcanal princhcanal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great job! nice tests ✅ ✅ ✅

Comment on lines +49 to +53
const createStartTimeAfterEndTimeSched = {
availability: AvailabilityEnum.AVAILABLE,
startTime: new Date('2022-9-3 2:00:00 PM'),
endTime: new Date('2022-9-3 1:00:00 PM'),
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to change but we could put a type to this variable. so it would become

const createStartTimeAfterEndTimeSched: ScheduleDTO = ...

endTime: new Date('2022-9-2 5:00:00 PM'),
};

export const createSchedule = async (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

@junix033101 junix033101 merged commit 4cecd71 into dev Sep 5, 2022
@junix033101 junix033101 deleted the test/PNRN-49-add-schedule-test branch September 5, 2022 10:19
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