[Bug 23830] New: Koha::AuthorisedValues should use Koha::Objects::Limit::Library
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23830 Bug ID: 23830 Summary: Koha::AuthorisedValues should use Koha::Objects::Limit::Library Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org CC: agustinmoyano@theke.io, fridolin.somers@biblibre.com, jonathan.druart@bugs.koha-community.org, katrin.fischer@bsz-bw.de, kyle@bywatersolutions.com, m.de.rooy@rijksmuseum.nl, martin.renvoize@ptfs-europe.com, tomascohen@gmail.com Depends on: 17755, 23272, 23281 Koha::Patron::Attribute::Types should be used as an example. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17755 [Bug 17755] Introduce Koha::Patron::Attribute::Type(s) https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23272 [Bug 23272] Koha::AuthorisedValue should use Koha::Object::Limit::Library https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23281 [Bug 23281] Add Koha::Objects::Limit::Library -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23830 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23830 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 115989 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115989&action=edit Bug 23830: Make Koha::AuthorisedValues use Koha::Objects::Limit::Library This patch makes Koha::AuthorisedValues inherit the 'search_with_library_limits' method from Koha::Objects::Limit::Library. This was left out of the original implementation and this patch makes this library have a similar implementation to that of Koha::ItemTypes, etc. To test: 1. Run: $ kshell k$ prove t/db_dependent/AuthorisedValues.t => SUCCESS: Tests pass! 2. Apply this patch 3. Notice Koha::AuthorisedValues now inherits this new library 4. Notice the overloaded ->search method is removed 5. Repeat (1) => SUCCESS: The behavior of the newly used method on the tests is the same as before. We only changed the method name and how we pass the library_id. 6. 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=23830 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 115990 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115990&action=edit Bug 23830: Adapt callers This patch is the result of making the same changes we did on the t/db_dependent/AuthorisedValues.t file (replacing the calls to Koha::AuthorisedValues->search with the tricky branchcode param, and call ->search_with_library_limits, with the library_id as a third parameter. What I did was: $ git grep 'Koha::AuthorisedValues\->search' and then revisited each of the grep results to check if they added the 'branchcode' parameter to the filters. This patch changes the calls to ->search, for ->search_with_library_limits in all the places that require it in the current codebase [1]. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Charges/Sales.t \ t/db_dependent/Ill* 3. Verify the batchmod.pl script is working and filtering the authorised values keeps working [1] Some places like the Koha/Template/Plugin/AuthorisedValues.pm plugin don't seem to be tested, at first glance. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23830 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23830 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #115989|0 |1 is obsolete| | --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 116185 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116185&action=edit Bug 23830: Make Koha::AuthorisedValues use Koha::Objects::Limit::Library This patch makes Koha::AuthorisedValues inherit the 'search_with_library_limits' method from Koha::Objects::Limit::Library. This was left out of the original implementation and this patch makes this library have a similar implementation to that of Koha::ItemTypes, etc. To test: 1. Run: $ kshell k$ prove t/db_dependent/AuthorisedValues.t => SUCCESS: Tests pass! 2. Apply this patch 3. Notice Koha::AuthorisedValues now inherits this new library 4. Notice the overloaded ->search method is removed 5. Repeat (1) => SUCCESS: The behavior of the newly used method on the tests is the same as before. We only changed the method name and how we pass the library_id. 6. Sign off :-D 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=23830 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #115990|0 |1 is obsolete| | --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 116186 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116186&action=edit Bug 23830: Adapt callers This patch is the result of making the same changes we did on the t/db_dependent/AuthorisedValues.t file (replacing the calls to Koha::AuthorisedValues->search with the tricky branchcode param, and call ->search_with_library_limits, with the library_id as a third parameter. What I did was: $ git grep 'Koha::AuthorisedValues\->search' and then revisited each of the grep results to check if they added the 'branchcode' parameter to the filters. This patch changes the calls to ->search, for ->search_with_library_limits in all the places that require it in the current codebase [1]. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Charges/Sales.t \ t/db_dependent/Ill* 3. Verify the batchmod.pl script is working and filtering the authorised values keeps working [1] Some places like the Koha/Template/Plugin/AuthorisedValues.pm plugin don't seem to be tested, at first glance. 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=23830 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Oooh, thanks for this Tomas.. I had entirely forgotten I'd reported this one! Code is trivial and good, all cases seem covered.. test suit passes, QA script passes (well, except for a false positive on POD removal). I think we can safely go for a straight to PQA on this one.! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23830 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23830 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23830 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 21.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=23830 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |caroline.cyr-la-rose@inlibr | |o.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23830 --- Comment #7 from Fridolin Somers <fridolin.somers@biblibre.com> --- Enhancement not pushed to 20.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23830 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This enhancement makes release notes| |Koha::AuthorisedValues | |inherit the | |'search_with_library_limits | |' method from | |Koha::Objects::Limit::Libra | |ry and thus makes it | |consistent with other | |locations where filtering | |by branch is required. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23830 --- Comment #8 from Jonathan Druart <jonathan.druart@gmail.com> --- - return Koha::AuthorisedValues->search({ - branchcode => $self->branchcode, - category => 'ILLSTATUS', - authorised_value => $self->SUPER::status_alias - })->next; + return Koha::AuthorisedValues->search( + { + category => 'ILLSTATUS', + authorised_value => $self->SUPER::status_alias + }, + {}, + $self->branchcode + )->next; This change is wrong: 1. left-over $self->branchcode 2. we forgot to use search_with_library_limits -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org