Bug 35292 - Define itemtype specific rules in the UpdateNotForLoanStatusOnCheckOut system preference
Summary: Define itemtype specific rules in the UpdateNotForLoanStatusOnCheckOut system...
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-11-08 16:11 UTC by Lisette Scheer
Modified: 2025-01-13 10:47 UTC (History)
4 users (show)

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


Attachments
Bug 35292: Database changes (6.75 KB, patch)
2024-12-04 16:38 UTC, Baptiste Wojtkowski (bwoj)
Details | Diff | Splinter Review
Bug 35292: Template changes (2.31 KB, patch)
2024-12-04 16:38 UTC, Baptiste Wojtkowski (bwoj)
Details | Diff | Splinter Review
Bug 35292: Have UpdateNotForLoanStatusOnCheckOut and UpdateNotForLoanStatusOnCheckIn use the same function (7.19 KB, patch)
2024-12-04 16:38 UTC, Baptiste Wojtkowski (bwoj)
Details | Diff | Splinter Review
Bug 35292: Database changes (6.75 KB, patch)
2024-12-09 16:35 UTC, Baptiste Wojtkowski (bwoj)
Details | Diff | Splinter Review
Bug 35292: Template changes (2.31 KB, patch)
2024-12-09 16:35 UTC, Baptiste Wojtkowski (bwoj)
Details | Diff | Splinter Review
Bug 35292: Have UpdateNotForLoanStatusOnCheckOut and UpdateNotForLoanStatusOnCheckIn use the same function (7.19 KB, patch)
2024-12-09 16:35 UTC, Baptiste Wojtkowski (bwoj)
Details | Diff | Splinter Review
Bug 35292: Move tests of UpdateNotforloan together (4.03 KB, patch)
2024-12-09 16:35 UTC, Baptiste Wojtkowski (bwoj)
Details | Diff | Splinter Review
Bug 35292: Update tests for UpdateNotForloanStatusOnCheckout (4.89 KB, patch)
2024-12-09 16:35 UTC, Baptiste Wojtkowski (bwoj)
Details | Diff | Splinter Review
Bug 35292: Add tests for _updateNotForLoanFromYaml (5.88 KB, patch)
2024-12-09 16:35 UTC, Baptiste Wojtkowski (bwoj)
Details | Diff | Splinter Review
Bug 35292: Move tests for UpdateNotForLoanStatusOnCheckin in a subtest (9.35 KB, patch)
2024-12-09 16:35 UTC, Baptiste Wojtkowski (bwoj)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lisette Scheer 2023-11-08 16:11:17 UTC
For parity and more workflow options, adding the same option to the new status that updates not for loan on checkout should also have the ability to limit by item type like Bug 25560 did for check in.
Comment 1 Baptiste Wojtkowski (bwoj) 2024-12-04 16:38:06 UTC
Created attachment 175201 [details] [review]
Bug 35292: Database changes
Comment 2 Baptiste Wojtkowski (bwoj) 2024-12-04 16:38:09 UTC
Created attachment 175202 [details] [review]
Bug 35292: Template changes
Comment 3 Baptiste Wojtkowski (bwoj) 2024-12-04 16:38:11 UTC
Created attachment 175203 [details] [review]
Bug 35292: Have UpdateNotForLoanStatusOnCheckOut and UpdateNotForLoanStatusOnCheckIn use the same function

Both sysprefs use the same codebase. Hence the recent upgrades for
UpdateNotForLoanStatusOnCheckOut were not applyied to checkout.
In this patch, we refacto code in order to have them both use the same
function.

Test plan:
Do not apply patch:
1 -  Set UpdateNotForLoanStatusOnCheckin to
2: 0
2 - Set  UpdateNotForLoanStatusOnCheckout to
_ALL_:
 0: 2
3 - Check out a book -> Its notforloan status is set to 2
4 - Check in the book -> Its notforloan status is set to 0, you get a
  notification
6 - APPLY PATCH
7 -  Set UpdateNotForLoanStatusOnCheckin to
_ALL_:
 2: 0

BK:
 3: 0

CR:
 4: 0

8 -  Set UpdateNotForLoanStatusOnCheckOut to
_ALL_:
 2: 0

BK:
 3: 0

CR:
 4: 0

8 - Check out a book -> Its notforloan status is set to 3
9 - Check in the book -> Its notforloan status is set to 0, you get a
10 - Check out a continuing resource -> Its notforloan status is set to 4
11 - Check in a continuing resource -> Its notforloan status is set to 0
11 - Check out another type of resource -> Its notforloan status is set to 2
12 - Check in the document -> Its notforloan status is set to 0
  notification
Comment 4 Baptiste Wojtkowski (bwoj) 2024-12-05 13:40:36 UTC
Removing status needs signoff, need to update the test suite
Comment 5 Baptiste Wojtkowski (bwoj) 2024-12-09 16:35:40 UTC
Created attachment 175316 [details] [review]
Bug 35292: Database changes
Comment 6 Baptiste Wojtkowski (bwoj) 2024-12-09 16:35:42 UTC
Created attachment 175317 [details] [review]
Bug 35292: Template changes
Comment 7 Baptiste Wojtkowski (bwoj) 2024-12-09 16:35:44 UTC
Created attachment 175318 [details] [review]
Bug 35292: Have UpdateNotForLoanStatusOnCheckOut and UpdateNotForLoanStatusOnCheckIn use the same function

Both sysprefs use the same codebase. Hence the recent upgrades for
UpdateNotForLoanStatusOnCheckOut were not applyied to checkout.
In this patch, we refacto code in order to have them both use the same
function.

Test plan:
Do not apply patch:
1 -  Set UpdateNotForLoanStatusOnCheckin to
2: 0
2 - Set  UpdateNotForLoanStatusOnCheckout to
_ALL_:
 0: 2
3 - Check out a book -> Its notforloan status is set to 2
4 - Check in the book -> Its notforloan status is set to 0, you get a
  notification
6 - APPLY PATCH
7 -  Set UpdateNotForLoanStatusOnCheckin to
_ALL_:
 2: 0

BK:
 3: 0

CR:
 4: 0

8 -  Set UpdateNotForLoanStatusOnCheckOut to
_ALL_:
 2: 0

BK:
 3: 0

CR:
 4: 0

8 - Check out a book -> Its notforloan status is set to 3
9 - Check in the book -> Its notforloan status is set to 0, you get a
10 - Check out a continuing resource -> Its notforloan status is set to 4
11 - Check in a continuing resource -> Its notforloan status is set to 0
11 - Check out another type of resource -> Its notforloan status is set to 2
12 - Check in the document -> Its notforloan status is set to 0
  notification
Comment 8 Baptiste Wojtkowski (bwoj) 2024-12-09 16:35:47 UTC
Created attachment 175319 [details] [review]
Bug 35292: Move tests of UpdateNotforloan together

Note: These patches have been split for sake of understandability for
review but they should probably be squashed
Comment 9 Baptiste Wojtkowski (bwoj) 2024-12-09 16:35:50 UTC
Created attachment 175320 [details] [review]
Bug 35292: Update tests for UpdateNotForloanStatusOnCheckout

Note: These patches have been split for sake of understandability for
review but they should probably be squashe
Comment 10 Baptiste Wojtkowski (bwoj) 2024-12-09 16:35:52 UTC
Created attachment 175321 [details] [review]
Bug 35292: Add tests for _updateNotForLoanFromYaml

Note: These patches have been split for sake of understandability for
review but they should probably be squashe
Comment 11 Baptiste Wojtkowski (bwoj) 2024-12-09 16:35:55 UTC
Created attachment 175322 [details] [review]
Bug 35292: Move tests for UpdateNotForLoanStatusOnCheckin in a subtest

Note: These patches have been split for sake of understandability for
review but they should probably be squashe
Comment 12 Emmanuel Bétemps 2025-01-13 10:47:22 UTC
Hi,

I just tried this test but it quickly failed (even before applying the patch)


Steps 1 to 2: ok 

Step 3 : ok in UNIMARC (but I had to erase the space  which stands just before the 2 in UpdateNotForLoanStatusOnCheckout)
but not ok in MARC21, the status is not set to 2

Step 4 : in UNIMARC, the status is not set to 0
(I didn't try in MARC21 because it failed before)


is there anything I missed ?