After bug 25560, the description for UpdateNotForLoanStatusOnCheckin read: >> - "This is a list of item types and value pairs.<br/><br/>" >> - "<strong>Examples:</strong><br/>" >> - "_ALL_:<br/>" >> - " -1: 0<br/><br/>" >> - "CR:<br/>" >> - " 1: 0<br/><br/>" >> - "When an item is checked in, if it's item type matches CR then when the value on the left (1) matches the items' not for loan value it will be updated to the value on the right.<br/><br/>The special term _ALL_ is used on the left side of the colon (:) to affect all item types, <strong>this does not override all other rules</strong><br/><br/>" >> - "Each item type needs to be defined on a separate line on the left side of the colon (:).<br/>Each pair of not for loan values, for that item type, should be listed on separate lines below the item type, each indented by a leading space<br/><br/>" After Bug 33390 was pushed, it now reads again as: >> - This is a list of value pairs. When an item is checked in, if the not for loan value on the left matches the items not for loan value >> - "it will be updated to the right-hand value. For example, '-1: 0' will cause an item that was set to 'Ordered' to now be available for loan." >> - "Can be used for showing only the not for loan description. E.g. '-1: ONLYMESSAGE'." >> - Each pair of values should be on a separate line.
This is a rebase issue most likely. If you look at my follow-up, you can see it only changed a line break (for nicer translatability). https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=158202 We still need to fix tho.
I think it's not as easy as it sounds. The patch you refer to removed the mention of ONLYMESSAGE making it an undocumented feature. type: textarea syntax: text/x-yaml class: code - - This is a list of value pairs. When an item is checked in, if the not for loan value on the left matches the items not for loan value - - "it will be updated to the right-hand value. For example, '-1: 0' will cause an item that was set to 'Ordered' to now be available for loan." - - "Can be used for showing only the not for loan description. E.g. '-1: ONLYMESSAGE'." - - Each pair of values should be on a separate line. + - "This is a list of item types and value pairs.<br/><br/>" + - "<strong>Examples:</strong><br/>" + - "_ALL_:<br/>" + - " -1: 0<br/><br/>" + - "CR:<br/>" + - " 1: 0<br/><br/>" + - "When an item is checked in, if it's item type matches CR then when the value on the left (1) matches the items' not for loan value it will be updated to the value on the right.<br/><br/>The special term _ALL_ is used on the left side of the colon (:) to affect all item types, <strong>this does not override all other rules</strong><br/><br/>" + - "Each item type needs to be defined on a separate line on the left side of the colon (:).<br/>Each pair of not for loan values, for that item type, should be listed on separate lines below the item type, each indented by a leading space<br/><br/>"
We are about to string freeze tomorrow and I am not sure I understand it all well enough yet :( Alex, could you confirm that ONLYMESSAGE should remain? It was added as a feature by bug 30407.
(In reply to Katrin Fischer from comment #3) > We are about to string freeze tomorrow and I am not sure I understand it all > well enough yet :( > > Alex, could you confirm that ONLYMESSAGE should remain? > > It was added as a feature by bug 30407. Hi Katrin, My apologies for my slow reply. I've checked and yes can confirm ONLYMESSAGE should remain. The way I confirmed this for sure was running the t/db_dependent/Circulation/issue.t unit test file, which passes both for my changes in bug 25560 and the ONLYMESSAGE changes in bug 30407. Please let me know if there is anything I can do to help, and apologies again for my delay.
If you have a moment to write the merged/combined system preference description I could sign off on it in the morning. But we are already in string freeze.
(In reply to Katrin Fischer from comment #5) > If you have a moment to write the merged/combined system preference > description I could sign off on it in the morning. But we are already in > string freeze. Hi Katrin, Thanks for that. Yes sure thing. Writing that now, attaching shortly
Created attachment 158850 [details] [review] Bug 35293: Add ONLYMESSAGE back into UpdateNotForLoanStatusOnCheckin syspref description Sponsored-by: Catalyst IT, New Zealand
Created attachment 158851 [details] [review] Bug 35293: Re-implement bug 30407 functionality To test: 1. Set items notforloan status as -1 (or create new one) 2. Add line "-1: ONLYMESSAGE" to UpdateNotForLoanStatusOnCheckin 3. Check item out for patron. 4. Check item in. => Description of notforloan status should be displayed. => Confirm notforloan status HAS changed. 5. Apply patch and update database if needed 6. Set items notforloan status as -1 (or create new one) 7. Check item out for patron. 8. Check item in. => Description of notforloan status should be displayed. => Confirm notforloan status HAS NOT changed. Also prove t/db_dependent/Circulation/issue.t Sponsored-by: Catalyst IT, New Zealand
Hi Katrin, I have attached a patch for combining the system preference description. I found that although the issue.t unit test passed successfully on master, the behaviour for the user in the bug 30407 test plan does not actually happen on master. i.e. when you check in an item it's notforloan status does change when it shouldn't. So I have attached a second patch which re-implements the behaviour from bug 30407 - not sure why that was lost on master. Please feel free to test or handle these patches as you see fit. I hope they are of some help. Thanks Alex
We tested this but could not reproduce the problem. 1. Notforloan item cannot be checked out with the provided systempreferences. 2. If you check in a not for loan item without checkin in it first, Koha only tells that the item was not onloan. 3. If you allow checkin out of notforloan items and then do the check out and check in, Koha does not tell anything about the notforloan status. Even when ONLYMESSAGE is configured to UpdateNotForLoanStatusOnCheckin. 4. The notforloan status did not dissapear on check in.
Created attachment 160360 [details] [review] Bug 35293: Re-implement bug 30407 functionality To test: 1. Set a book items notforloan status as -1 and set the AllowNotForLoanOverride system preference to 'Allow' 2. Add the following lines to UpdateNotForLoanStatusOnCheckin (the formatting and indentation is important to keep): BK: -1: ONLYMESSAGE 3. Check a book item out for patron. 4. Check book item in. => Description of notforloan status should be displayed under 'Check in message'. => Confirm notforloan status HAS changed. 5. Apply patch and update database if needed 6. Set book items notforloan status as -1 (or create new one) 7. Check book item out for patron. 8. Check book item in. => Description of notforloan status should be displayed under 'Check in message'. => Confirm notforloan status HAS NOT changed. Also prove t/db_dependent/Circulation/issue.t Sponsored-by: Catalyst IT, New Zealand
(In reply to Anneli Österman from comment #10) > We tested this but could not reproduce the problem. > > 1. Notforloan item cannot be checked out with the provided systempreferences. > 2. If you check in a not for loan item without checkin in it first, Koha > only tells that the item was not onloan. > 3. If you allow checkin out of notforloan items and then do the check out > and check in, Koha does not tell anything about the notforloan status. Even > when ONLYMESSAGE is configured to UpdateNotForLoanStatusOnCheckin. > 4. The notforloan status did not dissapear on check in. Hi Anneli, Happy holidays! Thank you for testing. I've tweaked the test plan - to include adding the item type into the UpdateNotForLoanStatusOnCheckin syspref then issuing and returning an item of that type. Would you mind please re-testing? Thanks very much, Alex
Created attachment 160361 [details] [review] Bug 35293: Add ONLYMESSAGE back into UpdateNotForLoanStatusOnCheckin syspref description Sponsored-by: Catalyst IT, New Zealand Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Created attachment 160362 [details] [review] Bug 35293: Re-implement bug 30407 functionality To test: 1. Set a book items notforloan status as -1 and set the AllowNotForLoanOverride system preference to 'Allow' 2. Add the following lines to UpdateNotForLoanStatusOnCheckin (the formatting and indentation is important to keep): BK: -1: ONLYMESSAGE 3. Check a book item out for patron. 4. Check book item in. => Description of notforloan status should be displayed under 'Check in message'. => Confirm notforloan status HAS changed. 5. Apply patch and update database if needed 6. Set book items notforloan status as -1 (or create new one) 7. Check book item out for patron. 8. Check book item in. => Description of notforloan status should be displayed under 'Check in message'. => Confirm notforloan status HAS NOT changed. Also prove t/db_dependent/Circulation/issue.t Sponsored-by: Catalyst IT, New Zealand Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Created attachment 160553 [details] [review] Bug 35293: Add ONLYMESSAGE back into UpdateNotForLoanStatusOnCheckin syspref description Sponsored-by: Catalyst IT, New Zealand Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 160554 [details] [review] Bug 35293: Re-implement bug 30407 functionality To test: 1. Set a book items notforloan status as -1 and set the AllowNotForLoanOverride system preference to 'Allow' 2. Add the following lines to UpdateNotForLoanStatusOnCheckin (the formatting and indentation is important to keep): BK: -1: ONLYMESSAGE 3. Check a book item out for patron. 4. Check book item in. => Description of notforloan status should be displayed under 'Check in message'. => Confirm notforloan status HAS changed. 5. Apply patch and update database if needed 6. Set book items notforloan status as -1 (or create new one) 7. Check book item out for patron. 8. Check book item in. => Description of notforloan status should be displayed under 'Check in message'. => Confirm notforloan status HAS NOT changed. Also prove t/db_dependent/Circulation/issue.t Sponsored-by: Catalyst IT, New Zealand Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 160555 [details] [review] Bug 35293: (follow-up) Handle all rules as well Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 160556 [details] [review] Bug 35293: Unit test Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
*** Bug 35692 has been marked as a duplicate of this bug. ***
Pushed for 24.05! Well done everyone, thank you!
Pushed to 23.11.x for 23.11.02
Missing dependencies for 23.05.x, no backport.