Adjust time math and API return data

Make sure that all DateTime objects that are in the floating timezone
are put into the configured (or default local) timezone, so that date
math is always sane.  Note that DateTime->now returns an object in UTC
so it can be used in math and calls to the database safely -- Postgres
converts for us as needed with any non-floating timezone.

Return the created or updated curbside appointment object when
applicable.

Return -1 on successful delete to avoid possible confusion with
appointment IDs.

Flesh the patron's card so we can display a barcode when returning a
list of appointments for grid display.

Return the copy object and wide display entry object for each hold's
bib record when sending the list of to-be-staged appointment slots,
so staff can cross-check title, author, item barcode, etc.

Signed-off-by: Mike Rylander <mrylander@gmail.com>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Curbside.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Curbside.pm
index ad682c1..c96c57d 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Curbside.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Curbside.pm
@@ -109,7 +109,7 @@ sub fetch_arrived {
         arrival => { '!=' => undef},
         delivered => undef,
     },{
-        flesh => 1, flesh_fields => {acsp => ['patron']},
+        flesh => 2, flesh_fields => {acsp => ['patron'], au => ['card']},
         order_by => { acsp => 'arrival' }
     }]);
 
@@ -191,7 +191,7 @@ sub fetch_staged {
         staged => { '!=' => undef},
         arrival => undef
     },{
-        flesh => 1, flesh_fields => {acsp => ['patron']},
+        flesh => 2, flesh_fields => {acsp => ['patron'], au => ['card']},
         order_by => { acsp => 'slot' }
     }]);
 
@@ -270,7 +270,7 @@ sub fetch_to_be_staged {
 
     my $gran = $U->ou_ancestor_setting_value($org, 'circ.curbside.granularity') || '15 minutes';
     my $gran_seconds = interval_to_seconds($gran);
-    my $horizon = DateTime->now;
+    my $horizon = DateTime->now; # NOTE: does not need timezone set because it gets UTC, not floating, so we can math with it
     $horizon->add(seconds => $gran_seconds * 2);
 
     my $slots = $e->search_action_curbside([{
@@ -278,21 +278,31 @@ sub fetch_to_be_staged {
         staged => undef,
         slot => { '<=' => $horizon->strftime('%FT%T%z') },
     },{
-        flesh => 1, flesh_fields => {acsp => ['patron']},
+        flesh => 2, flesh_fields => {acsp => ['patron'], au => ['card']},
         order_by => { acsp => 'slot' }
     }]);
 
     for my $s (@$slots) {
-        my $holds = $e->search_action_hold_request({
+        my $holds = $e->search_action_hold_request([{
             usr => $s->patron->id,
             current_shelf_lib => $s->org,
             pickup_lib => $s->org,
             shelf_time => {'!=' => undef},
             cancel_time => undef,
             fulfillment_time => undef
-        });
+        },{
+            flesh => 1, flesh_fields => {ahr => ['current_copy']},
+        }]);
 
-        $conn->respond({slot => $s, holds => $holds});
+        my $rhrr_list = $e->search_reporter_hold_request_record(
+            {id => [ map { $_->id } @$holds ]}
+        );
+
+        my %bib_data = map {
+            ($_->id => $e->retrieve_metabib_wide_display_entry( $_->bib_record))
+        } @$rhrr_list;
+
+        $conn->respond({slot => $s, holds => $holds, bib_data_by_hold => \%bib_data});
     }
 
     return undef;
@@ -323,7 +333,7 @@ sub fetch_latest_to_be_staged {
 
     my $gran = $U->ou_ancestor_setting_value($org, 'circ.curbside.granularity') || '15 minutes';
     my $gran_seconds = interval_to_seconds($gran);
-    my $horizon = DateTime->now;
+    my $horizon = DateTime->now; # NOTE: does not need timezone set because it gets UTC, not floating, so we can math with it
     $horizon->add(seconds => $gran_seconds * 2);
 
     my $slots = $e->search_action_curbside([{
@@ -380,16 +390,19 @@ sub times_for_date {
     my $close_time = $hoo->$close_method;
     return $conn->respond_complete if ($open_time eq $close_time); # location closed that day
 
-    $start_obj = $date_parser->parse_datetime($date.'T'.$open_time); # reset this to opening time
-    my $end_obj = $date_parser->parse_datetime($date.'T'.$close_time);
+    my $tz = $U->ou_ancestor_setting_value($org, 'lib.timezone') || 'local';
+    $start_obj = $date_parser->parse_datetime($date.'T'.$open_time)->set_time_zone($tz); # reset this to opening time
+    my $end_obj = $date_parser->parse_datetime($date.'T'.$close_time)->set_time_zone($tz);
 
-    my $now_obj = DateTime->now;
+    my $now_obj = DateTime->now; # NOTE: does not need timezone set because it gets UTC, not floating, so we can math with it
     my $step_obj = $start_obj->clone;
     while (DateTime->compare($step_obj,$end_obj) <= 0) { # inside HOO
-        if (DateTime->compare($step_obj,$now_obj) >= 0) { # don't offer times in the past
+        if (DateTime->compare($step_obj,$now_obj) >= 0) { # only offer times in the future
             my $step_ts = $step_obj->strftime('%FT%T%z');
             my $other_slots = $e->search_action_curbside({org => $org, slot => $step_ts}, {idlist => 1});
             my $available = $max - scalar(@$other_slots);
+            $available = $available < 0 ? 0 : $available; # so truthiness testing is always easy in the client
+
             $conn->respond([$step_obj->strftime('%T'), $available]);
         }
         $step_obj->add(seconds => $gran_seconds);
@@ -432,7 +445,7 @@ sub create_update_appointment {
         return $e->die_event unless $e->allowed("STAFF_LOGIN");
     }
 
-    my $date_obj = $date_parser->parse_datetime($date);
+    my $date_obj = $date_parser->parse_datetime($date); # no TZ necessary, just using it to test the input and get DOW
     return undef unless ($date_obj);
 
     if ($time =~ /^\d\d:\d\d$/) {
@@ -460,7 +473,6 @@ sub create_update_appointment {
         }
     }
 
-
     my $gran = $U->ou_ancestor_setting_value($org, 'circ.curbside.granularity') || '15 minutes';
     my $max = $U->ou_ancestor_setting_value($org, 'circ.curbside.max_concurrent') || 10;
 
@@ -486,19 +498,24 @@ sub create_update_appointment {
     return undef if ($time_seconds < $open_seconds); # too early
     return undef if ($time_seconds > $close_seconds + 1); # too late (/at/ closing allowed)
 
-    my $tz = $U->ou_ancestor_setting_value($org, 'lib.timezone') || 'local';
-
-    $date_obj = $date_parser->parse_datetime($date.'T'.$open_time)->set_time_zone($tz);
-
     my $time_into_open_second = $time_seconds - $open_seconds;
     if (my $extra_time = $time_into_open_second % $gran) { # a remainder means we got a time we shouldn't have
         $time_into_open_second -= $extra_time; # just back it off to have staff gather earlier
     }
 
+    my $tz = $U->ou_ancestor_setting_value($org, 'lib.timezone') || 'local';
+    $date_obj = $date_parser->parse_datetime($date.'T'.$open_time)->set_time_zone($tz);
+
     my $slot_ts = $date_obj->add(seconds => $time_into_open_second)->strftime('%FT%T%z');
 
     # finally, confirm that there aren't too many already
-    my $other_slots = $e->search_action_curbside({org => $org, slot => $slot_ts}, {idlist => 1});
+    my $other_slots = $e->search_action_curbside(
+        { org => $org,
+          slot => $slot_ts,
+          ( $slot ? (id => { '<>' => $slot->id }) : () ) # exclude our own slot from the count
+        },
+        {idlist => 1}
+    );
     if (scalar(@$other_slots) >= $max) { # oops... return error
         my $ev = new OpenILS::Event("CURBSIDE_MAX_FOR_TIME");
         $e->disconnect;
@@ -523,10 +540,10 @@ sub create_update_appointment {
     }
 
     $slot->slot($slot_ts);
-    $slot = $e->$method($slot);
+    $e->$method($slot) or return $e->die_event;
 
     $e->commit;
-    $conn->respond_complete($slot);
+    $conn->respond_complete($e->retrieve_action_curbside($slot->id));
 
     OpenSRF::AppSession
         ->create('open-ils.trigger')
@@ -588,7 +605,7 @@ sub delete_appointment {
     $e->delete_action_curbside($slot) or return $e->die_event;
     $e->commit;
 
-    return 1;
+    return -1;
 }
 __PACKAGE__->register_method(
     method   => "delete_appointment",
@@ -598,8 +615,8 @@ __PACKAGE__->register_method(
             {type => 'string', desc => 'Authentication token'},
             {type => 'number', desc => 'Appointment ID'},
         ],
-        return => { desc => 'Nothing on success or no appointment found'.
-                            'an ILS Event on permission error'}
+        return => { desc => '-1 on success, nothing when no appointment found, '.
+                            'or an ILS Event on permission error'}
     }
 );
 
@@ -614,10 +631,10 @@ sub mark_staged {
 
     $slot->staged('now');
     $slot->stage_staff($e->requestor->id);
-    $slot = $e->update_action_curbside($slot) or return $e->die_event;
+    $e->update_action_curbside($slot) or return $e->die_event;
     $e->commit;
 
-    return $slot;
+    return $e->retrieve_action_curbside($slot->id);
 }
 __PACKAGE__->register_method(
     method   => "mark_staged",
@@ -646,10 +663,10 @@ sub mark_arrived {
 
     $slot->arrival('now');
 
-    $slot = $e->update_action_curbside($slot) or return $e->die_event;
+    $e->update_action_curbside($slot) or return $e->die_event;
     $e->commit;
 
-    return $slot;
+    return $e->retrieve_action_curbside($slot->id);
 }
 __PACKAGE__->register_method(
     method   => "mark_arrived",
@@ -659,8 +676,8 @@ __PACKAGE__->register_method(
             {type => 'string', desc => 'Authentication token'},
             {type => 'number', desc => 'Appointment ID'},
         ],
-        return => { desc => 'Nothing on success or no appointment found'.
-                            'an ILS Event on permission error'}
+        return => { desc => 'Appointment on success, nothing when no appointment found, '.
+                            'or an ILS Event on permission error'}
     }
 );
 
@@ -708,7 +725,7 @@ sub mark_delivered {
     $conn->respond($_->gather(1)) for @requests;
     $circ_sess->disconnect;
 
-    return $slot->id;
+    return $e->retrieve_action_curbside($slot->id);
 }
 __PACKAGE__->register_method(
     method   => "mark_delivered",