Bug 35692 - ONLYMESSAGE option in UpdateNotforLoanStatusOnCheckin is not honored
Summary: ONLYMESSAGE option in UpdateNotforLoanStatusOnCheckin is not honored
Status: RESOLVED DUPLICATE of bug 35293
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Nick Clemens (kidclamp)
QA Contact: Testopia
URL:
Keywords:
Depends on: 25560 30407
Blocks:
  Show dependency treegraph
 
Reported: 2024-01-03 19:39 UTC by Nick Clemens (kidclamp)
Modified: 2024-01-04 19:34 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:
Circulation function:


Attachments
Bug 35692: Unit test (1.42 KB, patch)
2024-01-03 19:52 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 35692: Do not update not for loan status if ONLYMESSAGE is provided (2.61 KB, patch)
2024-01-03 19:52 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 35692: Unit test (1.47 KB, patch)
2024-01-03 22:15 UTC, David Nind
Details | Diff | Splinter Review
Bug 35692: Do not update not for loan status if ONLYMESSAGE is provided (2.66 KB, patch)
2024-01-03 22:16 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2024-01-03 19:39:12 UTC
It looks like between bug 30407 and bug 25560 we lost the line that made the update conditional:

 $item->notforloan($rules->{$key})->store({ log_action => 0, skip_record_index => 1, skip_holds_queue => 1 }) unless $rules->{$key} eq 'ONLYMESSAGE';
Comment 1 Nick Clemens (kidclamp) 2024-01-03 19:52:01 UTC
Created attachment 160499 [details] [review]
Bug 35692: Unit test
Comment 2 Nick Clemens (kidclamp) 2024-01-03 19:52:02 UTC
Created attachment 160500 [details] [review]
Bug 35692: Do not update not for loan status if ONLYMESSAGE is provided

To test:
1 - Set UpdateNotForLoanStatusOnCheckin to:
_ALL_:
  -1: ONLYMESSAGE
2 - Set an item in the catalog as -1/Ordered
3 - Check the item in
4 - Confirm you see the message
5 - View the record details
6 - Item is now for loan
7 - Check item in again, no message
8 - Apply patch, restart all
9 - Set item as not for loan -1/Ordered
10 - Check item in, you see the message
11 - Check it in again, still see message
12 - Check the detail page, item still marked as ordered
Comment 3 Alex Buckley 2024-01-03 20:14:37 UTC
Hi Nick, 

I noticed that too!

I submitted a patchset in https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35293 but happy for your patchset to be upstreamed instead :)

Thanks,
Alex
Comment 4 David Nind 2024-01-03 22:15:59 UTC
Created attachment 160501 [details] [review]
Bug 35692: Unit test

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2024-01-03 22:16:02 UTC
Created attachment 160502 [details] [review]
Bug 35692: Do not update not for loan status if ONLYMESSAGE is provided

To test:
1 - Set UpdateNotForLoanStatusOnCheckin to:
_ALL_:
  -1: ONLYMESSAGE
2 - Set an item in the catalog as -1/Ordered
3 - Check the item in
4 - Confirm you see the message
5 - View the record details
6 - Item is now for loan
7 - Check item in again, no message
8 - Apply patch, restart all
9 - Set item as not for loan -1/Ordered
10 - Check item in, you see the message
11 - Check it in again, still see message
12 - Check the detail page, item still marked as ordered

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 Alex Buckley 2024-01-04 16:41:03 UTC
Hi Nick, 

There is one difference between this and bug 35293 (the patchset I wrote) and that is 35293 re-adds the reference to ONLYMESSAGE into the UpdateNotForLoanStatusOnCheckin system preference which was also lost between bug 30407 and bug 25560 : https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160361

Would you like me to copy that patch over to your bug report here?

Thanks,
Alex
Comment 7 Nick Clemens (kidclamp) 2024-01-04 17:31:48 UTC
(In reply to Alex Buckley from comment #6)
> Hi Nick, 
> 
> There is one difference between this and bug 35293 (the patchset I wrote)
> and that is 35293 re-adds the reference to ONLYMESSAGE into the
> UpdateNotForLoanStatusOnCheckin system preference which was also lost
> between bug 30407 and bug 25560 :
> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160361
> 
> Would you like me to copy that patch over to your bug report here?
> 
> Thanks,
> Alex

Hi Alex, either way works, you could also just copy my unit test over to the other and then mark them duplicate. I didn't see yours initially because the depends on wasn't marked :-)
Comment 8 Nick Clemens (kidclamp) 2024-01-04 19:25:49 UTC

*** This bug has been marked as a duplicate of bug 35293 ***
Comment 9 Alex Buckley 2024-01-04 19:34:15 UTC
(In reply to Nick Clemens from comment #7)
> (In reply to Alex Buckley from comment #6)
> > Hi Nick, 
> > 
> > There is one difference between this and bug 35293 (the patchset I wrote)
> > and that is 35293 re-adds the reference to ONLYMESSAGE into the
> > UpdateNotForLoanStatusOnCheckin system preference which was also lost
> > between bug 30407 and bug 25560 :
> > https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160361
> > 
> > Would you like me to copy that patch over to your bug report here?
> > 
> > Thanks,
> > Alex
> 
> Hi Alex, either way works, you could also just copy my unit test over to the
> other and then mark them duplicate. I didn't see yours initially because the
> depends on wasn't marked :-)

Thanks so much Nick for making those changes! 

Apologies for not marking the depends on for bug 35293