[Bug 30526] New: The pagination in the REST API does not work if $c->objects->search is called multiple times.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30526 Bug ID: 30526 Summary: The pagination in the REST API does not work if $c->objects->search is called multiple times. Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: andreas.jonsson@kreablo.se CC: tomascohen@gmail.com The REST-plugin Koha::REST::Plugin::Pagination defines two methods: dbic_merge_pagination and add_pagination_headers. The method add_pagination_headers use the helper subroutine _build_link. The last call in add_pagination_headers looks like this: push @links, _build_link( $c, { page => $pages, per_page => $per_page, rel => 'last', params => $args->{params} } ); Here is _build_link: sub _build_link { my ( $c, $args ) = @_; my $params = $args->{params}; $params->{_page} = $args->{page}; $params->{_per_page} = $args->{per_page}; Note that the original parameters _page and _per_page is overwritten here. This means that _page will be $pages. Subsequent calls to $c->objects->search will, thus, always return the last page. At least REST::V1::Holds::pickup_locations is affected by this (because a redundant call to $c->objects->search is made if AllowHoldPolicyOverride is activated). This causes the dropdown menu with pickup locations in the holds view to only contain the last page of libraries if there are more than 20 pickup libraries and AllowHoldPolicyOverride is activated. -- 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=30526 --- Comment #1 from Andreas Jonsson <andreas.jonsson@kreablo.se> --- Created attachment 133262 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133262&action=edit Bug 30526: Copy parameters when building links in rest api. -- 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=30526 Andreas Jonsson <andreas.jonsson@kreablo.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff --- Comment #2 from Andreas Jonsson <andreas.jonsson@kreablo.se> --- Test plan: * Make sure you have at least 3 libraries that may be pickup locations. * Activate the system preference AllowHoldPolicyOverride * Change the system preference RESTdefaultPageSize to 1 * Go to a record and make a place a hold. * In the Holds view for the record, verify that all libraries appear in the pickup library drop down menu on the hold. -- 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=30526 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Andreas Jonsson from comment #2)
Test plan:
* Make sure you have at least 3 libraries that may be pickup locations. * Activate the system preference AllowHoldPolicyOverride * Change the system preference RESTdefaultPageSize to 1 * Go to a record and make a place a hold. * In the Holds view for the record, verify that all libraries appear in the pickup library drop down menu on the hold.
Hi, nice catch and really happy to see your patch. I suggest you take a look at this files: - t/Koha/REST/Plugin/Pagination.t - t/db_dependent/api/v1/pagination.t in those files, you should add regression tests at different levels. In case you need it, ping me on IRC (tcohen) or just drop an email to koha-devel. -- 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=30526 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Status|Needs Signoff |Failed QA --- Comment #4 from David Nind <david@davidnind.com> --- Patch still applies. Changed status to Failed QA - see comments from Tomás in comment #3 Following the test plan, I noticed no difference in either of the dropdown lists (hold details area and place a hold on a specific item) when placing a hold - both before and after the patch is applied. I even added some additional libraries so that there were 22 where holds could be palced. -- 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=30526 --- Comment #5 from Andreas Jonsson <andreas.jonsson@kreablo.se> --- It seems that the issue at hand have been accidentally fixed. Let me propose a simpler test plan: * Open up Koha/REST/Plugin/Query.pm and look at the subroutine _build_link * Observe that the subroutine overwrites $args->{params}->{_page} and $args->{params}->{_per_page} * Ponder for a second over how one can reasonable expect that these parameters remain unchanged for the duration of the request. * Ponder for a second over how this unexpected modification of the parameters might have caused the issue at hand, how it might be causing other undiscovered issues and how it may cause issues with code yet to be written. * Ponder for a second over how no sane programmer could have written any code which, outside of the subroutine _build_link, relies on this unexpected update of the parameters. * Ponder for a second over how the proposed patch unburdens you from these concerns. -- 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=30526 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=30526 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |DUPLICATE Status|Failed QA |RESOLVED --- Comment #6 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Fixed by bug 40417. *** This bug has been marked as a duplicate of bug 40417 *** -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org