If PatronSelfModificationBorrowerUnwantedField contains dateofbirth, all patron self modification requests will attempt to set dateofbirth to null instead of ignoring the field.
Created attachment 105908 [details] [review] Bug 25769: Patron self modification triggers change request for date of birth to null If PatronSelfModificationBorrowerUnwantedField contains dateofbirth, all patron self modification requests will attempt to set dateofbirth to null instead of ignoring the field. Test Plan: 1) Add dateofbirth to PatronSelfModificationBorrowerUnwantedField 2) Run a patron self modification 3) Note the request sets dateofbirth to null on the staff side 4) Apply this patch 5) Restart all the things! 6) Run another self-modification 7) Note of the staff side that dateofbirth is unaffected
Created attachment 105938 [details] [review] Bug 25769: Patron self modification triggers change request for date of birth to null If PatronSelfModificationBorrowerUnwantedField contains dateofbirth, all patron self modification requests will attempt to set dateofbirth to null instead of ignoring the field. Test Plan: 1) Add dateofbirth to PatronSelfModificationBorrowerUnwantedField 2) Run a patron self modification 3) Note the request sets dateofbirth to null on the staff side 4) Apply this patch 5) Restart all the things! 6) Run another self-modification 7) Note of the staff side that dateofbirth is unaffected Signed-off-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Comment on attachment 105938 [details] [review] Bug 25769: Patron self modification triggers change request for date of birth to null Review of attachment 105938 [details] [review]: ----------------------------------------------------------------- ::: opac/opac-memberentry.pl @@ +484,5 @@ > > + if ( defined $borrower{'dateofbirth'} ) { > + my $dob_dt; > + $dob_dt = eval { dt_from_string( $borrower{'dateofbirth'} ); } > + if ( $borrower{'dateofbirth'} ); You're now testing if $borrower{'dateofbirth'} is defined above.. do you still need to test it here too?
Failing QA whilst I wait for a reply.
(In reply to Martin Renvoize from comment #3) > Comment on attachment 105938 [details] [review] [review] > Bug 25769: Patron self modification triggers change request for date of > birth to null > > Review of attachment 105938 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: opac/opac-memberentry.pl > @@ +484,5 @@ > > > > + if ( defined $borrower{'dateofbirth'} ) { > > + my $dob_dt; > > + $dob_dt = eval { dt_from_string( $borrower{'dateofbirth'} ); } > > + if ( $borrower{'dateofbirth'} ); > > You're now testing if $borrower{'dateofbirth'} is defined above.. do you > still need to test it here too? I suppose not! Followup will be forthcoming.
(In reply to Martin Renvoize from comment #3) > Comment on attachment 105938 [details] [review] [review] > Bug 25769: Patron self modification triggers change request for date of > birth to null > > Review of attachment 105938 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: opac/opac-memberentry.pl > @@ +484,5 @@ > > > > + if ( defined $borrower{'dateofbirth'} ) { > > + my $dob_dt; > > + $dob_dt = eval { dt_from_string( $borrower{'dateofbirth'} ); } > > + if ( $borrower{'dateofbirth'} ); > > You're now testing if $borrower{'dateofbirth'} is defined above.. do you > still need to test it here too? After examining the code, yes we do still need it. This new check tests to see if the dateofbirth param is part of the form. If it *is*, it still may be blank. Without this check, we'd set the dateofbirth to the current date.
(In reply to Kyle M Hall from comment #6) > (In reply to Martin Renvoize from comment #3) > > Comment on attachment 105938 [details] [review] [review] [review] > > Bug 25769: Patron self modification triggers change request for date of > > birth to null > > > > Review of attachment 105938 [details] [review] [review] [review]: > > ----------------------------------------------------------------- > > > > ::: opac/opac-memberentry.pl > > @@ +484,5 @@ > > > > > > + if ( defined $borrower{'dateofbirth'} ) { > > > + my $dob_dt; > > > + $dob_dt = eval { dt_from_string( $borrower{'dateofbirth'} ); } > > > + if ( $borrower{'dateofbirth'} ); > > > > You're now testing if $borrower{'dateofbirth'} is defined above.. do you > > still need to test it here too? > > After examining the code, yes we do still need it. This new check tests to > see if the dateofbirth param is part of the form. If it *is*, it still may > be blank. Without this check, we'd set the dateofbirth to the current date. Ah, of course.. good catch.
Created attachment 106146 [details] [review] Bug 25769: Patron self modification triggers change request for date of birth to null If PatronSelfModificationBorrowerUnwantedField contains dateofbirth, all patron self modification requests will attempt to set dateofbirth to null instead of ignoring the field. Test Plan: 1) Add dateofbirth to PatronSelfModificationBorrowerUnwantedField 2) Run a patron self modification 3) Note the request sets dateofbirth to null on the staff side 4) Apply this patch 5) Restart all the things! 6) Run another self-modification 7) Note of the staff side that dateofbirth is unaffected Signed-off-by: Emmi Takkinen <emmi.takkinen@outlook.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Works as expected, passes qa scripts. Passing QA
Pushed to master for 20.11, thanks to everybody involved!
backported to 20.05.x for 20.05.02
backported to 19.11.x for 19.11.08
Backported to 19.05.x branch for 19.05.13