Summary: | CanBookBeIssued must take a Koha::Patron in parameter | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | J.P.Knight, m.de.rooy, nick |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | 19276 | ||
Bug Blocks: | 20251 | ||
Attachments: |
Bug 19280: Pass a Koha::Patron to CanBookBeIssued
Bug 19280: Pass a Koha::Patron to CanBookBeIssued Bug 19280: Pass a Koha::Patron to CanBookBeIssued Bug 19280: Pass a Koha::Patron to CanBookBeIssued Bug 19280: refresh unblessed patron too Bug 19280: Pass a Koha::Patron to CanBookBeIssued Bug 19280: refresh unblessed patron too |
Description
Jonathan Druart
2017-09-08 15:53:14 UTC
Created attachment 66999 [details] [review] Bug 19280: Pass a Koha::Patron to CanBookBeIssued We need to make subroutine from C4 use more Koha::Object objects Seeing bug 19276, starting here is a good start. Test plan: The tests should still pass. Patch fails to apply on my Kohadevbox VM and also on the PTFS-E sandbox: vagrant@kohadevbox:kohaclone(master)$ git bz apply 19280 Bug 19280 - CanBookBeIssued must take a Koha::Patron in parameter 66999 - Bug 19280: Pass a Koha::Patron to CanBookBeIssued Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 19280: Pass a Koha::Patron to CanBookBeIssued /home/vagrant/kohaclone/.git/rebase-apply/patch:24: trailing whitespace. ( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $patron, fatal: sha1 information is lacking or useless (C4/Circulation.pm). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 19280: Pass a Koha::Patron to CanBookBeIssued The copy of the patch that failed is found in: /home/vagrant/kohaclone/.git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-19280-Pass-a-KohaPatron-to-CanBookBeIssued-qZZNx2.patch Created attachment 68533 [details] [review] Bug 19280: Pass a Koha::Patron to CanBookBeIssued We need to make subroutine from C4 use more Koha::Object objects Seeing bug 19276, starting here is a good start. Test plan: The tests should still pass. Created attachment 68534 [details] [review] Bug 19280: Pass a Koha::Patron to CanBookBeIssued We need to make subroutine from C4 use more Koha::Object objects Seeing bug 19276, starting here is a good start. Test plan: The tests should still pass. Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk> Created attachment 68916 [details] [review] Bug 19280: Pass a Koha::Patron to CanBookBeIssued We need to make subroutine from C4 use more Koha::Object objects Seeing bug 19276, starting here is a good start. Test plan: The tests should still pass. Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk> - $patron = Koha::Patrons->find( $borrower->{borrowernumber} ); + $patron = Koha::Patrons->find( $patron->borrowernumber ); # FIXME Refetch just in case, to avoid regressions. But must not be needed [...] my $toomany = TooMany( $patron_unblessed, $item->{biblionumber}, $item, { onsite_checkout => $onsite_checkout, switch_onsite_checkout => $switch_onsite_checkout, } ); You do not refetch $patron_unblessed. I would opt for not refetching $patron. But this is not consistent.. Created attachment 70287 [details] [review] Bug 19280: refresh unblessed patron too (In reply to Marcel de Rooy from comment #6) > - $patron = Koha::Patrons->find( $borrower->{borrowernumber} ); > + $patron = Koha::Patrons->find( $patron->borrowernumber ); # FIXME > Refetch just in case, to avoid regressions. But must not be needed > > [...] > > my $toomany = TooMany( $patron_unblessed, $item->{biblionumber}, $item, > { onsite_checkout => $onsite_checkout, switch_onsite_checkout => > $switch_onsite_checkout, } ); > > You do not refetch $patron_unblessed. I would opt for not refetching > $patron. But this is not consistent.. Good catch Marcel. I also think it's not needed, that's why I added the FIXME. But I would prefer not to do it on this bug report as it will need more attention. Created attachment 70291 [details] [review] Bug 19280: Pass a Koha::Patron to CanBookBeIssued We need to make subroutine from C4 use more Koha::Object objects Seeing bug 19276, starting here is a good start. Test plan: The tests should still pass. Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 70292 [details] [review] Bug 19280: refresh unblessed patron too Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Pushed to master for 18.05, thanks to everybody involved! Enhancement, skipping for 17.11.x. Awesome work everybody! |