Bug 17766 - Patron notification does not work with multi item holds
Summary: Patron notification does not work with multi item holds
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 14695 14685
Blocks:
  Show dependency treegraph
 
Reported: 2016-12-13 12:51 UTC by Benjamin Rokseth
Modified: 2017-06-14 22:17 UTC (History)
6 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:


Attachments
Bug 17766 - Patron notification does not work with multi item holds (5.58 KB, patch)
2016-12-13 14:01 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 17766 - Patron notification does not work with multi item holds (5.64 KB, patch)
2016-12-30 17:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 17766 - Patron notification does not work with multi item holds (5.68 KB, patch)
2017-01-13 11:29 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Rokseth 2016-12-13 12:51:56 UTC
Bug 14685 introduces multi item holds on same record, but notifications triggered by ModReserveAffect does not honour multiple this. When a patron has multiple reserves on same record, notifications will only pick the first, as the database query only uses biblionumber and borrowernumber.

_koha_notify_reserve in C4::Reserves is only used once anyway, so it should probably be removed, but that is another bug.
Comment 1 Benjamin Rokseth 2016-12-13 14:01:34 UTC
Created attachment 58158 [details] [review]
Bug 17766 - Patron notification does not work with multi item holds

This patch fixes notification when same biblio has multiple reserves with same borrower,
introduced in Bug 14695. C4::Reserves::ModReserveAffect uses internal method
_koha_notify_reserve but sends itemnumber and biblionumber instead of record_id.

To test:
Prerequisites:
- One biblio with two items attached
- A patron with hold_filled notification activated
- A letter for HOLD with <<reserves.reserve_id>> in it
1) Place two reservations on same biblio
2) checkin item x on pickup branch, observe patron message generated
3) checkin item y on pickup branch, observe patron message generated
4) note that reserve_id is the same on both messages, which is wrong
5) apply this patch and repeat 1-3
6) now observe notifications have correct (different) reserve_id
Comment 2 Hugo Agud 2016-12-30 12:38:41 UTC
checked and working fine!
Comment 3 Kyle M Hall 2016-12-30 17:57:35 UTC
Created attachment 58536 [details] [review]
Bug 17766 - Patron notification does not work with multi item holds

This patch fixes notification when same biblio has multiple reserves with same borrower,
introduced in Bug 14695. C4::Reserves::ModReserveAffect uses internal method
_koha_notify_reserve but sends itemnumber and biblionumber instead of record_id.

To test:
Prerequisites:
- One biblio with two items attached
- A patron with hold_filled notification activated
- A letter for HOLD with <<reserves.reserve_id>> in it
1) Place two reservations on same biblio
2) checkin item x on pickup branch, observe patron message generated
3) checkin item y on pickup branch, observe patron message generated
4) note that reserve_id is the same on both messages, which is wrong
5) apply this patch and repeat 1-3
6) now observe notifications have correct (different) reserve_id

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 4 Kyle M Hall 2017-01-13 11:29:19 UTC
Created attachment 58948 [details] [review]
Bug 17766 - Patron notification does not work with multi item holds

This patch fixes notification when same biblio has multiple reserves with same borrower,
introduced in Bug 14695. C4::Reserves::ModReserveAffect uses internal method
_koha_notify_reserve but sends itemnumber and biblionumber instead of record_id.

To test:
Prerequisites:
- One biblio with two items attached
- A patron with hold_filled notification activated
- A letter for HOLD with <<reserves.reserve_id>> in it
1) Place two reservations on same biblio
2) checkin item x on pickup branch, observe patron message generated
3) checkin item y on pickup branch, observe patron message generated
4) note that reserve_id is the same on both messages, which is wrong
5) apply this patch and repeat 1-3
6) now observe notifications have correct (different) reserve_id

Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Kyle M Hall 2017-01-13 11:31:12 UTC
Pushed to master for 17.05, thanks Benjamin!
Comment 6 Katrin Fischer 2017-01-15 16:37:03 UTC
This patch has been pushed to 16.11.x and will be in 16.11.02.
Comment 7 Mason James 2017-01-25 00:44:35 UTC
Blocked by new feature, skipping for 16.05.x