[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]

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

Marcel de Rooy <m.de.rooy at rijksmuseum.nl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Signed Off                  |Failed QA

--- Comment #24 from Marcel de Rooy <m.de.rooy at rijksmuseum.nl> ---
(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.

> $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.

> Image::Magick, on the other hand, does not automatically "destroy" the
> original structure on each call, and so we need to take care of that.[2]
Fine with me.

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