Bug 13871: [QA Follow-up] Adjust Patron Status Request
authorMarcel de Rooy <m.de.rooy@rijksmuseum.nl>
Sat, 27 Feb 2016 13:26:51 +0000 (14:26 +0100)
committerBrendan A Gallagher <brendan@bywatersolutions.com>
Mon, 7 Mar 2016 17:22:20 +0000 (17:22 +0000)
Conform QA comment on Bugzilla, we do this:

[1] Attribute for overdrive mode/invalid credentials is not really needed.
    We can always pass a screen message that card or password is invalid.
[2] If the cardnumber is correct and the password is wrong, we should
    still honour the request. The bad password is recognized by BLN and
    an additional message in AF.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Verified by telnetting SIP server.
And tested additionally with the new unit test of bug 15956.

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
C4/SIP/Sip/MsgType.pm

index 451f228..65faef3 100644 (file)
@@ -23,6 +23,9 @@ use UNIVERSAL::can;
 
 use vars qw(@ISA $VERSION @EXPORT_OK);
 
+use constant INVALID_CARD => 'Invalid cardnumber';
+use constant INVALID_PW   => 'Invalid password';
+
 BEGIN {
     $VERSION   = 3.07.00.049;
     @ISA       = qw(Exporter);
@@ -406,26 +409,16 @@ sub handle {
 #
 sub build_patron_status {
     my ( $patron, $lang, $fields, $server ) = @_;
-    my $overdrive_mode = $server->{account}->{'overdrive-mode'};
+
     my $patron_pwd = $fields->{ (FID_PATRON_PWD) };
     my $resp = (PATRON_STATUS_RESP);
-    my $password_ok = 1;
     my $password_rc;
 
-    if ($patron) {
+    if ( $patron ) {
         if ($patron_pwd) {
             $password_rc = $patron->check_password($patron_pwd);
-            $password_ok = 0 unless $password_rc;
         }
-        elsif ( $overdrive_mode
-            and not exists $fields->{'AL'} # not block_request
-            and not $patron_pwd ) # no password supplied
-        {
-            $password_ok = 0;
-        }
-    }
 
-    if ( $patron and $password_ok ) {
         $resp .= patron_status_string($patron);
         $resp .= $lang . timestamp();
         $resp .= add_field( FID_PERSONAL_NAME, $patron->name );
@@ -443,13 +436,16 @@ sub build_patron_status {
             $resp .= maybe_add( FID_FEE_AMT,  $patron->fee_amount );
         }
 
-        $resp .= maybe_add( FID_SCREEN_MSG, $patron->screen_msg, $server );
+        my $msg = $patron->screen_msg;
+        $msg .= ' -- '. INVALID_PW if $patron_pwd && !$password_rc;
+        $resp .= maybe_add( FID_SCREEN_MSG, $msg, $server );
+
         $resp .= maybe_add( FID_SCREEN_MSG, $patron->{branchcode}, $server )
           if ( $server->{account}->{send_patron_home_library_in_af} );
         $resp .= maybe_add( FID_PRINT_LINE, $patron->print_line );
-    } else {
 
-        # Invalid patron id (and/or passwd for overdrive_mode)
+    } else {
+        # Invalid patron (cardnumber)
         # Report that the user has no privs.
 
         # no personal name, and is invalid (if we're using 2.00)
@@ -463,8 +459,7 @@ sub build_patron_status {
         ( $protocol_version >= 2 )
           and $resp .= add_field( FID_VALID_PATRON, 'N' );
 
-        $resp .=
-          maybe_add( FID_SCREEN_MSG, 'Invalid patron or patron password' );
+        $resp .= maybe_add( FID_SCREEN_MSG, INVALID_CARD );
     }
 
     $resp .= add_field( FID_INST_ID, $fields->{ (FID_INST_ID) } );