Bug 18785

Summary: Add Koha::Subscription::biblio
Product: Koha Reporter: Julian Maurice <julian.maurice>
Component: Architecture, internals, and plumbingAssignee: Julian Maurice <julian.maurice>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, mtompset
Version: master   
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: 18426    
Attachments: Bug 18785: Add Koha::Subscription::biblio
Bug 18785: Force scalar context in Koha::Subscription::biblio
Bug 18785: Add the Koha::Subscription->biblio method
Bug 18785: Force scalar context in Koha::Subscription::biblio
Bug 18785: Add the Koha::Subscription->biblio method
Bug 18785: Force scalar context in Koha::Subscription::biblio

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!