Bug 8700 - RESERVESLIP fields not being replaced correctly
Summary: RESERVESLIP fields not being replaced correctly
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low normal (vote)
Target Milestone: ---
Assignee: Kyle M Hall
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-29 12:44 UTC by Kyle M Hall
Modified: 2013-05-23 06:24 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 8700 - RESERVESLIP fields not being replaced correctly (1.36 KB, patch)
2012-08-29 12:45 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8700 - RESERVESLIP fields not being replaced correctly (1.52 KB, patch)
2012-09-21 05:31 UTC, Marc Véron
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2012-08-29 12:44:15 UTC
Tthe slip RESERVESLIP is not replacing fields correctly. C4::Reserves::ReserveSlip calls C4::Letters::GetPreparedLetter, and passes the $reserve hashref to it for each table except branches ( which is passed the branchcode ). The problem is, if you pass a hashref for a table, it uses that hashref for the replacing, rather than looking up the data from the database.
Comment 1 Kyle M Hall 2012-08-29 12:45:24 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2012-09-14 14:04:50 UTC
Can you please add a test plan for this?
Comment 3 Kyle M Hall 2012-09-18 14:31:50 UTC
Test plan:
1) Edit the slip RESERVESLIP, set it to the following:

borrowers.title: <<borrowers.title>>
<br/>
biblio.copyrightdate: <<biblio.copyrightdate>>
<br/>
items.replacementprice: <<items.replacementprice>>

2) Place a reserve for a borrower. Make sure that borrower has a borrowers.title value in the database ( I prefer "King of the World" ), as well as biblio.copyrightdate, and items.replacementprice

3) Return the item, confirm hold and print slip. The slip will probably show the *item's* title in borrowers.title. It did for me. The other two will probably by blank.

4) Apply the patch, repeat step 2 and 3, the values should now be correct and accurate.
Comment 4 Marc Véron 2012-09-21 05:31:14 UTC
Created attachment 12406 [details] [review]
[SIGNED-OFF] Bug 8700 - RESERVESLIP fields not being replaced correctly

The slip RESERVESLIP is not replacing fields correctly.
C4::Reserves::ReserveSlip calls C4::Letters::GetPreparedLetter,
and passes the $reserve hashref to it for each table except branches
( which is passed the branchcode ). The problem is, if you pass a
hashref for a table, it uses that hashref for the replacing, rather
than looking up the data from the database.

Fixed by passing the correct keys for each of the tables requested.

Signed-off-by: Marc Veron <veron@veron.ch>

Tested following the test plan.
Could reproduce the bug.
After applying the patch slip printed as expected.
Comment 5 David Cook 2012-09-26 19:27:05 UTC
I just want to add kudos for Kyle and Marc. I just had the same issue come up, so it's nice to see that someone has already taken care of it :)
Comment 6 Paul Poulain 2012-10-10 15:59:15 UTC
QA comment
 * tiny & trivial (from a QA POV) patch

passed QA
Comment 7 Paul Poulain 2012-10-10 15:59:29 UTC
Patch pushed to master
Comment 8 Chris Cormack 2012-10-10 21:32:57 UTC
Pushed to 3.8.x, will be in 3.8.6