[Bug 29807] New: Branches template doesn't handle empty lists correctly
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 Bug ID: 29807 Summary: Branches template doesn't handle empty lists correctly Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Templates Assignee: oleonard@myacpl.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org The current code, given an 'item' or a 'biblio' param, makes 'pickup_locations' set @pickup_locations an empty list. After that, there's a check for emptiness, in which case it populates the @libraries variable with ALL PICKUP LOCATIONS, which is totally wrong! This is a bug introduced by bug 24350. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andreas.jonsson@kreablo.se --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- *** Bug 27970 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|oleonard@myacpl.org |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29806 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29806 [Bug 29806] ->pickup_locations should always be called in scalar context -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129056 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129056&action=edit Bug 29807: Regression tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129057 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129057&action=edit Bug 29807: Make Branches plugin handle empty pickup locations list This patch makes the plugin handle empty Koha::Biblio->pickup_locations and Koha::Item->pickup_locations correctly. It does so by explicitly calling ->as_list. It also restores the logic that was changed by 6cd1ffab4e491349c99769018a7df33dc8a8aabf so, now that ->empty is handled correctly, it doesn't return ALL the pickup locations when the item/biblio doesn't have valid pickup locations. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/Template/Plugin/Branches.t => FAIL: Awful error 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Branches template doesn't |Branches template plugin |handle empty lists |doesn't handle empty lists |correctly |correctly -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |The Branches TT plugin had release notes| |wrong logic in it, that | |made it crash, or display | |wrong pickup locations when | |the item/biblio didn't have | |any valid pickup location. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129056|0 |1 is obsolete| | Attachment #129057|0 |1 is obsolete| | --- Comment #4 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 129065 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129065&action=edit Bug 29807: Regression tests Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 --- Comment #5 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 129066 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129066&action=edit Bug 29807: Make Branches plugin handle empty pickup locations list This patch makes the plugin handle empty Koha::Biblio->pickup_locations and Koha::Item->pickup_locations correctly. It does so by explicitly calling ->as_list. It also restores the logic that was changed by 6cd1ffab4e491349c99769018a7df33dc8a8aabf so, now that ->empty is handled correctly, it doesn't return ALL the pickup locations when the item/biblio doesn't have valid pickup locations. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/Template/Plugin/Branches.t => FAIL: Awful error 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129065|0 |1 is obsolete| | --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 129080 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129080&action=edit Bug 29807: Regression tests Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129066|0 |1 is obsolete| | --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 129081 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129081&action=edit Bug 29807: Make Branches plugin handle empty pickup locations list This patch makes the plugin handle empty Koha::Biblio->pickup_locations and Koha::Item->pickup_locations correctly. It does so by explicitly calling ->as_list. It also restores the logic that was changed by 6cd1ffab4e491349c99769018a7df33dc8a8aabf so, now that ->empty is handled correctly, it doesn't return ALL the pickup locations when the item/biblio doesn't have valid pickup locations. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/Template/Plugin/Branches.t => FAIL: Awful error 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Status|Signed Off |Passed QA --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Clear correction of code, covered by tests.. no regressions, tests passing, qa script happy Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #9 from Fridolin Somers <fridolin.somers@biblibre.com> --- + } else { + @libraries = Koha::Libraries->search( { pickup_location => 1 }, { order_by => ['branchname'] } ) + unless @libraries; Do we still need "unless @libraries" ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 --- Comment #10 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to master for 22.05, thanks to everybody involved 🦄 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable CC| |kyle@bywatersolutions.com Version(s)|22.05.00 |22.05.00,21.11.03 released in| | --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to 21.11.x for 21.11.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|22.05.00,21.11.03 |22.05.00,21.11.03,21.05.11 released in| | Status|Pushed to stable |Pushed to oldstable CC| |andrew@bywatersolutions.com --- Comment #12 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Pushed to 21.05.x for 21.05.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29807 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED Resolution|--- |FIXED CC| |victor@tuxayo.net --- Comment #13 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org