[Koha-bugs] [Bug 8604] Patron cards made for patrons which don't have patron images use preceding card's image

bugzilla-daemon at bugs.koha-community.org bugzilla-daemon at bugs.koha-community.org
Mon Jul 16 16:43:30 CEST 2018


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

--- Comment #30 from Chris Nighswonger <cnighswonger at foundations.edu> ---
(In reply to Marcel de Rooy from comment #24)
> (In reply to Chris Nighswonger from comment #22)
> > (In reply to Marcel de Rooy from comment #19)
> > > Chris:
> > > I have the impression
> > 
> > Just an impression? ;-) Did you test and find a case where this fix does not
> > work?
> Please keep in mind that we both have the same goal: fix bugs and improve
> code. QAing is more than testing. A careful look at the code can in my
> experience reveal bugs just as easy or show you what to test.

No offense intended. I understand that we have the same goal. I appreciate your
contribution to the QA process. Please keep in mind that my time is extremely
limited. Much more so than devs who are paid to work on Koha as I am paid to
work on other projects. When you provide QA feedback without a supporting
proof-case, it means that the dev has to spend time ensuring 1. your
observation is valid and 2. Ensuring that the fix does not cause other
problems. Time is expensive to be spending on an impression without
accompanying demonstrable failure cases.

> 
> > $binary_data is a reference to $image_data->{'imagefile'} which, in turn, is
> > a reference to a hash created by DBI containing the results of the SELECT.
> > DBI creates a new hash on every row retrieval[1], ensuring that the data
> > contained at our reference is fresh every time.
> The crux is here indeed. This is only partially true. The code does only
> update $image_data in two of the if statement branches. So my observation
> and request is just to clear $image_data each time in the loop or put its
> definition into the loop. Clearing the other variables might be unneeded
> since they are created in the loop.
> So this reveals a test case: Make sure that record 1 in the loop is a patron
> image and record 2 comes in the eq 'none' branch. Record 2 will use the old
> value of $image_data.

FTR: I have no problem undefing the entire $image_data reference. As soon as I
have time, I will submit a follow-up patch. I also inadvertently removed the
statements which short-circuit the loop in the case of no image data. That
needs to be corrected as well.

That said: Your suggested case does not cause the failure you suggest. I
understand that $image_data never drops out of scope during the loop execution.
I understand that in theory the reference can contain "stale" data. I just have
not been handed and cannot establish a test case where it does.

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


More information about the Koha-bugs mailing list