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: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
: 28209 (view as bug list)
Depends on: 24446 27281
Blocks:
  Show dependency treegraph
 
Reported: 2021-04-23 15:46 UTC by Martin Renvoize
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


Attachments
Bug 28210: Unit test (1.72 KB, patch)
2021-04-23 15:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28210: Pass skip_record_index through LostItem (1.81 KB, patch)
2021-04-23 15:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28210: Unit test (1.78 KB, patch)
2021-04-26 15:58 UTC, Nick Clemens
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
Details | Diff | Splinter Review
Bug 28210: Pass skip_record_index through LostItem (1.87 KB, patch)
2021-04-26 15:58 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 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 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 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 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 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 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 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