-
Notifications
You must be signed in to change notification settings - Fork 15
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
feat(place): event scrape #118
base: master
Are you sure you want to change the base?
Conversation
systems: {} of String => SystemWithEvents | ||
} | ||
|
||
now = Time.local |
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 unlikely the the server TZ will be the same as the users TZ.
May be worth pulling a larger time slice, then filtering on front-ends, or support passing a timezone either to this method call, or applying as a setting for the driver instance.
end_epoch = now.at_end_of_day.to_unix | ||
|
||
@zone_ids.each do |z_id| | ||
staff_api.systems(zone_id: z_id).get.as_a.each do |sys| |
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.
The #get
here will block while each request completes, forcing them to run sequentially. It looks like these could happen safely concurrently.
It should be possible to reorganise this so that you map @zone_ids
to an array of Futures (the staff_api.systems(..)
call, then combine these into your response.
|
||
response[:systems][sys_id] = SystemWithEvents.new( | ||
name: sys["name"].as_s, | ||
zones: Array(String).from_json(sys["zones"].to_json), |
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.
There's a roundtrip through some JSON serialisation here that doesn't look to be required.
booking_module = staff_api.modules_from_system(sys_id).get.as_a.find { |mod| mod["name"] == "Bookings" } | ||
# If the system has a booking module with bookings | ||
if booking_module && (bookings = staff_api.get_module_state(booking_module["id"].as_s).get["bookings"]?) |
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.
You may need to expand the staff API driver to support this, but this can collapse into a single query via this endpoint:
Looks good to me testing on my local partner environment. Let me know if anything else should be changed or added @w-le @stakach
Also @stakach , might need your help figuring out why these methods
drivers/drivers/place/staff_api.cr
Lines 225 to 242 in 0e234bc
staff_api.cr
as they're more general than the working methods I added insteaddrivers/drivers/place/staff_api.cr
Lines 189 to 219 in 0e234bc
Oh yeah, tried writing a spec but couldn't get it working as it seems like the spec runner is a little bit flaky.