Summary: | Add Koha::Subscription::biblio | ||
---|---|---|---|
Product: | Koha | Reporter: | Julian Maurice <julian.maurice> |
Component: | Architecture, internals, and plumbing | Assignee: | 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
Created attachment 64197 [details] [review] Bug 18785: Add Koha::Subscription::biblio 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. (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. 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. Created attachment 65700 [details] [review] Bug 18785: Force scalar context in Koha::Subscription::biblio Created attachment 67383 [details] [review] Bug 18785: Add the Koha::Subscription->biblio method Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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> 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> 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> Pushed to master for 17.11, thanks to everybody involved! |