Currently the card number is generated when the user enters the patron creation form. This creates a problem of concurrency - when two or more simulataneous users are registering members, the error "card no. in use" can occur. This change moves the card number generation to occur after the "Save" button is pressed.
Confirmed. The method described by Savitra is the way it works when generating barcodes for items and it should indeed be the one used to generate card numbers as well.
Created attachment 9945 [details] [review] Attaching Patch.
The field is still marked as required, so fails the javascript validation on form submit. The comment under the field would be better if it followed the style under the other form elements - <div class="hint">
Michael, you can choose which fields are mandatory in the BorrowerMandatoryField syspref, cardnumber is mandatory by default. So with this patch if you activate autoMemberNum then you need to remove cardnumber form BorrowerMandatoryField. I would say either we just add it in the text that goes with the syspref : "Do / Don't default the card number field on the patron addition screen to the next available card number (for example, if the largest currently used card number is 26345000012941, then this field will default to 26345000012942). ++Remember to remove cardnumber from the BorrowerMandatoryField syspref if you switch this on++", or we remove it from BorrowerMandatoryField's default value (i think this would be a bad idea), or we find a way to bypass the mandatory behaviour on this field when autoMemberNum is on (This sounds like a complicated idea). I'll sign this off for now, and leave it to the QA team to decide wether we need to add something to make things clearer.
Created attachment 10625 [details] [review] [SIGNED-OFF] Bug 6782 - Move auto member cardnumber generation to occur when record is "Saved" (avoid collisions). Currently the card number is generated when the user enters the patron creation form. This creates a problem of concurrency - when two or more simulataneous users are registering members, the error "card no. in use" can occur. This change moves the card number generation to occur after the "Save" button is pressed. Changes: -C4/Members.pm: Added code to fixup_cardnumber,If the cardnumber is blank and "autoMemberNum" ON. -koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt: Added code to display "leave blank for auto calc during registration" in cardnumber label in patron registration form only if "autoMemberNum" ON. -members/memberentry.pl: Added code to get weather or not "autoMemberNum" is on or off and removed fixup_cardnumber generation. Test cases: -If "autoMemberNum" ON: ->In blank case, must generate auto card number in simulataneous users. ->If user entered, check for unique card number. -If "autoMemberNum" OFF: Must work normal. Signed-off-by: Gaetan Boisson <gaetan.boisson@biblibre.com>
Comment on attachment 9945 [details] [review] Attaching Patch. This patch obsoleted by the signed-off one
QA comment: some Perl comments: + if(C4::Context->preference("autoMemberNum")) { + if ($data{'cardnumber'} eq '') { + $data{'cardnumber'}= fixup_cardnumber($data{'cardnumber'},C4::Context->userenv->{'branch'}, $data{'categorycode'}); + } + } => you should write + if (C4::Context->preference("autoMemberNum") && $data{'cardnumber'} eq '') { + $data{'cardnumber'}= fixup_cardnumber($data{'cardnumber'},C4::Context->userenv->{'branch'}, $data{'categorycode'}); + } and +if (C4::Context->preference("autoMemberNum")) { + $template->param( autoMemberNum => 1); +} else { + $template->param( autoMemberNum => 0); +} could be set + $template->param( autoMemberNum => C4::Context->preference("autoMemberNum")); Marking failed QA for that reason, please fix & resubmit, i'll push quickly
Seems still to be an issue. To reproduce: - Set syspref autoMemberNum to 'on' - Open two browser windows/tabs and go in both to Home > Patrons > Add patron - Verify that in both entry forms the field 'Card number' is populated with the same value - Save form in browser window 1 =>OK - Save form in browser window 2 => Result in window 2: Message "Cardnumber already in use.", and the field 'Card number' is pupulated with the next higher number. Saving again is then OK
- Still an issue on current master, see comment #8 - Patch does not apply
Created attachment 55952 [details] [review] Bug 6782 - Move auto member cardnumber generation to occur when record is "Saved" (avoid collisions). Currently the card number is generated when the user enters the patron creation form. This creates a problem of concurrency - when two or more simulataneous users are registering members, the error "card no. in use" can occur. This change moves the card number generation to occur after the "Save" button is pressed. Changes: -C4/Members.pm: Added code to fixup_cardnumber,If the cardnumber is blank and "autoMemberNum" ON. -koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt: Added code to display "leave blank for auto calc during registration" in cardnumber label in patron registration form only if "autoMemberNum" ON. -members/memberentry.pl: Added code to get weather or not "autoMemberNum" is on or off and removed fixup_cardnumber generation. Test cases: -If "autoMemberNum" ON: ->In blank case, must generate auto card number in simulataneous users. ->If user entered, check for unique card number. -If "autoMemberNum" OFF: Must work normal.
Revived patch, tests will need to be provided for changes to C4::Members::AddMember, but everything is testable.
Created attachment 55964 [details] [review] Bug 6782 - Move auto member cardnumber generation to occur when record is "Saved" (avoid collisions). Currently the card number is generated when the user enters the patron creation form. This creates a problem of concurrency - when two or more simulataneous users are registering members, the error "card no. in use" can occur. This change moves the card number generation to occur after the "Save" button is pressed. Changes: -C4/Members.pm: Added code to fixup_cardnumber,If the cardnumber is blank and "autoMemberNum" ON. -koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt: Added code to display "leave blank for auto calc during registration" in cardnumber label in patron registration form only if "autoMemberNum" ON. -members/memberentry.pl: Added code to get weather or not "autoMemberNum" is on or off and removed fixup_cardnumber generation. Test cases: -If "autoMemberNum" ON: ->In blank case, must generate auto card number in simulataneous users. ->If user entered, check for unique card number. -If "autoMemberNum" OFF: Must work normal. Followed test plan, works as expected. Note: Syspref PorrowerMandatoryField must not include cardnumber, otherwise you can not save. Maybe that should be mentioned in the comment for syspref autoMemberNum. Signed-off-by: Marc Véron <veron@veron.ch>
QAers: See (potential) answers of bug 17215 comment 11.
Created attachment 58173 [details] [review] Bug 6782 - Move auto member cardnumber generation to occur when record is "Saved" (avoid collisions). Currently the card number is generated when the user enters the patron creation form. This creates a problem of concurrency - when two or more simulataneous users are registering members, the error "card no. in use" can occur. This change moves the card number generation to occur after the "Save" button is pressed. Changes: -C4/Members.pm: Added code to fixup_cardnumber,If the cardnumber is blank and "autoMemberNum" ON. -koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt: Added code to display "leave blank for auto calc during registration" in cardnumber label in patron registration form only if "autoMemberNum" ON. -members/memberentry.pl: Added code to get weather or not "autoMemberNum" is on or off and removed fixup_cardnumber generation. Test cases: -If "autoMemberNum" ON: ->In blank case, must generate auto card number in simulataneous users. ->If user entered, check for unique card number. -If "autoMemberNum" OFF: Must work normal. Followed test plan, works as expected. Note: Syspref PorrowerMandatoryField must not include cardnumber, otherwise you can not save. Maybe that should be mentioned in the comment for syspref autoMemberNum. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 58544 [details] [review] Bug 6782 - Move auto member cardnumber generation to occur when record is "Saved" (avoid collisions). Currently the card number is generated when the user enters the patron creation form. This creates a problem of concurrency - when two or more simulataneous users are registering members, the error "card no. in use" can occur. This change moves the card number generation to occur after the "Save" button is pressed. Changes: -C4/Members.pm: Added code to fixup_cardnumber,If the cardnumber is blank and "autoMemberNum" ON. -koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt: Added code to display "leave blank for auto calc during registration" in cardnumber label in patron registration form only if "autoMemberNum" ON. -members/memberentry.pl: Added code to get weather or not "autoMemberNum" is on or off and removed fixup_cardnumber generation. Test cases: -If "autoMemberNum" ON: ->In blank case, must generate auto card number in simulataneous users. ->If user entered, check for unique card number. -If "autoMemberNum" OFF: Must work normal. Followed test plan, works as expected. Note: Syspref PorrowerMandatoryField must not include cardnumber, otherwise you can not save. Maybe that should be mentioned in the comment for syspref autoMemberNum. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 58545 [details] [review] Bug 6782 [QA Followup] - Remove unused param and limit calls to Koha.Preference Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Pushed to master for 17.05, thanks Meenakshi!
This broke t/db_dependent/Reserves.t t/db_dependent/Reserves.t .. 1/72 Can't use an undefined value as a HASH reference at /home/vagrant/kohaclone/C4/Members.pm line 502. Please fix ASAP
(In reply to Jonathan Druart from comment #18) > This broke t/db_dependent/Reserves.t > > t/db_dependent/Reserves.t .. 1/72 Can't use an undefined value as a HASH > reference at /home/vagrant/kohaclone/C4/Members.pm line 502. > > Please fix ASAP And 17 other files.
Has broken tests and is an enhancement - not going to be in 16.11.02.
Created attachment 59002 [details] [review] Bug 6782: Fix fixup_cardnumber call The fixup_cardnumber subroutine takes only 1 parameter, the cardnumber. This call is wrong and morevover makes a lot of tests fail: t/db_dependent/Letters.t .. 1/79 Can't use an undefined value as a HASH reference at /home/vagrant/kohaclone/C4/Members.pm line 502. This happens because the userenv is not mocked in a lot of test files.
Followup pushed to master!