[Koha-bugs] [Bug 9805] Lost items are un-lost if returned, but not if renewed

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

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

--- Comment #50 from Jonathan Druart <jonathan.druart at bugs.koha-community.org> ---
(In reply to Kyle M Hall from comment #48)
> (In reply to Jonathan Druart from comment #46)
> > (In reply to Kyle M Hall from comment #45)
> > > Created attachment 49165 [details] [review] [review] [review]
> > > Bug 9805 [QA Followup] - Stop calling GetMember repeatedly
> > 
> > Kyle, This change looks safe, but it's very hard to say it won't introduce a
> > regression. The $borrower var is passed from a sub to another without being
> > "refreshed" like it was before this patch. This means we could use an
> > outdated data.
> 
> I do not believe this is a valid concern. There is no reason to suspect
> there could be any problems caused by this. If we force each subroutine in
> Koha to fetch all db data with each call, we will continue to make Koha
> slower and slower. If you have a specific scenario in mind where this will
> cause an issue, I'm certainly open to amending my position. Otherwise I we
> should keep this patch to improve speed and efficiency.

Of course it's a valid concern :)
It's the same problematic we have for caching.
I don't suggest to "fetch all db data with each call", but we need to take care
of the obsolescence of the data we manipulate.
You are fetching the patron info using GetMemberDetails, which returns
accounting data, but AddRenewal modify these infos. At some point your
$borrower is obsolete. For instance if SendCirculationAlert (which does not
refetch patron infos) needs to access the accounting infos to sent the letter
and decide to get if from $borrower, we will have a very hard to debug issue.

It would make more sense to have this kind of patches on a separate bug report,
not to block the integration of this bugfix into master.

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