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 <rjs7@calvin.edu>
Signed-off-by: Dan Wells <dbw2@calvin.edu>

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 4af8dae..afb508e 100644 (file)
--- 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 957fb25..44685cf 100644 (file)
--- 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,
     );