From 669e2f813ca919be35a210ec82eea4546a235750 Mon Sep 17 00:00:00 2001
From: Baptiste <baptiste.wojtkowski@biblibre.com>
Date: Wed, 4 Dec 2024 16:43:15 +0100
Subject: [PATCH] Bug 35292: Have UpdateNotForLoanStatusOnCheckOut and
 UpdateNotForLoanStatusOnCheckIn use the same function

Both sysprefs use the same codebase. Hence the recent upgrades for
UpdateNotForLoanStatusOnCheckOut were not applyied to checkout.
In this patch, we refacto code in order to have them both use the same
function.

Test plan:
Do not apply patch:
1 -  Set UpdateNotForLoanStatusOnCheckin to
2: 0
2 - Set  UpdateNotForLoanStatusOnCheckout to
_ALL_:
 0: 2
3 - Check out a book -> Its notforloan status is set to 2
4 - Check in the book -> Its notforloan status is set to 0, you get a
  notification
6 - APPLY PATCH
7 -  Set UpdateNotForLoanStatusOnCheckin to
_ALL_:
 2: 0

BK:
 3: 0

CR:
 4: 0

8 -  Set UpdateNotForLoanStatusOnCheckOut to
_ALL_:
 2: 0

BK:
 3: 0

CR:
 4: 0

8 - Check out a book -> Its notforloan status is set to 3
9 - Check in the book -> Its notforloan status is set to 0, you get a
10 - Check out a continuing resource -> Its notforloan status is set to 4
11 - Check in a continuing resource -> Its notforloan status is set to 0
11 - Check out another type of resource -> Its notforloan status is set to 2
12 - Check in the document -> Its notforloan status is set to 0
  notification
---
 C4/Circulation.pm | 92 +++++++++++++++++++++--------------------------
 1 file changed, 40 insertions(+), 52 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 65f440d5c38..1e2e42e386a 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -1796,25 +1796,7 @@ sub AddIssue {
                     $item_unblessed->{charge} = $charge;
                 }
             }
-
-            my $yaml = C4::Context->preference('UpdateNotForLoanStatusOnCheckout');
-            if ($yaml) {
-                $yaml = "$yaml\n\n";
-
-                my $rules;
-                eval { $rules = YAML::XS::Load(Encode::encode_utf8($yaml)); };
-                if ($@) {
-                    warn "Unable to parse UpdateNotForLoanStatusOnCheckout syspref : $@";
-                }
-                else {
-                    foreach my $key ( keys %$rules ) {
-                        if ( $item_object->notforloan eq $key ) {
-                            $item_object->notforloan($rules->{$key})->store({ log_action => 0, skip_record_index => 1 });
-                            last;
-                        }
-                    }
-                }
-            }
+            _updateNotForLoanFromYaml( $item_object, 'UpdateNotForLoanStatusOnCheckout' );
 
             # Record the fact that this book was issued.
             C4::Stats::UpdateStats(
@@ -2140,7 +2122,7 @@ sub AddReturn {
     $branch = C4::Context->userenv->{'branch'} unless $branch;  # we trust userenv to be a safe fallback/default
     my $return_date_specified = !!$return_date;
     $return_date //= dt_from_string();
-    my $messages;
+    my $messages = {};
     my $patron;
     my $doreturn       = 1;
     my $validTransfer = 1;
@@ -2216,38 +2198,7 @@ sub AddReturn {
         $messages->{ $loc_msg_key } = $loc_messages->{ $loc_msg_key };
     }
 
-    my $yaml = C4::Context->preference('UpdateNotForLoanStatusOnCheckin');
-    if ($yaml) {
-        $yaml = "$yaml\n\n";  # YAML is anal on ending \n. Surplus does not hurt
-        my $rules;
-        eval { $rules = YAML::XS::Load(Encode::encode_utf8($yaml)); };
-        if ($@) {
-            warn "Unable to parse UpdateNotForLoanStatusOnCheckin syspref : $@";
-        }
-        else {
-            if ( defined $rules->{$item->itype} ) {
-                foreach my $notloan_rule_key (keys %{ $rules->{$item->itype}} ) {
-                    if ( $item->notforloan eq $notloan_rule_key ) {
-                        $messages->{'NotForLoanStatusUpdated'} = { from => $item->notforloan, to => $rules->{$item->itype}->{$notloan_rule_key} };
-                        $item->notforloan( $rules->{ $item->itype }->{$notloan_rule_key} )
-                            ->store( { log_action => 0, skip_record_index => 1, skip_holds_queue => 1 } )
-                            unless $rules->{ $item->itype }->{$notloan_rule_key} eq 'ONLYMESSAGE';
-                        last;
-                    }
-                }
-            } elsif ( defined $rules->{'_ALL_'} ) {
-                foreach my $notloan_rule_key (keys %{ $rules->{'_ALL_'}} ) {
-                    if ( $item->notforloan eq $notloan_rule_key ) {
-                        $messages->{'NotForLoanStatusUpdated'} = { from => $item->notforloan, to => $rules->{'_ALL_'}->{$notloan_rule_key} };
-                        $item->notforloan( $rules->{'_ALL_'}->{$notloan_rule_key} )
-                            ->store( { log_action => 0, skip_record_index => 1, skip_holds_queue => 1 } )
-                            unless $rules->{ '_ALL_' }->{$notloan_rule_key} eq 'ONLYMESSAGE';
-                        last;
-                    }
-                }
-            }
-        }
-    }
+    _updateNotForLoanFromYaml( $item, 'UpdateNotForLoanStatusOnCheckin', $messages );
 
     # check if the return is allowed at this branch
     my ($returnallowed, $message) = CanBookBeReturned($item->unblessed, $branch);
@@ -4634,6 +4585,43 @@ sub _CanBookBeAutoRenewed {
     return "ok";
 }
 
+sub _updateNotForLoanFromYaml {
+    my ( $item, $NotForLoanUpdatePreference, $messages ) = @_;
+
+    my $yaml = C4::Context->preference($NotForLoanUpdatePreference);
+    if ($yaml) {
+        $yaml = "$yaml\n\n";
+        my $rules;
+        eval { $rules = YAML::XS::Load( Encode::encode_utf8($yaml) ); };
+        if ($@) {
+            warn "Unable to parse $NotForLoanUpdatePreference syspref : $@";
+        } else {
+            if ( defined $rules->{ $item->itype } ) {
+                foreach my $notloan_rule_key ( keys %{ $rules->{ $item->itype } } ) {
+                    if ( $item->notforloan eq $notloan_rule_key ) {
+                        $messages->{'NotForLoanStatusUpdated'} =
+                            { from => $item->notforloan, to => $rules->{ $item->itype }->{$notloan_rule_key} };
+                        $item->notforloan( $rules->{ $item->itype }->{$notloan_rule_key} )
+                            ->store( { log_action => 0, skip_record_index => 1, skip_holds_queue => 1 } )
+                            unless $rules->{ $item->itype }->{$notloan_rule_key} eq 'ONLYMESSAGE';
+                        last;
+                    }
+                }
+            } elsif ( defined $rules->{'_ALL_'} ) {
+                foreach my $notloan_rule_key ( keys %{ $rules->{'_ALL_'} } ) {
+                    if ( $item->notforloan eq $notloan_rule_key ) {
+                        $messages->{'NotForLoanStatusUpdated'} =
+                            { from => $item->notforloan, to => $rules->{'_ALL_'}->{$notloan_rule_key} };
+                        $item->notforloan( $rules->{'_ALL_'}->{$notloan_rule_key} )
+                            ->store( { log_action => 0, skip_record_index => 1, skip_holds_queue => 1 } )
+                            unless $rules->{'_ALL_'}->{$notloan_rule_key} eq 'ONLYMESSAGE';
+                        last;
+                    }
+                }
+            }
+        }
+    }
+}
 
 1;
 
-- 
2.30.2