You can set cardnumber to NULL even if not allowed
commit 785af7310daa4633359755add9474e81f108d022 Bug 6521 - allow blank cardnumbers to not trigger "already in use" + # If the cardnumber is blank, treat it as null. + $newdata{'cardnumber'} = undef if $newdata{'cardnumber'} =~ /^\s*$/; What's the expected behaviour?
Created attachment 153774 [details] [review] Bug 34338: Do not allow spaces in cardnumber
Hum see comment 1. What's the expected behaviour? Should we keep the "if blank treat as null" in memberentry.pl for staff, but reject in other places?
(In reply to Jonathan Druart from comment #3) > Hum see comment 1. What's the expected behaviour? Should we keep the "if > blank treat as null" in memberentry.pl for staff, but reject in other places? No we should reject it too. But something strange is going on here in the direct context: my $new_barcode = $newdata{'cardnumber'}; Koha::Plugins->call( 'patron_barcode_transform', \$new_barcode ); $newdata{'cardnumber'} = $new_barcode; That is interfering with our new logic here. Does not look good to me.
koha-tmpl/intranet-tmpl/prog/en/includes/onboarding_messages.inc Card number contains whitespaces koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt Cardnumber already contains whitespaces. koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt [% ELSIF cardnumber_not_trimmed %] <a href="#borrower_cardnumber"><strong>The entered card number contains whitespaces.</strong></a> Three different strings. And they are not exact. We dont like leading or trailing whitespace.
Created attachment 153967 [details] [review] Bug 34338: Do not allow leading or trailing whitespaces in cardnumber
(In reply to Marcel de Rooy from comment #5) > koha-tmpl/intranet-tmpl/prog/en/includes/onboarding_messages.inc > Card number contains whitespaces > > koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt > Cardnumber already contains whitespaces. > > koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt > [% ELSIF cardnumber_not_trimmed %] > <a href="#borrower_cardnumber"><strong>The entered card number contains > whitespaces.</strong></a> > > Three different strings. > And they are not exact. We dont like leading or trailing whitespace. Fixed. There are still 2 different messages, to match the other strings in the same files. (In reply to Marcel de Rooy from comment #4) > (In reply to Jonathan Druart from comment #3) > > Hum see comment 1. What's the expected behaviour? Should we keep the "if > > blank treat as null" in memberentry.pl for staff, but reject in other places? > > No we should reject it too. It's not clear to me if we want to introduce a behaviour change here. > But something strange is going on here in the direct context: > > my $new_barcode = $newdata{'cardnumber'}; > Koha::Plugins->call( 'patron_barcode_transform', \$new_barcode ); > > $newdata{'cardnumber'} = $new_barcode; > > That is interfering with our new logic here. > Does not look good to me. I don't understand what you mean, can you explain a bit more?
(In reply to Jonathan Druart from comment #7) > It's not clear to me if we want to introduce a behaviour change here. Please explain that will be the benefits of keeping this 'behavior'. > > But something strange is going on here in the direct context: > > > > my $new_barcode = $newdata{'cardnumber'}; > > Koha::Plugins->call( 'patron_barcode_transform', \$new_barcode ); > > > > $newdata{'cardnumber'} = $new_barcode; > > > > That is interfering with our new logic here. > > Does not look good to me. > > I don't understand what you mean, can you explain a bit more? Plugin is called apparently based on cardnumber. But stores info in cardnumber. Should be barcode, right?
If there are no plugins, cardnumber is undef ??
(In reply to Marcel de Rooy from comment #8) > (In reply to Jonathan Druart from comment #7) > > It's not clear to me if we want to introduce a behaviour change here. > Please explain that will be the benefits of keeping this 'behavior'. > > > > But something strange is going on here in the direct context: > > > > > > my $new_barcode = $newdata{'cardnumber'}; > > > Koha::Plugins->call( 'patron_barcode_transform', \$new_barcode ); > > > > > > $newdata{'cardnumber'} = $new_barcode; > > > > > > That is interfering with our new logic here. > > > Does not look good to me. > > > > I don't understand what you mean, can you explain a bit more? > > Plugin is called apparently based on cardnumber. But stores info in > cardnumber. Should be barcode, right? barcode==cardnumber here (In reply to Marcel de Rooy from comment #9) > If there are no plugins, cardnumber is undef ?? No, it's just not modified.
Created attachment 153972 [details] [review] Bug 34338: Do not allow leading or trailing whitespaces in cardnumber