[Bug 22688] New: TT plugin for piickup locations code wrong
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Bug ID: 22688 Summary: TT plugin for piickup locations code wrong Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com Target Milestone: --- I haven't spent the time to determine the consequences, but bug 7614 got wrong at some point on rebasing and this code looks plain wrong, looking at the patches history in bugzilla it seems that the lines prefixed with minus sign need to be removed: sub pickup_locations { my ( $self, $params ) = @_; - $params->{search_params} ||= {}; - $params->{search_params}->{pickup_location} = 1; - return $self->all($params); my $selected = $params->{selected}; my $libraries = Koha::Libraries->pickup_locations($params); for my $l (@$libraries) { if ( defined $selected and $l->{branchcode} eq $selected or not defined $selected and C4::Context->userenv and $l->{branchcode} eq C4::Context->userenv->{branch} ) { $l->{selected} = 1; } } return $libraries; } -- 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=22688 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | CC| |agustinmoyano@theke.io Depends on| |7614 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7614 [Bug 7614] Use branch transfer limits for determining available opac holds pickup locations -- 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=22688 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|TT plugin for piickup |TT plugin for pickup |locations code wrong |locations code wrong CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 87764 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87764&action=edit Bug 22688: Unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 87765 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87765&action=edit Bug 22688: TT plugin for pickup locations code wrong Test Plan: 1) Apply this patch 2) prove t/db_dependent/Koha/Libraries.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Kyle M Hall <kyle@bywatersolutions.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=22688 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87764|0 |1 is obsolete| | --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 87766 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87766&action=edit Bug 22688: Unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87766|0 |1 is obsolete| | --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 87768 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87768&action=edit Bug 22688: Unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial 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=22688 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87765|0 |1 is obsolete| | --- Comment #5 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 87772 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87772&action=edit Bug 22688: TT plugin for pickup locations code wrong Test Plan: 1) Apply this patch 2) prove t/db_dependent/Koha/Libraries.t Signed-off-by: Agustín Moyano <agustinmoyano@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87768|0 |1 is obsolete| | --- Comment #6 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 87773 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87773&action=edit Bug 22688: Unit tests Signed-off-by: Agustín Moyano <agustinmoyano@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Kyle, it seems to me that the test case you picked is the one that doesn't test pickup limitations (compared to all()). I think we should test: - Passing a selected => branchcode, then if selected => 1 is added to the hashref - Using mock_userenv({ branchcode => $library->branchcode }) to test the case in which C4::Context->userenv is used to pick the 'selected' library. I confess I spend a good amount of time today trying to implement a fully mocked test, only to learn that there's a problem with the 'selected' param because it would be passed all the way down to Koha::Libraries->search... This got into a rabbit hole (writing this tests). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Status|Failed QA |In Discussion -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87772|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87773|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 87817 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87817&action=edit Bug 22688: Unit tests Signed-off-by: Agustín Moyano <agustinmoyano@theke.io> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 87818 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87818&action=edit Bug 22688: (follow-up) Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 87819 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87819&action=edit Bug 22688: TT plugin for pickup locations code wrong Test Plan: 1) Apply this patch 2) prove t/db_dependent/Koha/Libraries.t Signed-off-by: Agustín Moyano <agustinmoyano@theke.io> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 87820 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87820&action=edit Bug 22688: (QA follow-up) Use search_params as all() does It looks like selected would be passed all the way down to Koha::Libraries->search and this would fail all the time. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I decided to submit what I had done. But it needs your review, Kyle, and maybe the mocked tests are not enough. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- It is such a tiny thing, used in a single place... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |22284 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22284 [Bug 22284] Groups of pickup locations for holds -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87817|0 |1 is obsolete| | Attachment #87818|0 |1 is obsolete| | Attachment #87819|0 |1 is obsolete| | Attachment #87820|0 |1 is obsolete| | --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 87884 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87884&action=edit Bug 22688: Unit tests Signed-off-by: Agustín Moyano <agustinmoyano@theke.io> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87884|0 |1 is obsolete| | --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 87885 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87885&action=edit Bug 22688: Unit tests Signed-off-by: Agustín Moyano <agustinmoyano@theke.io> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 87886 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87886&action=edit Bug 22688: (follow-up) Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 87887 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87887&action=edit Bug 22688: TT plugin for pickup locations code wrong Test Plan: 1) Apply this patch 2) prove t/db_dependent/Koha/Libraries.t Signed-off-by: Agustín Moyano <agustinmoyano@theke.io> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 --- Comment #18 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 87888 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87888&action=edit Bug 22688: (QA follow-up) Use search_params as all() does It looks like selected would be passed all the way down to Koha::Libraries->search and this would fail all the time. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Tomás Cohen Arazi <tomascohen@gmail.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=22688 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com Status|Passed QA |Pushed to Master --- Comment #19 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 19.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |19.05.00, 18.11.05 released in| | Status|Pushed to Master |Pushed to Stable CC| |martin.renvoize@ptfs-europe | |.com --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Pushed to 18.11.x for 18.11.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22688 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #21 from Lucas Gass <lucas@bywatersolutions.com> --- missing dependencies for 18.05.x, not backporting -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org