Bug 24448 - Add Koha::Biblio->subscriptions_count
Summary: Add Koha::Biblio->subscriptions_count
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: Testopia
URL:
Keywords:
Depends on:
Blocks: 20212 24467
  Show dependency treegraph
 
Reported: 2020-01-17 14:17 UTC by Tomás Cohen Arazi
Modified: 2020-02-10 22:37 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00


Attachments
Bug 24448: Unit tests (3.48 KB, patch)
2020-01-17 14:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 24448: Add Koha::Biblio->subscriptions_count (1.41 KB, patch)
2020-01-17 14:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 24448: Unit tests (3.54 KB, patch)
2020-01-17 20:28 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24448: Add Koha::Biblio->subscriptions_count (1.47 KB, patch)
2020-01-17 20:28 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24448: Unit tests (3.61 KB, patch)
2020-01-28 14:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24448: Add Koha::Biblio->subscriptions_count (1.54 KB, patch)
2020-01-28 14:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24448: Adding a notice to tell the _count methods are temporary (890 bytes, patch)
2020-01-28 14:06 UTC, Jonathan Druart
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 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