Repository navigation
fix(place/room_booking_approval): answer approvals at once - #640
Merged
Merged
Conversation
accept_event, decline_event and the recurring variants ended with refresh_bookings, whose last expression was the Tasker task from schedule.in. The driver executor waits on any returned value that responds to get, so every approval exec blocked for the whole refresh_debounce_seconds window (10 s by default) before replying. Concierge gives an exec 10 s, so approving a booking raced the timeout and usually looked like nothing happened even though the calendar had accepted the event. Give refresh_bookings a Nil return type so the exec replies as soon as the calendar call is queued. The spec now checks the exec answers well inside the debounce window.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Problem
Jonathan reported on dev Concierge that approving a pending room booking does nothing. Reproduced on placeos-dev today: the
accept_eventexec on RoomBookingApproval took 10,011 ms (core log), which is exactlyrefresh_debounce_seconds.accept_event,decline_event,accept_recurring_eventanddecline_recurring_eventend withrefresh_bookings, whose last expression is theTaskertask returned byschedule.in. The driver executor callsgeton any returned value that responds to it, so the exec blocks until the debounce timer fires. The ts-client default exec timeout is also 10 s, so Concierge raced the reply and usually lost: spinner for 10 s, then either nothing or "Request timed out", even though the calendar had accepted the event.Fix
refresh_bookingsnow has aNilreturn type, so the exec replies as soon as the calendar call is queued and the debounced re-poll still runs on its timer.The spec asserts the accept and decline execs answer in well under the debounce window.
Verification
./harness report drivers/place/room_booking_approval_spec.cr: 1 tested, 0 failures.Companion frontend change (already on
user-interfacesdevelop, eea67f658e): Concierge gives approve and decline the same 30 s allowance the series actions had, so it also works against the currently deployed driver.