It would be awesome if there were a batch patron edit tool that included the patron attributes in it. An example use of this would be for school libraries that keep track of home rooms as an attribute - that changes each year and it would be great to be able to batch edit that.
Created attachment 8687 [details] [review] But 5742: Batch edit patrons new permission : edit_patrons Plan test: - Go on the page: tools > Patrons modification (modborrowers.pl) - Enter a list of cardnumbers (or use a file) - Modify one or more patron's fields (you can modify surname, firstname, branchname, category, sort1, sort2, date of enrollment, date of expiry, debarred date, debarred comment and borrower note) - Save - Check on the result page (or in database for non-displayed fields) if modifications have been correctly made. - re test with different attributes. For each attributes filled with an authorized value category, you can select a value in a drop-down list. Else it's a free input text. If your attribute is filled with a patron category, the modification performs only on patrons belonging to this category - Save - Verify on the result page
I applied the patch and followed the plan test but I've got an error on step 1 by clicking on Patrons modification link (/cgi-bin/koha/tools/modborrowers.pl). The error message is below : "Software error: Type of arg 1 to values must be hash (not private variable) at /home/koha/src/tools/modborrowers.pl line 89, near "$patron_attribute_types ) " Execution of /home/koha/src/tools/modborrowers.pl aborted due to compilation errors." Tested on sandbox.
Created attachment 8696 [details] [review] But 5742 Batch edit patrons Oups, perl v5.10 don't like an hashref for values. (It's ok in 5.14)
Created attachment 8708 [details] [review] But 5742 Batch edit patrons - Adds few comments - Fix perlcritic for C4/Members/AttributeTypes.pm and C4/Members.pm - Removes MYSQLism - Fix crash if there is no patron attributes defined
Created attachment 8709 [details] [review] But 5742 Batch edit patrons - Adds few comments - Fix perlcritic for C4/Members/AttributeTypes.pm and C4/Members.pm - Removes MYSQLism - Fix crash if there is no patron attributes defined
i try to test the patch in a sandbox (number 1) but i have this message Applying: But 5742: Batch edit patrons Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Failed to merge in the changes. Patch failed at 0001 But 5742: Batch edit patrons
Created attachment 8801 [details] [review] But 5742 Batch edit patrons Rebased patch
I get an error when submitting card numbers, whether it be in a file or via the textarea: Can't use an undefined value as a HASH reference at /home/oleonard/kohaclone/tools/modborrowers.pl line 336
I have tested the patch in a sandbox and it works. With the scans of cardnumbers(textarea) or with a file that lists of cardnumbers 1/ update the borrower branch --> ok 2/ update the category --> ok 3/ changing the expiration date --> ok 4/ add a debarred date and a comment --> ok 5/ add a borrower note --> ok 6/ Delete the borrower note --> ok 7/ Add or replace one or more borrower attributes --> ok
Hi Owen, I can't reproduce this error. l.336 is: $borrower->{category_description} = GetBorrowercategory( $borrower->{categorycode} )->{description}; Is it possible you have a problem with your borrower's data ? $borrower must have a categorycode and GetBorrowercategory returns a hashref for this categorycode.
(In reply to comment #10) > Is it possible you have a problem with your borrower's data ? Indeed I did. I was working from a fresh install and I neglected to import my patron categories. Sorry for the false alarm. One concern I have: Should we even allow the librarian to empty the patron's home branch or category fields? This seems like a dangerous operation. I wonder if we shouldn't also prevent the user from emptying any fields which are specified in the BorrowerMandatoryField system preference?
one more system preference ?.... instead : it is possible to hide the opportunity to give clear in front the fields category and branch ?
Created attachment 8867 [details] [review] But 5742 Batch edit patrons Hi Owen, This new patch gets in the BorrowerMandatoryField system preference which fields are mandatory and disables the checkbox.
Created attachment 8876 [details] [review] [SIGN OFF] But 5742: Batch edit patrons new permission : edit_patrons Plan test: - Go on the page: tools > Patrons modification (modborrowers.pl) - Enter a list of cardnumbers (or use a file) - Modify one or more patron's fields (you can modify surname, firstname, branchname, category, sort1, sort2, date of enrollment, date of expiry, debarred date, debarred comment and borrower note) - Save - Check on the result page (or in database for non-displayed fields) if modifications have been correctly made. - re test with different attributes. For each attributes filled with an authorized value category, you can select a value in a drop-down list. Else it's a free input text. If your attribute is filled with a patron category, the modification performs only on patrons belonging to this category - Save - Verify on the result page Signed-off-by: Stephane Delaye <stephane.delaye@biblibre.com>
Hi, I was curious about the interface and did some tests. I have one suggestion about the sort1 and sort2 fields - some libraries use those as free text fields. The new tool will only let you change them when there is an authorized value connected to it. So it might be a nice addition to remove that limit and make it possible to empty/change the fields without a connected authorized value too. But that's not something that should stop this :)
Created attachment 8903 [details] [review] Bug 5742: Followup: sort* fields can to be filled with an input text Hello Katrin, Yes you are right, this followup fix this problem.
I have tested the patch in a sandbox and it don't works. Something went wrong ! Applying: But 5742: Batch edit patrons Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging C4/Members.pm CONFLICT (content): Merge conflict in C4/Members.pm Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt Failed to merge in the changes. Patch failed at 0001 But 5742: Batch edit patrons When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort". Bug 5742 - batch edit patrons
Created attachment 9149 [details] [review] [SIGNED OFF] But 5742: Batch edit patrons rebased patch
Created attachment 9150 [details] [review] Bug 5742: Followup: sort* fields can to be filled with an input text Rebased patch
I test the patch in a sandbox(1) but in the screen ../cgi-bin/koha/tools/tools-home.pl i don't see the link : ../cgi-bin/koha/tools/modborrowers.pl
i don't see the new permission
after test in a sand box i test the update of the attribute , the dateexpiry, debarrred, location etc , its ok i change
QA comment: * this patch contains also a lot of coding guidelines or PBP fixes, like - replacing tabs by spaces - avoiding things like "return undef" or my $x if $x eq 'condition'; That's a good thing, but the "usefull" code is harder to spot ! * Question = why do you define new subs like UpdateBorrowerAttribute, and don't export them ? I think you should (even if they are not used outside from the package for now, couldn't they be in the future ?) * tiny harmless typo "modifivation" instead of "modification" ;-) * modborrowers.pl passes perlcritic * the new template is valid => just answer my question (with a follow-up if applicable), and it will earn pass QA !
(In reply to comment #23) > QA comment: > * this patch contains also a lot of coding guidelines or PBP fixes, like > - replacing tabs by spaces > - avoiding things like "return undef" or my $x if $x eq 'condition'; > That's a good thing, but the "usefull" code is harder to spot ! > > * Question = why do you define new subs like UpdateBorrowerAttribute, and > don't export them ? I think you should (even if they are not used outside > from the package for now, couldn't they be in the future ?) OK, silly me ! they are. (note that this patch has been signed off by Mathilde during the hackfest, through sandboxes, I'll add his signoff on the patch itself, with the one from Stéphane)
QA comments (continued) * C4/Members passes perlcritic * tools/modborrowers.pl passes perlcritic prove xt/permissions.t fails = xt/permissions.t .. 1/? # Failed test 'No user permissions to add' # at xt/permissions.t line 95. # User permissions to add in uk-UA/mandatory/permissions_and_user_flags.sql: edit_patrons # Failed test 'No user permissions to add' # at xt/permissions.t line 95. # User permissions to add in ru-RU/mandatory/permissions_and_user_flags.sql: edit_patrons # Looks like you failed 2 tests of 34. => i've added a small follow-up to add the 2 permissions, now passes this test passed QA
This bug breaks members/memberentry.pl?op=add when you are using extended attributes since it displays all extended attributes available in database as opposed to empty input boxes for available attributes. Bug 8196 will contain fix for this problem.
There have been no further reports of problems so I am marking this bug resolved.