Bug 24448

Summary: Add Koha::Biblio->subscriptions_count
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: 1joynelson, agustinmoyano, jonathan.druart, josef.moravec, katrin.fischer, kyle, liz, martin.renvoize, nick, nicolas.legrand, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24435
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00
Bug Depends on:    
Bug Blocks: 20212, 24467    
Attachments: Bug 24448: Unit tests
Bug 24448: Add Koha::Biblio->subscriptions_count
Bug 24448: Unit tests
Bug 24448: Add Koha::Biblio->subscriptions_count
Bug 24448: Unit tests
Bug 24448: Add Koha::Biblio->subscriptions_count
Bug 24448: Adding a notice to tell the _count methods are temporary

Description Tomás Cohen Arazi 2020-01-17 14:17:18 UTC

    
Comment 1 Tomás Cohen Arazi 2020-01-17 14:25:56 UTC
Created attachment 97525 [details] [review]
Bug 24448: Unit tests

This patch adds tests for the new method to be introduced. It also moves
the tests for Koha::Biblio->subscriptions to the Biblio.t where it
really belongs.

To test, run this tests along with the patch implementing the new
method.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi 2020-01-17 14:25:59 UTC
Created attachment 97526 [details] [review]
Bug 24448: Add Koha::Biblio->subscriptions_count

This patch adds the handy subscriptions_count method to the Koha::Biblio
class. It is intended to be used as a shortcut for

    $biblio->subscriptions->count

in places like the API where we can embed the output of a method.

To test:
1. Apply this patches
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Biblio.t \
           t/db_dependent/Koha/Biblios.t
=> SUCCESS: Tests pass! i.e. Biblios.t still passes, and the tests for
the new method in Biblio.t pass as well.
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Nick Clemens 2020-01-17 20:28:11 UTC
Created attachment 97571 [details] [review]
Bug 24448: Unit tests

This patch adds tests for the new method to be introduced. It also moves
the tests for Koha::Biblio->subscriptions to the Biblio.t where it
really belongs.

To test, run this tests along with the patch implementing the new
method.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 4 Nick Clemens 2020-01-17 20:28:14 UTC
Created attachment 97572 [details] [review]
Bug 24448: Add Koha::Biblio->subscriptions_count

This patch adds the handy subscriptions_count method to the Koha::Biblio
class. It is intended to be used as a shortcut for

    $biblio->subscriptions->count

in places like the API where we can embed the output of a method.

To test:
1. Apply this patches
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Biblio.t \
           t/db_dependent/Koha/Biblios.t
=> SUCCESS: Tests pass! i.e. Biblios.t still passes, and the tests for
the new method in Biblio.t pass as well.
3. Sign off :-D

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 5 Jonathan Druart 2020-01-21 09:38:57 UTC
IMO we should not have ->subscriptions_count, but callers should ->subscriptions->count instead. What's the point of this?
Comment 6 Katrin Fischer 2020-01-21 09:40:42 UTC
(In reply to Jonathan Druart from comment #5)
> IMO we should not have ->subscriptions_count, but callers should
> ->subscriptions->count instead. What's the point of this?

I have to admit I was a bit confused by this too - can you explain Tomas?
Comment 7 Tomás Cohen Arazi 2020-01-21 11:04:18 UTC
The short explanation is that we developed a way to embed things from the API using HTTP headers, and it can embed methods output. Take a look at this branch:

https://gitlab.com/thekesolutions/Koha/tree/bug_20212_slowness_acq

We could develop a way to specify we want to count things from the HTTP headers, but it is kind of overkill only for the sake of avoiding this simple method.
Comment 8 Katrin Fischer 2020-01-21 12:48:12 UTC
(In reply to Tomás Cohen Arazi from comment #7)
> The short explanation is that we developed a way to embed things from the
> API using HTTP headers, and it can embed methods output. Take a look at this
> branch:
> 
> https://gitlab.com/thekesolutions/Koha/tree/bug_20212_slowness_acq
> 
> We could develop a way to specify we want to count things from the HTTP
> headers, but it is kind of overkill only for the sake of avoiding this
> simple method.

Sorry, not understanding it yet :(

What's the difference between 

subscriptions->count 
subscriptions->subscription_count

in terms of the API? The first looks more standardized to me?
Comment 9 Katrin Fischer 2020-01-21 13:09:09 UTC
> 
> What's the difference between 
> 
> subscriptions->count 
> subscriptions->subscription_count
> 
> in terms of the API? The first looks more standardized to me?

Oh!.... this is a method in Biblio - that makes more sense now!
Comment 10 Tomás Cohen Arazi 2020-01-21 13:10:03 UTC
I will comment on koha-devel.
Comment 11 Jonathan Druart 2020-01-28 14:06:47 UTC
Created attachment 98033 [details] [review]
Bug 24448: Unit tests

This patch adds tests for the new method to be introduced. It also moves
the tests for Koha::Biblio->subscriptions to the Biblio.t where it
really belongs.

To test, run this tests along with the patch implementing the new
method.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 12 Jonathan Druart 2020-01-28 14:06:52 UTC
Created attachment 98034 [details] [review]
Bug 24448: Add Koha::Biblio->subscriptions_count

This patch adds the handy subscriptions_count method to the Koha::Biblio
class. It is intended to be used as a shortcut for

    $biblio->subscriptions->count

in places like the API where we can embed the output of a method.

To test:
1. Apply this patches
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Biblio.t \
           t/db_dependent/Koha/Biblios.t
=> SUCCESS: Tests pass! i.e. Biblios.t still passes, and the tests for
the new method in Biblio.t pass as well.
3. Sign off :-D

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2020-01-28 14:06:56 UTC
Created attachment 98036 [details] [review]
Bug 24448: Adding a notice to tell the _count methods are temporary

See bug 24467. You are allowed to flog tcohen with an RJ45 cable if this
is not fixed in the next months.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 14 Martin Renvoize 2020-01-28 15:06:45 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 15 Joy Nelson 2020-02-10 22:37:57 UTC
Enhancement not backported to 19.11.x