Bug 29094 - Placing holds via SIP2 does not check if a patron can hold the given item
Summary: Placing holds via SIP2 does not check if a patron can hold the given item
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-09-23 12:20 UTC by Kyle M Hall
Modified: 2023-06-08 22:26 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes holds placed using SIP2 to check that the patron can actually place a hold for the item.
Version(s) released in:
22.11.00, 22.05.05


Attachments
Bug 29094: Adding hold via SIP should check if patron can hold item first (5.31 KB, patch)
2021-09-23 13:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 29094: Adding hold via SIP should check if patron can hold item first (5.30 KB, patch)
2022-06-27 11:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 29094: Adding hold via SIP should check if patron can hold item first (5.34 KB, patch)
2022-06-27 14:13 UTC, David Nind
Details | Diff | Splinter Review
Bug 29094: Adding hold via SIP should check if patron can hold item first (5.41 KB, patch)
2022-07-14 09:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29094: Adding hold via SIP should check if patron can hold item first [21.11.x] (5.42 KB, patch)
2022-08-26 14:53 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 Kyle M Hall 2021-09-23 12:20:19 UTC
When placing holds via SIP2, there is no holdability check. This seems very incorrect.
Comment 1 Kyle M Hall 2021-09-23 13:07:04 UTC
Created attachment 125189 [details] [review]
Bug 29094: Adding hold via SIP should check if patron can hold item first

When placing holds via SIP2, there is no holdability check. This seems very incorrect.

Test Plan:
1) Apply this patch
2) prove -r t/db_dependent/SIP
Comment 2 David Nind 2022-06-18 22:45:12 UTC
The patch no longer applies 8-(..

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 29094: Adding hold via SIP should check if patron can hold item first
Using index info to reconstruct a base tree...
M	t/db_dependent/SIP/ILS.t
M	t/db_dependent/SIP/Transaction.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/SIP/Transaction.t
CONFLICT (content): Merge conflict in t/db_dependent/SIP/Transaction.t
Auto-merging t/db_dependent/SIP/ILS.t
error: Failed to merge in the changes.
Patch failed at 0001 Bug 29094: Adding hold via SIP should check if patron can hold item first
Comment 3 Kyle M Hall 2022-06-27 11:31:08 UTC
Created attachment 136570 [details] [review]
Bug 29094: Adding hold via SIP should check if patron can hold item first

When placing holds via SIP2, there is no holdability check. This seems very incorrect.

Test Plan:
1) Apply this patch
2) prove -r t/db_dependent/SIP
Comment 4 David Nind 2022-06-27 14:13:08 UTC
Created attachment 136611 [details] [review]
Bug 29094: Adding hold via SIP should check if patron can hold item first

When placing holds via SIP2, there is no holdability check. This seems very incorrect.

Test Plan:
1) Apply this patch
2) prove -r t/db_dependent/SIP

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Martin Renvoize 2022-07-14 09:55:08 UTC
Created attachment 137706 [details] [review]
Bug 29094: Adding hold via SIP should check if patron can hold item first

When placing holds via SIP2, there is no holdability check. This seems very incorrect.

Test Plan:
1) Apply this patch
2) prove -r t/db_dependent/SIP

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2022-07-14 09:55:44 UTC
Thanks Kyle,

Works as described, contains unit tests and QA scripts are all happy.

Passing QA
Comment 7 Tomás Cohen Arazi 2022-07-18 13:31:23 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 8 Lucas Gass 2022-08-23 17:14:19 UTC
Backported to 22.05.x for 22.05.05
Comment 9 Arthur Suzuki 2022-08-26 11:58:30 UTC
Patch applies with conflicts + tests failing.
Please provide a patch for 21.11 if needed.

"
#   Failed test 'do_hold'
#   at t/db_dependent/SIP/Transaction.t line 272.
DBIx::Class::ResultSource::_minimal_valueset_satisfying_constraint(): Unable to satisfy requested constraint 'primary', missing values for column(s): 'itemnumber' at /kohadevbox/koha/Koha/Objects.pm line 96
# Looks like your test exited with 11 just after 7.
Dubious, test returned 11 (wstat 2816, 0xb00)
Failed 6/12 subtests 
"
Comment 10 Kyle M Hall 2022-08-26 14:53:45 UTC
Created attachment 139830 [details] [review]
Bug 29094: Adding hold via SIP should check if patron can hold item first [21.11.x]

When placing holds via SIP2, there is no holdability check. This seems very incorrect.

Test Plan:
1) Apply this patch
2) prove -r t/db_dependent/SIP

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>