Bug 18785 - Add Koha::Subscription::biblio
Summary: Add Koha::Subscription::biblio
Status: CLOSED 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: Julian Maurice
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 18426
  Show dependency treegraph
 
Reported: 2017-06-12 11:14 UTC by Julian Maurice
Modified: 2018-06-04 20:18 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 18785: Add Koha::Subscription::biblio (2.49 KB, patch)
2017-06-12 11:15 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 18785: Force scalar context in Koha::Subscription::biblio (738 bytes, patch)
2017-08-09 09:33 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 18785: Add the Koha::Subscription->biblio method (2.57 KB, patch)
2017-09-26 18:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18785: Force scalar context in Koha::Subscription::biblio (811 bytes, patch)
2017-09-26 18:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18785: Add the Koha::Subscription->biblio method (2.61 KB, patch)
2017-10-07 00:27 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18785: Force scalar context in Koha::Subscription::biblio (861 bytes, patch)
2017-10-07 00:27 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2017-06-12 11:14:46 UTC

    
Comment 1 Julian Maurice 2017-06-12 11:15:14 UTC
Created attachment 64197 [details] [review]
Bug 18785: Add Koha::Subscription::biblio
Comment 2 Mark Tompsett 2017-07-01 00:58:50 UTC
Comment on attachment 64197 [details] [review]
Bug 18785: Add Koha::Subscription::biblio

Review of attachment 64197 [details] [review]:
-----------------------------------------------------------------

::: Koha/Subscription.pm
@@ +44,5 @@
> +
> +sub biblio {
> +    my ($self) = @_;
> +
> +    return Koha::Biblios->find($self->biblionumber);

There are a bunch of other bugzilla issues related to ->find and a scalar context.
Perhapts actually using ->find({ biblionumber => $self->biblionumber }); or
I think others get lazy ->find( scalar $self->biblionumber )... or was it
return scalar Koha::Biblios->find($self->biblionumber);
Hmmm... anyways, this isn't right, as far as I can tell.
Plus if it is singular, then why Biblios? This is the part I don't understand in this and other code.
Comment 3 Julian Maurice 2017-07-03 09:59:37 UTC
(In reply to M. Tompsett from comment #2)
> There are a bunch of other bugzilla issues related to ->find and a scalar
> context.
> Perhapts actually using ->find({ biblionumber => $self->biblionumber }); or
> I think others get lazy ->find( scalar $self->biblionumber )... or was it
> return scalar Koha::Biblios->find($self->biblionumber);
> Hmmm... anyways, this isn't right, as far as I can tell.
I assume you are talking about bug 18179, but the patch I wrote doesn't make any call to 'find' in a list context. Plus, there is a check in Koha::Objects::find that prevents it to be used in a list context, so I think we're good. Callers of Koha::Subsbcription::biblio will be the ones to have to make sure it's called in scalar context.

> Plus if it is singular, then why Biblios? This is the part I don't
> understand in this and other code.
The 'find' subroutine is defined in Koha::Objects, not Koha::Object.
Comment 4 Jonathan Druart 2017-07-13 21:16:20 UTC
Julian you must force the context to make sure the following code will not explode:

  my $s = Koha::Subscriptions->find(42);
  $template->param( biblio => $s->biblio );

Cannot use "->find" in list context at /home/vagrant/kohaclone/Koha/Subscription.pm line 48.
Comment 5 Julian Maurice 2017-08-09 09:33:38 UTC
Created attachment 65700 [details] [review]
Bug 18785: Force scalar context in Koha::Subscription::biblio
Comment 6 Jonathan Druart 2017-09-26 18:40:10 UTC
Created attachment 67383 [details] [review]
Bug 18785: Add the Koha::Subscription->biblio method

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Jonathan Druart 2017-09-26 18:40:14 UTC
Created attachment 67384 [details] [review]
Bug 18785: Force scalar context in Koha::Subscription::biblio

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Nick Clemens 2017-10-07 00:27:07 UTC
Created attachment 67764 [details] [review]
Bug 18785: Add the Koha::Subscription->biblio method

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 9 Nick Clemens 2017-10-07 00:27:10 UTC
Created attachment 67765 [details] [review]
Bug 18785: Force scalar context in Koha::Subscription::biblio

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 10 Jonathan Druart 2017-10-09 19:18:12 UTC
Pushed to master for 17.11, thanks to everybody involved!