[Koha-bugs] [Bug 23112] Circulation of ILL items

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23112

--- Comment #46 from Andrew Isherwood <andrew.isherwood at ptfs-europe.com> ---
(In reply to Jonathan Druart from comment #45)
> Comment on attachment 95566 [details] [review]
> Bug 23112: Add circulation to ILL requests
> 
> Review of attachment 95566 [details] [review]:
> -----------------------------------------------------------------
> 
> Also, I think you should use patron-title.inc to display patron's info.

Aha, good spot, thanks! I'm about to add a patch to address that.

> 
> ::: Koha/Illrequest.pm
> @@ +1211,5 @@
> > +        # For some reason, AddIssue requires an unblessed Patron
> > +        $issue_args[0] = $patron->unblessed;
> > +        my $issue = C4::Circulation::AddIssue(@issue_args);
> > +
> > +        if ($issue && %{$issue}) {
> 
> AddIssue return a Koha::Checkout object. The test will work but I think it's
> confusing.

Do you mean the variable name $issue is confusing? If so, I thought it made
sense since the function returning it is called AddIssue and, internally,
AddIssue also calls the returned value $issue.

> 
> ::: koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt
> @@ +704,4 @@
> >      </script>
> >      [% INCLUDE 'ill-list-table-strings.inc' %]
> >      [% Asset.js("js/ill-list-table.js") | $raw %]
> > +    [% Asset.js("js/ill-check-out.js") | $raw %]
> 
> Where does this file come from?

Another good spot, this include was leftover junk from an earlier attempt at
this bug. About to attach a patch that removes this include.

-- 
You are receiving this mail because:
You are watching all bug changes.