From f31720588f20ea03ab67247f8d87fbbe3c7bd940 Mon Sep 17 00:00:00 2001
From: Bill Erickson <berickxx@gmail.com>
Date: Wed, 10 Apr 2019 12:45:56 -0400
Subject: [PATCH] LP1731021 SIP Fine detail repairs / display fields

Resolve commit issues with the second commit from the source branch
working/user/dpearl/LP1731021_fine_item_detail

Migrating circ title/author lookup to use metabib display fields instead
of MVR's.

Minor code refactoring.

Signed-off-by: Bill Erickson <berickxx@gmail.com>
Signed-off-by: Galen Charlton <gmc@equinoxinitiative.org>
---
 Open-ILS/examples/oils_sip.xml.example        |  14 +-
 .../src/perlmods/lib/OpenILS/SIP/Patron.pm    | 173 +++++++++++-------
 docs/RELEASE_NOTES_NEXT/SIP/AV_format.adoc    |  39 ++--
 3 files changed, 133 insertions(+), 93 deletions(-)

diff --git a/Open-ILS/examples/oils_sip.xml.example b/Open-ILS/examples/oils_sip.xml.example
index fd7f83112e..2146775f19 100644
--- a/Open-ILS/examples/oils_sip.xml.example
+++ b/Open-ILS/examples/oils_sip.xml.example
@@ -29,6 +29,13 @@
 	</listeners>
 
 	<accounts>
+		<!--
+		Fine Item Detail returned by the Patron Information Request is
+		manufacturer-specific.  We support the following formats:
+		3m,  Swyer_A, Swyer_B, and EG_Legacy (default).
+		Specify which treatment you want in the av_format attribute.
+		For example: <login id="sc" password="pwd" institution="main" av_format="3m">
+		-->
 		<login id="scclient" password="clientpwd" institution="gapines"/>
 	</accounts>
 
@@ -129,13 +136,6 @@
 						flag instead of always returning not-OK
 					<option name='patron_calculate_recal_ok' value='true' />
 					-->
-		    <!-- 
-			Fine Item Detail returned by the Patron Information Request is  
-		        manufacturer-specific.  We support the following formats: 
-		           3m,  Swyer_A, Swyer_B, and EG_Legacy (default)
-		        Specify which treatment you want in the av_format option.
-	            <option name='av_format' value='eg_legacy' />
-                    -->
 
 				</options>
 
diff --git a/Open-ILS/src/perlmods/lib/OpenILS/SIP/Patron.pm b/Open-ILS/src/perlmods/lib/OpenILS/SIP/Patron.pm
index 3118bf5290..c9b59022b6 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/SIP/Patron.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/SIP/Patron.pm
@@ -846,75 +846,110 @@ sub charged_items_impl {
 sub fine_items {
     my ($self, $start, $end, $ids_only) = @_;
     my @fines;
-    my ($AV_format_orig, $AV_format, $line);
-
-    eval {
-       $AV_format_orig = OpenILS::SIP->get_option_value('av_format') || '';
-       $AV_format_orig = "eg_legacy" if $AV_format_orig == '';
-
-       $AV_format = lc $AV_format_orig;  # For case-insensitivity
-
-        # Do a prescan for validity and default to eg_legacy
-       if ($AV_format ne "swyer_a" && 
-           $AV_format ne "swyer_b" && 
-           $AV_format ne "eg_legacy" && 
-           $AV_format ne "3m") {
-           syslog('LOG_WARNING', 
-                   "OILS: Unknown value for AV_format: %s", $AV_format_orig);
-           $AV_format = "eg_legacy";
-       }
-
-       my $xacts = $U->simplereq('open-ils.actor', 'open-ils.actor.user.transactions.history.have_balance', $self->{authtoken}, $self->{user}->id);
-
-       foreach my $xact (@{$xacts}) {
-           if ($ids_only) {
-                $line = $xact;
-           
-           #-------------------
-	   #  eg_legacy 
-           #-------------------
-           } elsif ($AV_format eq "eg_legacy") {
-	       $line = $xact->balance_owed . " " . $xact->last_billing_type . " ";
-	       if ($xact->xact_type eq 'circulation') {
-	           my $mods = $U->simplereq('open-ils.circ', 'open-ils.circ.circ_transaction.find_title', $self->{authtoken}, $xact->id);
-		   $line .= $mods->title . ' / ' . $mods->author;
-	       } else {
-		   $line .= $xact->last_billing_note;
-	       }	
-	
-           #-------------------
-	   #  3m  or  swyer_a
-           #-------------------
-	   } elsif ($AV_format eq "3m" or $AV_format eq "swyer_a") {
-	       $line = $xact->id . ' $' . $xact->balance_owed . ' "FINE" ' . $xact->last_billing_type . ' ';
-	       if ($xact->xact_type eq 'circulation') {
-	           my $mods = $U->simplereq('open-ils.circ', 'open-ils.circ.circ_transaction.find_title', $self->{authtoken}, $xact->id);
-		   $line .= $mods->title . ' / ' . $mods->author;
-	       } else {
-	           $line .= $xact->last_billing_note;
-	       }
-
-
-           #-------------------
-	   #  swyer_b
-           #-------------------
-	   } elsif ($AV_format eq "swyer_b") {
-	       $line =   "Charge-Number: " . $xact->id;
-	       $line .=  ", Amount-Due: "  . $xact->balance_owed;
-	       $line .=  ", Fine-Type: FINE";
-
-	       if ($xact->xact_type eq 'circulation') {
-	           my $mods = $U->simplereq('open-ils.circ', 'open-ils.circ.circ_transaction.find_title', $self->{authtoken}, $xact->id);
-	           $line .= ", Title: " . $mods->title;
-	       } else {
-	           $line .= ", Title: " . $xact->last_billing_note;
-	       }
-
-	    }
-
-            push @fines, $line;
-       }
-    };
+
+    my $login = OpenILS::SIP->login_account();
+    my $AV_format = lc($login->{av_format}) || 'eg_legacy';
+
+    # Do a prescan for validity and default to eg_legacy
+    if ($AV_format ne "swyer_a" &&
+        $AV_format ne "swyer_b" &&
+        $AV_format ne "eg_legacy" &&
+        $AV_format ne "3m") {
+
+        syslog('LOG_WARNING',
+            "OILS: Unknown value for AV_format: ". $login->{av_format});
+        $AV_format = "eg_legacy";
+    }
+
+    my $xacts = $U->simplereq('open-ils.actor',
+        'open-ils.actor.user.transactions.history.have_balance',
+        $self->{authtoken}, $self->{user}->id);
+
+    my $line;
+    foreach my $xact (@{$xacts}) {
+
+        if ($ids_only) {
+            push @fines, $xact->id;
+            next;
+        }
+
+        # fine item details requested
+
+        my $title;
+        my $author;
+        my $line;
+
+        if ($xact->xact_type eq 'circulation') {
+            my $e = OpenILS::SIP->editor();
+            my $circ = $e->retrieve_action_circulation([
+                $xact->id, {
+                    flesh => 2,
+                    flesh_fields => {
+                        circ => ['target_copy'],
+                        acp => ['call_number']
+                    }
+                }
+            ]);
+
+            my $displays = $e->search_metabib_flat_display_entry({
+                source => $circ->target_copy->call_number->record,
+                name => ['title', 'author']
+            });
+
+            ($title) = map {$_->value} grep {$_->name eq 'title'} @$displays;
+            ($author) = map {$_->value} grep {$_->name eq 'author'} @$displays;
+
+            # Scrub "/" chars since they are used in some cases 
+            # to delineate title/author.
+            if ($title) {
+                $title =~ s/\///g;
+            } else {
+                $title = '';
+            }
+
+            if ($author) {
+                $author =~ s/\///g;
+            } else {
+                $author = '';
+            }
+        }
+
+        if ($AV_format eq "eg_legacy") {
+
+            $line = $xact->balance_owed . " " . $xact->last_billing_type . " ";
+
+            if ($xact->xact_type eq 'circulation') {
+                $line .= "$title/$author";
+            } else {
+                $line .= $xact->last_billing_note;
+            }
+
+        } elsif ($AV_format eq "3m" or $AV_format eq "swyer_a") {
+
+            $line = $xact->id . ' $' . $xact->balance_owed .
+                ' "FINE" ' . $xact->last_billing_type . ' ';
+
+            if ($xact->xact_type eq 'circulation') {
+                $line .= "$title/$author";
+            } else {
+                $line .= $xact->last_billing_note;
+            }
+
+        } elsif ($AV_format eq "swyer_b") {
+
+            $line =   "Charge-Number: " . $xact->id;
+            $line .=  ", Amount-Due: "  . $xact->balance_owed;
+            $line .=  ", Fine-Type: FINE";
+
+            if ($xact->xact_type eq 'circulation') {
+                $line .= ", Title: $title";
+            } else {
+                $line .= ", Title: " . $xact->last_billing_note;
+            }
+        }
+
+        push @fines, $line;
+    }
 
     my $log_status = $@ ? 'ERROR: ' . $@ : 'OK';
     syslog('LOG_DEBUG', 'OILS: Patron->fine_items() ' . $log_status);
diff --git a/docs/RELEASE_NOTES_NEXT/SIP/AV_format.adoc b/docs/RELEASE_NOTES_NEXT/SIP/AV_format.adoc
index f1c2a942bf..3512b6133c 100644
--- a/docs/RELEASE_NOTES_NEXT/SIP/AV_format.adoc
+++ b/docs/RELEASE_NOTES_NEXT/SIP/AV_format.adoc
@@ -1,18 +1,19 @@
 Fine Item Detail Enhancements
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-SIP now suppports enhancements for the Fine Item Detail returned by by Patron Information
-Response (code 64).  Different manufacturers of self-check systems specify the format
-of the fine item detail differently.  A new option allows you to select the format to
-return.
+SIP now suppports enhancements for the Fine Item Detail returned by
+by Patron Information Response (code 64).  Different manufacturers
+of self-check systems specify the format of the fine item detail
+differently.  A new option allows you to select the format to return.
 
 Configuration
 +++++++++++++
-After installation of Evergreen and SIP, in the Evergreen configuration directory (typically
-/openils/conf) the SIP configuration file oils_sip.xml awaits your modifications to use this
-feature.
+After installation of Evergreen and SIP, in the Evergreen configuration
+directory (typically /openils/conf) the SIP configuration file
+oils_sip.xml awaits your modifications to use this feature.
+
+In the <accounts><login> sections, you can add an attribute of the form
+    `av_format="__<value>__"`
 
-In the <implementation_config><options> section, you can place an option of the form +
-        `<option name='av_format' value='`__<value>__`'>` +
 where __<value>__ is one of thsee values:
 
 * `3m`
@@ -20,14 +21,18 @@ where __<value>__ is one of thsee values:
 * `swyer_a`
 * `swyer_b`
 
-If you omit the option, the default will act as though you wrote +
-        `<option name='av_format' value='eg_legacy'>`
+For example:
+
+<login id="sipclient" password="password" institution="gapines" av_format="3m"/>
+
+If you omit the option, 'eg_legacy' will be used as the default.
+
+Currently, the behaviour of `eg_legacy` is close to, but not precisely
+that of `3m`.  The `eg_legacy` produces the pre-enhancement behavior in
+Evergreen.  Currently, the `swyer_a` behavior is identical to that of
+`3m`, but there is no guarantee that this will always be the case.
 
-Currently, the behaviour of `eg_legacy` is close to, but not precisely that of `3m`.  The
-`eg_legacy` produces the pre-enhancement behavior in Evergreen.   Currently, the
-`swyer_a` behavior
-is identical to that of `3m`, but there is no guarantee that this will always be the case.
+If you change the brand of your self-check equipment, you may need to
+change the value of the option to be consistent with the new brand.
 
-If you change the brand of your self-check equipment, you may need to change the value
-of the option to be consistent with the new brand.
 
-- 
2.43.2