Summary: | Import patrons form should have drop-downs for some values | ||
---|---|---|---|
Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Component: | Tools | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | abl, nengard, sandboxes |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 12859, 12918, 12929 | ||
Attachments: |
Bug 12716: Allow the import patrons form have drop-downs and datepickers
Bug 12716: unit tests for GetColumnDefs Bug 12716: Allow the import patrons form have drop-downs and datepickers Bug 12716: unit tests for GetColumnDefs [PASSED QA] Bug 12716: Allow the import patrons form have drop-downs and datepickers [PASSED QA] Bug 12716: unit tests for GetColumnDefs |
Description
Tomás Cohen Arazi (tcohen)
2014-08-04 12:37:32 UTC
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! |