It is impossible to print the expiration date on a hold slip (HOLD_SLIP) if the patron has more than one hold on that bib. The expirationdate comes back blank.
I have confirmed by looking at [% hold.reserve_id %] that HOLD_SLIP is looking at the wrong hold record.
If I assign <<reserves.expirationdate>> to another variable, for some reason, [% hold.expirationdate %] works. Very wonky behavior.
Created attachment 98738 [details] [review] Bug 24612: Make hold-transfer-slip take reserve_id To make sure we are going to display the correct hold's info we need to pass the reserve_id.
I think this patch will fix the issue you have. Can you confirm? hold skip needs to be tested from return.pl (checkin) and circulation.pl (checkout)
Created attachment 98739 [details] [review] Bug 24612: Make hold-transfer-slip take reserve_id To make sure we are going to display the correct hold's info we need to pass the reserve_id.
Christopher, I'm not able to replicate this in Master. I tried to confirm the behavior before applying the patch and haven't gotten the expiration date to come up null. Can you please confirm the problem still exists and/or provide steps to replicate it?
(In reply to Andrew Fuerste-Henry from comment #6) > Christopher, I'm not able to replicate this in Master. I tried to confirm > the behavior before applying the patch and haven't gotten the expiration > date to come up null. Can you please confirm the problem still exists and/or > provide steps to replicate it? I was able to replicate is this way: 1. In notices and slips, add [% hold.reserve_id %] to your hold slip. 2. Place 2 bib level holds for the same patron on a bib. 3. Check in one of the items and click print slip. 4. Observe that the slip doesn't have an expiration date at all. 5. Cancel the waiting hold. 6. Check in the other item and click print slip. 7. Observe the expiration date on the slip.
I've tested and I can't get the patch to solve the problem. This is what I did: TEST PLAN: 1. Add some content to HOLD_SLIP notice, e.g. <h2>[% branch.branchname %]</h2> <div>[% biblio.author %]<br>[% biblio.title %]<br>[% item.barcode %] <ul><li> Reserve ID: [% hold.reserve_id %]</li> <li>Expiration date: [% hold.expirationdate %]</li></ul> 2. Add 2 holds for 1 patron to a single record 3. Check the reserve IDs in the reserves table - on a clean sandbox, they will be 1 and 2 4. Check in one of the items from the record and print the slip 5. Note that the reserve ID on the slip is 2 and the expiration date is blank 6. Repeated check ins do not change this 7. Check in a second item from the record 8. Note that the reserve ID for this hold is also 2, but this time the expiration date is filled in 9. Check in the first item again - the reserve ID stays as 2, but this time the expiration date is filled in I repeated this several times, and the odd numbered holds (1, 3, 5, 7 etc) never appear - but the even ones appear for all items (2, 4, 6, 8). I set the bug as Failed QA because it doesn't seem to solve the problem - is that the correct status? Or should I have chosen Patch Doesn't Apply?
Created attachment 102818 [details] [review] Bug 24612: Make hold-transfer-slip take reserve_id To make sure we are going to display the correct hold's info we need to pass the reserve_id. See comment 7 and 8 for test plans
Created attachment 102819 [details] [review] Bug 24612: Add tests ReserveSlip call need to be adjusted in test. We also add a new reserve (same biblio, same patron) to highlight the problem.
Created attachment 102820 [details] [review] Bug 24612: Use the reserve_id to identify a reserve when building a notice Now that we have the reserve_id PK on the reserves table we should use it (instead of the couple borrowernumber, biblionumber)
(In reply to Sally from comment #8) > I've tested and I can't get the patch to solve the problem. I think it's now fixed with the last patchset! > I set the bug as Failed QA because it doesn't seem to solve the problem - is > that the correct status? Or should I have chosen Patch Doesn't Apply? "Failed QA" is the correct status when the patches do no work as you expected.
Created attachment 104803 [details] [review] Bug 24612: Make hold-transfer-slip take reserve_id To make sure we are going to display the correct hold's info we need to pass the reserve_id. == Test plan == 1. Add some content to HOLD_SLIP notice, e.g. <h2>[% branch.branchname %]</h2> <div>[% biblio.author %]<br>[% biblio.title %]<br>[% item.barcode %] <ul><li> Reserve ID: [% hold.reserve_id %]</li> <li>Expiration date: [% hold.expirationdate %]</li></ul> 2. Add 2 holds for 1 patron to a single record 3. Check the reserve IDs in the reserves table - on a clean sandbox, they will be 1 and 2 4. Check in one of the items from the record and print the slip 5. Note that the reserve ID on the slip is 2 and the expiration date is blank 6. Repeated check ins do not change this 7. Check in a second item from the record 8. Note that the reserve ID for this hold is also 2, but this time the expiration date is filled in 9. Check in the first item again - the reserve ID stays as 2, but this time the expiration date is filled in 10. Apply patch 11. cancel the holds to come back to a clean state (and maybe ensure items aren't in transit) 12. redo the test and see the following differences 13. 1st checkin: 1. expiration date ok 2. the reserve ID is the one of the first hold 14. 2nd checkin: 1. expiration date ok 2. the reserve ID is the one of the second hold Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 104804 [details] [review] Bug 24612: Add tests ReserveSlip call need to be adjusted in test. We also add a new reserve (same biblio, same patron) to highlight the problem. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 104805 [details] [review] Bug 24612: Use the reserve_id to identify a reserve when building a notice Now that we have the reserve_id PK on the reserves table we should use it (instead of the couple borrowernumber, biblionumber) Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
It works! (an so the automated tests) thanks Jonathan. :D Thanks Lisette and Sally for the test plan.
Created attachment 105202 [details] [review] Bug 24612: Make hold-transfer-slip take reserve_id To make sure we are going to display the correct hold's info we need to pass the reserve_id. == Test plan == 1. Add some content to HOLD_SLIP notice, e.g. <h2>[% branch.branchname %]</h2> <div>[% biblio.author %]<br>[% biblio.title %]<br>[% item.barcode %] <ul><li> Reserve ID: [% hold.reserve_id %]</li> <li>Expiration date: [% hold.expirationdate %]</li></ul> 2. Add 2 holds for 1 patron to a single record 3. Check the reserve IDs in the reserves table - on a clean sandbox, they will be 1 and 2 4. Check in one of the items from the record and print the slip 5. Note that the reserve ID on the slip is 2 and the expiration date is blank 6. Repeated check ins do not change this 7. Check in a second item from the record 8. Note that the reserve ID for this hold is also 2, but this time the expiration date is filled in 9. Check in the first item again - the reserve ID stays as 2, but this time the expiration date is filled in 10. Apply patch 11. cancel the holds to come back to a clean state (and maybe ensure items aren't in transit) 12. redo the test and see the following differences 13. 1st checkin: 1. expiration date ok 2. the reserve ID is the one of the first hold 14. 2nd checkin: 1. expiration date ok 2. the reserve ID is the one of the second hold Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 105203 [details] [review] Bug 24612: Add tests ReserveSlip call need to be adjusted in test. We also add a new reserve (same biblio, same patron) to highlight the problem. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 105204 [details] [review] Bug 24612: Use the reserve_id to identify a reserve when building a notice Now that we have the reserve_id PK on the reserves table we should use it (instead of the couple borrowernumber, biblionumber) Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Nice work everyone! Pushed to master for 20.05
Created attachment 105253 [details] [review] Bug 24612: Fix existing ReserveSlip tests
Created attachment 105254 [details] [review] Bug 24612: Fix existing ReserveSlip tests
Created attachment 105775 [details] [review] Bug 24612: Fix existing ReserveSlip tests
backported to 19.11.x for 19.11.07
Patch a bit too large to warrant a backport to oldoldstable (19.05.x) considering it's severity is only of level "normal" And there is a non-trivial conflict when cherry-picking.
*** Bug 14099 has been marked as a duplicate of this bug. ***