Bug 19209

Summary: Koha::Objects should provide ->is_paged method
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: fridolin.somers, jonathan.druart, kyle, m.de.rooy
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 18731    
Attachments: Bug 19209: Add ->is_paged method to Koha::Objects
Bug 19209: (QA followup) Improve tests
Bug 19209: Add ->is_paged method to Koha::Objects
Bug 19209: (QA followup) Improve tests
Bug 19209 [Followup] - Ensure checkouts don't prevent deleting patrons
Bug 19209: Add ->is_paged method to Koha::Objects
Bug 19209: (QA followup) Improve tests
Bug 19209 [Followup] - Ensure checkouts don't prevent deleting patrons

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