-
Notifications
You must be signed in to change notification settings - Fork 0
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
Session schedule schema #150
Conversation
@@ -15,6 +16,7 @@ defmodule Dbservice.Batches.Batch do | |||
|
|||
has_many :group_type, GroupType, foreign_key: :child_id, where: [type: "batch"] | |||
many_to_many :program, Program, join_through: "batch_program", on_replace: :delete | |||
has_one :session_schedule, SessionSchedule |
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.
This looks so nice :) Batch has one session schedule. Makes sense in English also :)
iex> get_session_schedule!(456) | ||
** (Ecto.NoResultsError) | ||
""" | ||
def get_session_schedule!(id), do: Repo.get!(SessionSchedule, id) |
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.
id
here refers to the internal db id, right?
Can we add a method to get session schedule based on batch id?
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.
It's already implemented. We can just use batch_id
as a query parameter something like:
/api/session-schedule?batch_id=1
@@ -36,6 +37,7 @@ defmodule Dbservice.Sessions.Session do | |||
many_to_many(:users, User, join_through: "user_session", on_replace: :delete) | |||
many_to_many(:group_type, GroupType, join_through: "group_session", on_replace: :delete) | |||
belongs_to(:form_schema, FormSchema) | |||
has_many(:session_schedule, SessionSchedule) |
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.
What is the use case for one session having multiple session schedules?
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.
If one session is happening for different batches and the schedule for each batch is different, then I think one to many will work. Its like one session has different schedules for different batches.
response(200, "OK", Schema.ref(:SessionSchedules)) | ||
end | ||
|
||
def index(conn, params) do |
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.
If we have days of week and start and end time, can we store them so that it doesn't require any reformatting on client end? Maybe not, actually -- but something to think about
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.
I'm sorry I didn't get the point. Can you please explain?
def index(conn, params) do | ||
today_day_of_week = Calendar.Date.day_of_week_name(Date.utc_today()) | ||
|
||
query = |
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.
Sorry, maybe I'm not understanding elixir code ;)
but why not filter by batch ID also here?
The main (and only, for now) use case is to get the current session schedule for a batch, right?
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.
batch_id
is being sent as a query parameter from client side, so that filter is already there. In db, let's say if we have 12 sessions (for different days and times) belonging to a same batch_id
, then this will return only the session happening today.
get("/api/session-schedule/{sessionScheduleId}") | ||
|
||
parameters do | ||
sessionScheduleId(:path, :integer, "The id of the session schedule record", required: true) |
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.
We need one to be able to retrieve session schedule for a batch (most common use case)
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.
Already implemented. So there are 5 types of API endpoints:
- Index API (GET /api/session-schedule):
- Purpose: Retrieve a list of session schedules
- Query parameters can be anything we want to filter (like
batch_id
,session_id
,start_time
etc. etc.)
- Create API (POST /api/session-schedule):
- Purpose: Create a new session schedule.
- Show API (GET /api/session-schedule/{sessionScheduleId}):
- Purpose: Retrieve details of a specific session schedule based on id (PK) of session schedule
- Update API (PATCH /api/session-schedule/{sessionScheduleId}):
- Purpose: Update an existing session schedule.
- Delete API (DELETE /api/session-schedule/{sessionScheduleId}):
- Purpose: Delete a session schedule.
This is the 3rd one listed above. If we want to filter session schedule based on batch, we will use the first one (Index API) by sending batch_id
as query param.
send_resp(conn, :no_content, "") | ||
end | ||
end | ||
end |
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.
just clarifying my understanding here: Currently creation of session schedule here is through a API that we can't really use now. This API will be called by something external to DB service when sessions or their schedules are created/modified (for example, on Google Calendar?)
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.
Correct!
Description
Checklist