Bug 28210 - C4::Circulation::LostItem should pass through skip_record_index to MarkIssueReturned
Summary: C4::Circulation::LostItem should pass through skip_record_index to MarkIssueR...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Martin Renvoize (ashimema)
QA Contact: Testopia
URL:
Keywords:
: 28209 (view as bug list)
Depends on: 24446 27281
Blocks:
  Show dependency tree / graph
 
Reported: 2021-04-23 15:46 UTC by Martin Renvoize (ashimema)
Modified: 2021-12-13 21:12 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.05.00
Circulation function:

Attachments
Bug 28210: Unit test (1.72 KB, patch)
2021-04-23 15:54 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 28210: Pass skip_record_index through LostItem (1.81 KB, patch)
2021-04-23 15:54 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 28210: Unit test (1.78 KB, patch)
2021-04-26 15:58 UTC, Nick Clemens (kidclamp) 		Details | Diff | Splinter Review
Bug 28210: Add issue, replacementprice, and set onloan so check-in tests work (2.97 KB, patch)
2021-04-26 15:58 UTC, Nick Clemens (kidclamp) 		Details | Diff | Splinter Review
Bug 28210: Pass skip_record_index through LostItem (1.87 KB, patch)
2021-04-26 15:58 UTC, Nick Clemens (kidclamp) 		Details | Diff | Splinter Review
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2021-04-23 15:46:38 UTC 
With Bug 27281 we removed a superflous call to update the homebranch of items in transfer.

However, this highlighted a test that needed to be removed and lead to the removal of the 'skip_record_index' parameter from the method signature.

This lead Nick to spot that we never passed the parameter on to MarkIssueAsReturned when we should..

This bug will add back the parameter in LostItem, add a test for the MarkIssueReturned case and start passing the parameters through properly.
Comment 1 Martin Renvoize (ashimema) 2021-04-23 15:54:22 UTC 
Created attachment 120118 [details] [review]
Bug 28210: Unit test

Add a unit test to check that skip_record_index is adhered to when
passed to LostItem with the force_mark_returned option enabled.
Comment 2 Martin Renvoize (ashimema) 2021-04-23 15:54:26 UTC 
Created attachment 120119 [details] [review]
Bug 28210: Pass skip_record_index through LostItem

This patch restores the skip_record_index optional parameter for
LostItem and passes it through to MarkIssueReturned as required.
Comment 3 Jonathan Druart 2021-04-26 14:48:50 UTC 
*** Bug 28209 has been marked as a duplicate of this bug. ***
Comment 4 Nick Clemens (kidclamp) 2021-04-26 15:58:34 UTC 
Created attachment 120179 [details] [review]
Bug 28210: Unit test

Add a unit test to check that skip_record_index is adhered to when
passed to LostItem with the force_mark_returned option enabled.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 5 Nick Clemens (kidclamp) 2021-04-26 15:58:37 UTC 
Created attachment 120180 [details] [review]
Bug 28210: Add issue, replacementprice, and set onloan so check-in tests work

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 6 Nick Clemens (kidclamp) 2021-04-26 15:58:40 UTC 
Created attachment 120181 [details] [review]
Bug 28210: Pass skip_record_index through LostItem

This patch restores the skip_record_index optional parameter for
LostItem and passes it through to MarkIssueReturned as required.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Nick Clemens (kidclamp) 2021-04-26 16:00:19 UTC 
I added some data needed to make the tests pass, can you verify Martin?

For testing this also requires applying the follow-ups on 24446 and 27281

While we remove/readd some code in the series I think it makes sense to remove it to fix the tests, then add it as it should have been here
Comment 8 Jonathan Druart 2021-04-27 09:42:09 UTC 
Pushed to master for 21.05, thanks to everybody involved!
Comment 9 Fridolin Somers 2021-04-30 08:29:08 UTC 
Depends on Bug 24446 not in 20.11.x