Summary: | Branches template plugin doesn't handle empty lists correctly | ||
---|---|---|---|
Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Component: | Templates | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | andreas.jonsson, andrew, fridolin.somers, kyle, martin.renvoize, victor |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
The Branches TT plugin had wrong logic in it, that made it crash, or display wrong pickup locations when the item/biblio didn't have any valid pickup location.
|
Version(s) released in: |
22.05.00,21.11.03,21.05.11
|
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 29806 | ||
Attachments: |
Bug 29807: Regression tests
Bug 29807: Make Branches plugin handle empty pickup locations list Bug 29807: Regression tests Bug 29807: Make Branches plugin handle empty pickup locations list Bug 29807: Regression tests Bug 29807: Make Branches plugin handle empty pickup locations list |
Description
Tomás Cohen Arazi (tcohen)
2022-01-06 11:49:14 UTC
*** Bug 27970 has been marked as a duplicate of this bug. *** Created attachment 129056 [details] [review] Bug 29807: Regression tests Created attachment 129057 [details] [review] 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 Created attachment 129065 [details] [review] Bug 29807: Regression tests Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 129066 [details] [review] 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> Created attachment 129080 [details] [review] Bug 29807: Regression tests Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 129081 [details] [review] 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> Clear correction of code, covered by tests.. no regressions, tests passing, qa script happy Passing QA + } else { + @libraries = Koha::Libraries->search( { pickup_location => 1 }, { order_by => ['branchname'] } ) + unless @libraries; Do we still need "unless @libraries" ? Pushed to master for 22.05, thanks to everybody involved [U+1F984] Pushed to 21.11.x for 21.11.03 Pushed to 21.05.x for 21.05.11 Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed. |