[Bug 9457] New: Ordering branches should be case independent (2)
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9457 Bug ID: 9457 Summary: Ordering branches should be case independent (2) Classification: Unclassified Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Templates Assignee: bgkriegel@gmail.com Reporter: bgkriegel@gmail.com On the footsteps of Bug 5634. "Some libraries use lower case & upper case in branch description. The list is ordered case sensitively, it's hard to read." Use C4::Branch::GetBranchesLoop() to get an ordered list of branches. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9457 --- Comment #1 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 14787 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=14787&action=edit Bug 9457: [ENH] Ordering branches should be case independent (2) This patch replace use of CGI::scroll_list() to show list of branches. In two files, marc21_linking_section.pl and unimarc_field_4XX.pl, the scrolling list is created but not used in the template file, so the code is removed. Also minor renaming/normalizing of variables. To test: 1) Install with some branches, records and patrons 2.1) Select a record, click 'Place hold', select user, there is a library pull-down next to 'Pickup at:', list is ordered case sensitive 2.2) Go to Reports > Average loan time, next to Library is a pull-down, list without order 2.3) Go to Reports > Catalog by item type, next to 'Select a library' is a pull-down, list is ordered case sensitive 2.4) This is tricky, go to Reports home, change last part of URL 'reports-home.pl' with 'manager.pl?report_name=issues_by_borrower_category' (can't find a direct link), next to 'Select a library' is a library pull-down, list without order 2.5) Edit/Add a patron, on section 'Library management' there is a library pull-down, case sensitive 2.6) OPAC, as logged user, make a suggestion or hold, there is library pull-down, correct order 3) Apply the patch 4.1) Repeat 2.1), correctly ordered list 4.2) Repeat 2.2), correctly ordered list 4.3) Repeat 2.3), correctly ordered list 4.4) Repeat 2.4), correctly ordered list 4.5) This is a bit more work There are 3 possible situations to test: A) No branches, must show a message that are no libraries defined B) New patron, must show a correctly ordered list of branches, current branch selected C) Edit patron, must show a correctly ordered list of branches, patron branch selected 4.6) Small changes on variable names, so retest 2.6) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9457 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9457 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=5634 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9457 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #14787|0 |1 is obsolete| | --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 15184 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=15184&action=edit Bug 9457: [ENH] Ordering branches should be case independent (2) This patch replace use of CGI::scroll_list() to show list of branches. In two files, marc21_linking_section.pl and unimarc_field_4XX.pl, the scrolling list is created but not used in the template file, so the code is removed. Also minor renaming/normalizing of variables. To test: 1) Install with some branches, records and patrons 2.1) Select a record, click 'Place hold', select user, there is a library pull-down next to 'Pickup at:', list is ordered case sensitive 2.2) Go to Reports > Average loan time, next to Library is a pull-down, list without order 2.3) Go to Reports > Catalog by item type, next to 'Select a library' is a pull-down, list is ordered case sensitive 2.4) This is tricky, go to Reports home, change last part of URL 'reports-home.pl' with 'manager.pl?report_name=issues_by_borrower_category' (can't find a direct link), next to 'Select a library' is a library pull-down, list without order 2.5) Edit/Add a patron, on section 'Library management' there is a library pull-down, case sensitive 2.6) OPAC, as logged user, make a suggestion or hold, there is library pull-down, correct order 3) Apply the patch 4.1) Repeat 2.1), correctly ordered list 4.2) Repeat 2.2), correctly ordered list 4.3) Repeat 2.3), correctly ordered list 4.4) Repeat 2.4), correctly ordered list 4.5) This is a bit more work There are 3 possible situations to test: A) No branches, must show a message that are no libraries defined B) New patron, must show a correctly ordered list of branches, current branch selected C) Edit patron, must show a correctly ordered list of branches, patron branch selected 4.6) Small changes on variable names, so retest 2.6) Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9457 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |kyle@bywatersolutions.com --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- Bernardo++ for getting rid of CGI::scroll_list's -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9457 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |ASSIGNED CC| |jonathan.druart@biblibre.co | |m --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- QA comment: Great patch Bernardo! And great test plan too, it is really pleasant to test. I just found a little (big?) stuff. You forget to test (at several places) if C4::Context->userenv exists before to access to the 'branch' key. Sometimes this hashref (C4::Context->userenv) is not defined and the user get a software error. I think there is just one case: if the insecure syspref is turn ON. *But* I suppose this feature is broken, so are there actually someone using it? (I suppose that because of there are already several places where this test does not exist (e.g. reserve/request.pl)). So there are 2 possibilities: 1/ You send a mail to koha-devel in order to know if this feature can be deleted (in this case, we open a new report for that). 2/ You submit a followup taking account of this syspref. Switch the status to in discussion. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9457 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |In Discussion -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9457 --- Comment #5 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- (In reply to comment #4)
I just found a little (big?) stuff. You forget to test (at several places) if C4::Context->userenv exists before to access to the 'branch' key. Sometimes this hashref (C4::Context->userenv) is not defined and the user get a software error. I think there is just one case: if the insecure syspref is turn ON.
Good point. I was not aware of this syspref, and do not check that there is a possibility that userenv could return undef.
*But* I suppose this feature is broken, so are there actually someone using it? (I suppose that because of there are already several places where this test does not exist (e.g. reserve/request.pl)).
It is broken. After turned on, could not display staff login page. Must revert value on database.
So there are 2 possibilities: 1/ You send a mail to koha-devel in order to know if this feature can be deleted (in this case, we open a new report for that). 2/ You submit a followup taking account of this syspref.
I think is better to do both, to disentangle the feature and this bug. I'll send a mail to koha-devel and add a followup, just in case :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9457 --- Comment #6 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 16171 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16171&action=edit Bug 9457 - Followup - Ordering branches should be case independent (2) This follow up reinstates or add check for undef returned by C4::Context->userenv, only where previous patch touch code. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9457 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff --- Comment #7 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- I change this to Needs Signoff. Followup is small and only add 4 checks. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9457 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off Patch complexity|--- |Medium patch QA Contact| |jonathan.druart@biblibre.co | |m --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- QA comment: Last patch fixes the issue, thanks Bernardo. I followed the test plan and did not found any problem. Code looks good. Marked as Passed QA. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9457 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9457 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #15184|0 |1 is obsolete| | Attachment #16171|0 |1 is obsolete| | --- Comment #9 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 16209 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16209&action=edit Bug 9457: [ENH] Ordering branches should be case independent (2) This patch replace use of CGI::scroll_list() to show list of branches. In two files, marc21_linking_section.pl and unimarc_field_4XX.pl, the scrolling list is created but not used in the template file, so the code is removed. Also minor renaming/normalizing of variables. To test: 1) Install with some branches, records and patrons 2.1) Select a record, click 'Place hold', select user, there is a library pull-down next to 'Pickup at:', list is ordered case sensitive 2.2) Go to Reports > Average loan time, next to Library is a pull-down, list without order 2.3) Go to Reports > Catalog by item type, next to 'Select a library' is a pull-down, list is ordered case sensitive 2.4) This is tricky, go to Reports home, change last part of URL 'reports-home.pl' with 'manager.pl?report_name=issues_by_borrower_category' (can't find a direct link), next to 'Select a library' is a library pull-down, list without order 2.5) Edit/Add a patron, on section 'Library management' there is a library pull-down, case sensitive 2.6) OPAC, as logged user, make a suggestion or hold, there is library pull-down, correct order 3) Apply the patch 4.1) Repeat 2.1), correctly ordered list 4.2) Repeat 2.2), correctly ordered list 4.3) Repeat 2.3), correctly ordered list 4.4) Repeat 2.4), correctly ordered list 4.5) This is a bit more work There are 3 possible situations to test: A) No branches, must show a message that are no libraries defined B) New patron, must show a correctly ordered list of branches, current branch selected C) Edit patron, must show a correctly ordered list of branches, patron branch selected 4.6) Small changes on variable names, so retest 2.6) Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9457 --- Comment #10 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 16210 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16210&action=edit Bug 9457 - Followup - Ordering branches should be case independent (2) This follow up reinstates or add check for undef returned by C4::Context->userenv, only where previous patch touch code. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9457 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #11 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- This patch has been pushed to master. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org