From 47f5ddaa8b029d41c710255380260614ef7ea6ae Mon Sep 17 00:00:00 2001 From: Remington Steed Date: Fri, 22 Feb 2019 11:31:59 -0500 Subject: [PATCH] LP#1801191: Refactor for clarity; bugfix This commit makes a few small adjustments: - Replace two instances of 'cleanse_ISO8601' with 'clean_ISO8601', as the former does not exist (at least in these contexts) - Delay converting DateTimes to strings until necessary (which allows us to compare the original DateTime objects instead of having to recreate them from strings) - Increase comments Signed-off-by: Remington Steed Signed-off-by: Dan Wells --- .../Application/Storage/Publisher/action.pm | 19 +++++++------- .../lib/OpenILS/Utils/HoldTargeter.pm | 25 ++++++++----------- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm index 4af8dae616..afb508ed67 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm @@ -1661,7 +1661,7 @@ sub process_recall { $log->info("Found " . scalar(@$all_copies) . " eligible checked-out copies for recall"); - my $return_date = DateTime->now(time_zone => 'local')->add(seconds => interval_to_seconds($return_interval))->iso8601(); + my $return_date = DateTime->now(time_zone => 'local')->add(seconds => interval_to_seconds($return_interval)); # Iterate over the checked-out copies to find a copy with a # loan period longer than the recall threshold: @@ -1675,23 +1675,24 @@ sub process_recall { my $circ = $circs->[0]; $log->info("Recalling circ ID : " . $circ->id); - my $old_due_date = DateTime::Format::ISO8601->parse_datetime(cleanse_ISO8601($circ->due_date))->iso8601(); + my $old_due_date = DateTime::Format::ISO8601->parse_datetime(clean_ISO8601($circ->due_date)); # Give the user a new due date of either a full recall threshold, # or the return interval, whichever is further in the future - my $threshold_date = DateTime::Format::ISO8601->parse_datetime(clean_ISO8601($circ->xact_start))->add(seconds => interval_to_seconds($recall_threshold))->iso8601(); - if (DateTime->compare(DateTime::Format::ISO8601->parse_datetime($threshold_date), DateTime::Format::ISO8601->parse_datetime($return_date)) == 1) { + my $threshold_date = DateTime::Format::ISO8601->parse_datetime(clean_ISO8601($circ->xact_start))->add(seconds => interval_to_seconds($recall_threshold)); + if (DateTime->compare($threshold_date, $return_date) == 1) { + # extend $return_date to threshold $return_date = $threshold_date; } - - # But if the new due date is later than the old one, - # keep the old one. - if (DateTime->compare(DateTime::Format::ISO8601->parse_datetime($return_date), DateTime::Format::ISO8601->parse_datetime($old_due_date)) == 1) { + # But don't go past the original due date + # (the threshold should not be past the due date, but manual edits can cause it to be) + if (DateTime->compare($return_date, $old_due_date) == 1) { + # truncate $return_date to due date $return_date = $old_due_date; } my $update_fields = { - due_date => $return_date, + due_date => $return_date->iso8601(), renewal_remaining => 0, }; diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm b/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm index 957fb25a8b..44685cff7c 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm @@ -1181,34 +1181,31 @@ sub process_recalls { $self->log_hold("recalling circ ".$circ->id); - my $old_due_date = DateTime::Format::ISO8601->parse_datetime(cleanse_ISO8601($circ->due_date))->iso8601(); + my $old_due_date = DateTime::Format::ISO8601->parse_datetime(clean_ISO8601($circ->due_date)); # Give the user a new due date of either a full recall threshold, # or the return interval, whichever is further in the future. my $threshold_date = DateTime::Format::ISO8601 ->parse_datetime(clean_ISO8601($circ->xact_start)) - ->add(seconds => interval_to_seconds($threshold)) - ->iso8601(); + ->add(seconds => interval_to_seconds($threshold)); my $return_date = DateTime->now(time_zone => 'local')->add( - seconds => interval_to_seconds($interval))->iso8601(); + seconds => interval_to_seconds($interval)); - if (DateTime->compare( - DateTime::Format::ISO8601->parse_datetime($threshold_date), - DateTime::Format::ISO8601->parse_datetime($return_date)) == 1) { + if (DateTime->compare($threshold_date, $return_date) == 1) { + # extend $return_date to threshold $return_date = $threshold_date; } - - # But if the new due date is later than the old one, - # keep the old one. - if (DateTime->compare( - DateTime::Format::ISO8601->parse_datetime($return_date), - DateTime::Format::ISO8601->parse_datetime($old_due_date)) == 1) { + # But don't go past the original due date + # (the threshold should not be past the due date, but manual edits can + # cause it to be) + if (DateTime->compare($return_date, $old_due_date) == 1) { + # truncate $return_date to due date $return_date = $old_due_date; } my %update_fields = ( - due_date => $return_date, + due_date => $return_date->iso8601(), renewal_remaining => 0, ); -- 2.43.2