Categorycode and branches should be selectable drop downs.
Created attachment 30557 [details] [review] Bug 12716: Allow the import patrons form have drop-downs and datepickers To reproduce: - Go to Tools > Import patrons - Notice branchcode and categorycode don't have dropdown menus with valid options. - Notice dateofbirth, dateenrolled and dateexpiry don't have date-pickers - Notice each input field is labeled with the column name instead of a proper description. To test: - Apply the patch, go to Tools > Import patrons - Notice branchcode and categorycode have dropdown menus with valid options, and by default the empty value is selected. - Notice that if you go into the dateofbirth, dateenrolled and dateexpiry fields, a nice date picker widget appears. - Notice each field is labeled with the field description, and that the column name is conveniently shown at the right of the input field. Important: test switching the ExtendedPatronAttributes syspref, and verify that if enabled, the patron_attributes field appears, and also the checkbox selector controlling the extended patron attributes import behaviour shows too. Bonus points: Verify that on a different language, the descriptions get translated, and the column names show correctly Regards To+
Created attachment 30558 [details] [review] Bug 12716: unit tests for GetColumnDefs The introduced function gets tested.
Patch tested with a sandbox, by Aleisha <aleishaamohia@hotmail.com>
Created attachment 30728 [details] [review] Bug 12716: Allow the import patrons form have drop-downs and datepickers To reproduce: - Go to Tools > Import patrons - Notice branchcode and categorycode don't have dropdown menus with valid options. - Notice dateofbirth, dateenrolled and dateexpiry don't have date-pickers - Notice each input field is labeled with the column name instead of a proper description. To test: - Apply the patch, go to Tools > Import patrons - Notice branchcode and categorycode have dropdown menus with valid options, and by default the empty value is selected. - Notice that if you go into the dateofbirth, dateenrolled and dateexpiry fields, a nice date picker widget appears. - Notice each field is labeled with the field description, and that the column name is conveniently shown at the right of the input field. Important: test switching the ExtendedPatronAttributes syspref, and verify that if enabled, the patron_attributes field appears, and also the checkbox selector controlling the extended patron attributes import behaviour shows too. Bonus points: Verify that on a different language, the descriptions get translated, and the column names show correctly Regards To+ Signed-off-by: Aleisha <aleishaamohia@hotmail.com>
Created attachment 30729 [details] [review] Bug 12716: unit tests for GetColumnDefs The introduced function gets tested. Signed-off-by: Aleisha <aleishaamohia@hotmail.com>
Comment on attachment 30728 [details] [review] Bug 12716: Allow the import patrons form have drop-downs and datepickers Review of attachment 30728 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt @@ +3,5 @@ > <title>Koha › Tools › Import patrons [% IF ( uploadborrowers ) %]› Results[% END %]</title> > [% INCLUDE 'doc-head-close.inc' %] > <style type="text/css"> > + .yui-u fieldset.rows .widelabel { width: 12em; } > + label.description { width: 20em; } I'd like this a bit better in the staff CSS file, so people could overwrite it with OpacUserCSS - different language might need more or less space there. But not a blocker. @@ +133,5 @@ > + <option value="" selected="selected"></option> > + [% FOREACH branch IN branches %] > + <option value="[% branch.branchcode %]"> > + [% branch.branchname %]</option> > + [% END %] I think you could have used the TT plugin for branches here :)
Created attachment 30816 [details] [review] [PASSED QA] Bug 12716: Allow the import patrons form have drop-downs and datepickers To reproduce: - Go to Tools > Import patrons - Notice branchcode and categorycode don't have dropdown menus with valid options. - Notice dateofbirth, dateenrolled and dateexpiry don't have date-pickers - Notice each input field is labeled with the column name instead of a proper description. To test: - Apply the patch, go to Tools > Import patrons - Notice branchcode and categorycode have dropdown menus with valid options, and by default the empty value is selected. - Notice that if you go into the dateofbirth, dateenrolled and dateexpiry fields, a nice date picker widget appears. - Notice each field is labeled with the field description, and that the column name is conveniently shown at the right of the input field. Important: test switching the ExtendedPatronAttributes syspref, and verify that if enabled, the patron_attributes field appears, and also the checkbox selector controlling the extended patron attributes import behaviour shows too. Bonus points: Verify that on a different language, the descriptions get translated, and the column names show correctly Regards To+ Signed-off-by: Aleisha <aleishaamohia@hotmail.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes tests and QA script. Works as described, no regressions found. Dates are output in ISO/database format, this is ok.
Created attachment 30817 [details] [review] [PASSED QA] Bug 12716: unit tests for GetColumnDefs The introduced function gets tested. Signed-off-by: Aleisha <aleishaamohia@hotmail.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Patches pushed to master.
It may be something specific to my test setup, but I've just tried to test-import ~30k patron records with lots and lots of extended attributes - there are no errors reported, but none of those attributes are getting added to the database. I guess that removal of this code part: -if ($extended) { - push @columnkeys, 'patron_attributes'; -} around line 65 in tools/import_borrowers.pl possibly does have some unforseen side effects, resulting in all extended attributes being ignored during import?
Jaceck, I don't think it is related to that specifically, but please fill a bug so we dig a bit.
Jacek, could you add an example line with the syntax you used in the import file too?
(In reply to Katrin Fischer from comment #12) > Jacek, could you add an example line with the syntax you used in the import > file too? While I encountered this when trying to import patrons with somewhat more complex entries in 'patron_attibutes' CSV field, eg.: """DPCODE:PK-B"",""HISCHANNR:B/000123; E/000123; ^/000123"",""KNTPAP:BPK"",""KNTPAP:BWM"",""PESEL:1234567890"",""SHOW_BCODE:1""" extended atributes import seems to be broken as well for more trivial cases - I did some tests in clean master with simpler 'patron_attributes'; same outcome. Also, on the practical side, re-adding aforementioned 'if' solved the problem, at least for me ;). This script (tools/import_patrons.pl) is not exactly the easiest one to analyze comprehensivelly, but it seems that 'patron_attributes' was artifically added to @columnkeys as pseudo-colum name (= a kludge, as there is no such column in borrowers table); when that pseudo column is no longer in @columnkeys, apparently extended attributes are not being picked up at all from the relevant CSV field (?). Follow up & proposed patch: in Bug 12859.
(In reply to Jacek Ablewicz from comment #13) > > This script (tools/import_patrons.pl) is not exactly the easiest one to > analyze comprehensivelly, but it seems that 'patron_attributes' was > artifically added to @columnkeys as pseudo-colum name (= a kludge, as there > is no such column in borrowers table); when that pseudo column is no longer > in @columnkeys, apparently extended attributes are not being picked up at > all from the relevant CSV field (?). > > Follow up & proposed patch: in Bug 12859. Well done Jaceck! Thanks a lot!