Bug 19209 - Koha::Objects should provide ->is_paged method
Summary: Koha::Objects should provide ->is_paged method
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 18731
  Show dependency treegraph
 
Reported: 2017-08-30 15:09 UTC by Tomás Cohen Arazi
Modified: 2017-10-03 08:24 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19209: Add ->is_paged method to Koha::Objects (2.11 KB, patch)
2017-08-30 15:14 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19209: (QA followup) Improve tests (2.28 KB, patch)
2017-08-30 15:32 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19209: Add ->is_paged method to Koha::Objects (2.16 KB, patch)
2017-08-30 16:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19209: (QA followup) Improve tests (2.34 KB, patch)
2017-08-30 16:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19209 [Followup] - Ensure checkouts don't prevent deleting patrons (919 bytes, patch)
2017-08-30 16:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19209: Add ->is_paged method to Koha::Objects (2.26 KB, patch)
2017-09-01 05:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19209: (QA followup) Improve tests (2.43 KB, patch)
2017-09-01 05:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19209 [Followup] - Ensure checkouts don't prevent deleting patrons (1016 bytes, patch)
2017-09-01 05:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2017-08-30 15:09:30 UTC
It is already implemented by DBIC resultsets and it should be a tiny addition to AUTOLOAD.
It would save several lines of coding when pagination is used.
Comment 1 Tomás Cohen Arazi 2017-08-30 15:14:13 UTC
Created attachment 66620 [details] [review]
Bug 19209: Add ->is_paged method to Koha::Objects

This patch adds ->is_paged to Koha::Objects. It is inherited from the underlying resultset
from DBIC so there's no code besides adding it to the known methods in AUTOLOAD.

Tests are added for the newly exported method.

To test:
- Apply this patch
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/db_dependent/Koha/Objects.t
=> SUCCESS: Tests pass!
- Sign off :-D

Sponsored-by: Camden County
Comment 2 Tomás Cohen Arazi 2017-08-30 15:32:40 UTC
Created attachment 66627 [details] [review]
Bug 19209: (QA followup) Improve tests

This path merges the pager() test and adds search results count tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Kyle M Hall 2017-08-30 16:07:05 UTC
Created attachment 66631 [details] [review]
Bug 19209: Add ->is_paged method to Koha::Objects

This patch adds ->is_paged to Koha::Objects. It is inherited from the underlying resultset
from DBIC so there's no code besides adding it to the known methods in AUTOLOAD.

Tests are added for the newly exported method.

To test:
- Apply this patch
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/db_dependent/Koha/Objects.t
=> SUCCESS: Tests pass!
- Sign off :-D

Sponsored-by: Camden County

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 4 Kyle M Hall 2017-08-30 16:07:12 UTC
Created attachment 66632 [details] [review]
Bug 19209: (QA followup) Improve tests

This path merges the pager() test and adds search results count tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Kyle M Hall 2017-08-30 16:07:15 UTC
Created attachment 66633 [details] [review]
Bug 19209 [Followup] - Ensure checkouts don't prevent deleting patrons

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Tomás Cohen Arazi 2017-08-30 21:00:21 UTC
(In reply to Kyle M Hall from comment #5)
> Created attachment 66633 [details] [review] [review]
> Bug 19209 [Followup] - Ensure checkouts don't prevent deleting patrons
> 
> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Thanks for the followup!
Comment 7 Marcel de Rooy 2017-09-01 05:49:16 UTC
QA: Looking here now
Comment 8 Marcel de Rooy 2017-09-01 05:57:29 UTC
Created attachment 66683 [details] [review]
Bug 19209: Add ->is_paged method to Koha::Objects

This patch adds ->is_paged to Koha::Objects. It is inherited from the underlying resultset
from DBIC so there's no code besides adding it to the known methods in AUTOLOAD.

Tests are added for the newly exported method.

To test:
- Apply this patch
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/db_dependent/Koha/Objects.t
=> SUCCESS: Tests pass!
- Sign off :-D

Sponsored-by: Camden County

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2017-09-01 05:57:34 UTC
Created attachment 66684 [details] [review]
Bug 19209: (QA followup) Improve tests

This path merges the pager() test and adds search results count tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2017-09-01 05:57:38 UTC
Created attachment 66685 [details] [review]
Bug 19209 [Followup] - Ensure checkouts don't prevent deleting patrons

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Marcel de Rooy 2017-09-01 05:58:19 UTC
Small remark:

+    is( ref($patrons->pager), 'DBIx::Class::ResultSet::Pager',
+       'Koha::Objects->pager returns a valid DBIx::Class object' );

If that is what we really want? Until now we wrapped lots of things in Koha objects.
Btw: pager returns a Data::Page object for the current resultset.
Comment 12 Tomás Cohen Arazi 2017-09-01 10:05:00 UTC
(In reply to Marcel de Rooy from comment #11)
> Small remark:
> 
> +    is( ref($patrons->pager), 'DBIx::Class::ResultSet::Pager',
> +       'Koha::Objects->pager returns a valid DBIx::Class object' );
> 
> If that is what we really want? Until now we wrapped lots of things in Koha
> objects.
> Btw: pager returns a Data::Page object for the current resultset.

yeah, that looked weird, but I'm not touching it besides moving the tests into another subtsests. It is probably worth discussing and eventually filling a new bug.
Comment 13 Jonathan Druart 2017-09-01 16:09:43 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 14 Fridolin SOMERS 2017-10-03 08:24:41 UTC
Enhancement not pushed to 17.05.x