added(server/api): availability slots "find" and "set" api handlers. #278
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mmoehabb
force-pushed
the
moehab/api-availability-slots
branch
from
d26c6df to
949ebfc
Compare
mmoehabb
force-pushed
the
moehab/api-availability-slots
branch
3 times, most recently
from
222472c to
1d5aa0a
Compare
mmoehabb
force-pushed
the
moehab/api-availability-slots
branch
from
1d5aa0a to
e19fdab
Compare
|
@mmoehabb please rebase and resolve the conflicts. |
mmoehabb
force-pushed
the
moehab/api-availability-slots
branch
from
e19fdab to
7ec0498
Compare
mmoehabb
force-pushed
the
moehab/api-availability-slots
branch
from
7ec0498 to
447a967
Compare
mmoehabb
force-pushed
the
moehab/api-availability-slots
branch
from
447a967 to
f23361f
Compare
mmoehabb
force-pushed
the
moehab/api-availability-slots
branch
from
f23361f to
1560efe
Compare
|
@neuodev |
|
@mmoehabb please rebase and resolve the conflicts. |
|
You are missing the |
neuodev
requested changes
Jan 12, 2025
mmoehabb
force-pushed
the
moehab/api-availability-slots
branch
2 times, most recently
from
edee3ad to
a4c9e4b
Compare
mmoehabb
force-pushed
the
moehab/api-availability-slots
branch
from
a4c9e4b to
1861159
Compare
mmoehabb
force-pushed
the
moehab/api-availability-slots
branch
from
1861159 to
1906f49
Compare
neuodev
reviewed
Jan 14, 2025
packages/models/src/lessons.ts
Outdated
| @@ -151,6 +157,24 @@ export class Lessons { | |||
| .where(this.columns.lessons("id"), id); | |||
| } | |||
|
|
|||
| async cancelBatch({ | |||
There was a problem hiding this comment.
In this case the old one should be updated to accept a list of id not just one @mmoehabb
| import { dayjs } from "@/dayjs"; | ||
| import { flatten, orderBy } from "lodash"; | ||
| import { INTERVIEW_DURATION } from "./constants"; |
There was a problem hiding this comment.
Suggested change
| import { INTERVIEW_DURATION } from "./constants"; | |
| import { INTERVIEW_DURATION } from "@/constants"; |
neuodev
reviewed
Jan 14, 2025
services/server/fixtures/db.ts
Outdated
Comment on lines
114
to
122
| return ( | ||
| await availabilitySlots.create([ | ||
| { | ||
| userId: payload?.userId || 1, | ||
| start: start.toISOString(), | ||
| end: end.toISOString(), | ||
| }, | ||
| ]) | ||
| )[0]; |
There was a problem hiding this comment.
This is not clean and prone to errors. Please refactor it.
neuodev
reviewed
Jan 14, 2025
Comment on lines
59
to
87
| const canUseFullFlag = | ||
| after && before && dayjs.utc(before).diff(after, "days") <= 30; | ||
|
|
||
| const paginatedSlots = await availabilitySlots.find({ | ||
| users: [userId], | ||
| after, | ||
| before, | ||
| pagination: { | ||
| page: pagination?.page || 1, | ||
| size: pagination?.size || 10, | ||
| full: pagination?.full || !!canUseFullFlag, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Suggested change
| const canUseFullFlag = | |
| after && before && dayjs.utc(before).diff(after, "days") <= 30; | |
| const paginatedSlots = await availabilitySlots.find({ | |
| users: [userId], | |
| after, | |
| before, | |
| pagination: { | |
| page: pagination?.page || 1, | |
| size: pagination?.size || 10, | |
| full: pagination?.full || !!canUseFullFlag, | |
| }, | |
| }); | |
| const canUseFullFlag = | |
| after && before && dayjs.utc(before).diff(after, "days") <= 30; | |
| if (!canUseFullFlag) reutrn next(bad); | |
| const paginatedSlots = await availabilitySlots.find({ | |
| users: [userId], | |
| after, | |
| before, | |
| pagination, | |
| }); |
Comment on lines
97
to
107
| const result: IAvailabilitySlot.FindAvailabilitySlotsApiResponse = { | ||
| list: [ | ||
| { | ||
| slots: paginatedSlots.list, | ||
| subslots: asSubSlots([ | ||
| ...paginatedLessons.list, | ||
| ...userInterviews.list, | ||
| ]), | ||
| }, | ||
| ], | ||
| total: paginatedSlots.total, |
There was a problem hiding this comment.
The response sturcture is not a little bit misleading. The location of the paginated data is not right. I believe a better response sturcutre should look like this:
type Response = {
slots: Paginated<IAvailabilitySlot.Self[]>,
subslots: IAvailabilitySlot.SubSlot[],
}The slots are pagianted that's why it has the Paginated type. Subslot isn't.
Comment on lines
74
to
95
| if (slotsOnly && (isTutor(user.role) || isTutorManager(user.role))) { | ||
| const result = { | ||
| list: [{ slots: paginatedSlots.list, subslots: [] }], | ||
| total: paginatedSlots.total, | ||
| }; | ||
| res.status(200).json(result); | ||
| return; | ||
| } | ||
|
|
||
| const slotIds = paginatedSlots.list.map((slot) => slot.id); | ||
|
|
||
| const paginatedLessons = await lessons.find({ | ||
| users: [userId], | ||
| slots: slotIds, | ||
| after, | ||
| before, | ||
| }); | ||
|
|
||
| const userInterviews = await interviews.find({ | ||
| users: [userId], | ||
| slots: slotIds, | ||
| }); |
There was a problem hiding this comment.
We can move this logic into a seperate function. It will be more cleaner. Here is how the final code should look like.
function getSubslot(payload: {slotIds: number[], userId: number}): Promise<IAvailabilitySlot.SubSlot[]>;
const subslots = isTutor(user) || isTutorManager(user) ? [] : getSubSlot(slotIds, user.id);This way we will only have one res.json() and the logic will relatively simple to grasb what this handler is doing from a first look.
| }: { | ||
| currentUserId: number; | ||
| ids: number[]; | ||
| tx?: Knex.Transaction; |
There was a problem hiding this comment.
Let's make the tx required to force the consumer to pass it.
Suggested change
| tx?: Knex.Transaction; | |
| tx: Knex.Transaction; |
Comment on lines
22
to
32
| await lessons.cancelBatch({ | ||
| ids: associatedLessons.list.map((l) => l.id), | ||
| canceledBy: currentUserId, | ||
| tx, | ||
| }); | ||
| await interviews.cancel({ | ||
| ids: associatedInterviews.list.map((i) => i.ids.self), | ||
| canceledBy: currentUserId, | ||
| tx, | ||
| }); | ||
| await availabilitySlots.markAsDeleted({ ids: ids, tx }); |
There was a problem hiding this comment.
- Empty line for readability.
- Use
Promise.all.
Suggested change
| await lessons.cancelBatch({ | |
| ids: associatedLessons.list.map((l) => l.id), | |
| canceledBy: currentUserId, | |
| tx, | |
| }); | |
| await interviews.cancel({ | |
| ids: associatedInterviews.list.map((i) => i.ids.self), | |
| canceledBy: currentUserId, | |
| tx, | |
| }); | |
| await availabilitySlots.markAsDeleted({ ids: ids, tx }); | |
| await lessons.cancelBatch({ | |
| ids: associatedLessons.list.map((l) => l.id), | |
| canceledBy: currentUserId, | |
| tx, | |
| }); | |
| await interviews.cancel({ | |
| ids: associatedInterviews.list.map((i) => i.ids.self), | |
| canceledBy: currentUserId, | |
| tx, | |
| }); | |
| await availabilitySlots.markAsDeleted({ ids: ids, tx }); |
neuodev
reviewed
Jan 14, 2025
| const user = req.user; | ||
| if (!isUser(user)) return next(forbidden()); | ||
|
|
||
| const { userId, after, before, slotsOnly, pagination } = findPayload.parse( |
There was a problem hiding this comment.
As for the time being, it is not required to control the toggle of subslots on and off for outside. It might change in the future but not now. In this case we can remove this flag.
Suggested change
| const { userId, after, before, slotsOnly, pagination } = findPayload.parse( | |
| const { userId, after, before, pagination } = findPayload.parse( |
mmoehabb
force-pushed
the
moehab/api-availability-slots
branch
2 times, most recently
from
6987738 to
425de44
Compare
neuodev
approved these changes
Jan 15, 2025
neuodev
force-pushed
the
moehab/api-availability-slots
branch
from
e607fc2 to
f7d8990
Compare
neuodev
force-pushed
the
moehab/api-availability-slots
branch
from
f7d8990 to
620c5fa
Compare
neuodev
force-pushed
the
moehab/api-availability-slots
branch
from
620c5fa to
8d1ebd7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ClickUp Task: https://app.clickup.com/t/86978ub4n