Bug 37525 - Self checkout: "Return this item" doesn't show up in scan confirmation screen despite SCOAllowCheckin being allowed
Summary: Self checkout: "Return this item" doesn't show up in scan confirmation screen...
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Self checkout (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Oliver Behnke
QA Contact: Kyle M Hall (khall)
URL:
Keywords:
Depends on: 32256
Blocks:
  Show dependency treegraph
 
Reported: 2024-07-30 11:59 UTC by Oliver Behnke
Modified: 2024-11-18 22:26 UTC (History)
4 users (show)

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


Attachments
Patch to fix the regression (774 bytes, patch)
2024-07-30 11:59 UTC, Oliver Behnke
Details | Diff | Splinter Review
Patch to fix the regression (1.71 KB, patch)
2024-08-29 12:29 UTC, Oliver Behnke
Details | Diff | Splinter Review
Bug 37525: [SCO] Regression in 32256: "Return this item" doesn't show up despite SCOAllowCheckin being allowed (2.11 KB, patch)
2024-09-04 15:24 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 37525: [SCO] Regression in 32256: "Return this item" doesn't show up despite SCOAllowCheckin being allowed (2.18 KB, patch)
2024-09-27 14:54 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Behnke 2024-07-30 11:59:51 UTC
Created attachment 169856 [details] [review]
Patch to fix the regression

There's a regression in bug 32256 and commit bb059abe respectively:

https://gitlab.com/koha-community/Koha/-/commit/bb059abe90#a22dc682a84c41672294c66e441329a451b5cd5a_137_137

The refactoring of "renew" -> "c.renew" missed this line (at least), so the condition is never met.

The attached patch should do.

Thanks
Comment 1 Katrin Fischer 2024-07-31 11:44:36 UTC
Don't forget to switch to "Needs signoff" when you upload a patch - this will make it visible to the testers.
Comment 2 Katrin Fischer 2024-07-31 11:48:14 UTC
After taking a closer look, there are some more stumbling stones:

The attached file is not a patch file, but a diff and needs some work before it can be tested with our tools.

The commit message should include a test plan and also the bug number in the first line, see:

https://wiki.koha-community.org/wiki/Commit_messages
Comment 3 Katrin Fischer 2024-07-31 11:48:47 UTC
... and let us know if we can help :)
Comment 4 Oliver Behnke 2024-07-31 12:03:13 UTC
Just use the diff for additional illustration purposes then. I don't mind if the patch attribution gets lost and someone else takes the credit :)

Cheers
Comment 5 Olli-Antti Kivilahti 2024-08-27 10:59:34 UTC
(In reply to Oliver Behnke from comment #4)
> Just use the diff for additional illustration purposes then. I don't mind if
> the patch attribution gets lost and someone else takes the credit :)
> 
> Cheers

Hi!
I had implemented exactly the same solution to one of our customers two weeks ago.
I applied it at Bug 26479.
If you could kindly go sign it off, we could get this ball rolling, thank you.

It is exactly the solution you did so I presume you have it working somewhere with a real client already so should be easy to verify it works.

Unfortunately it doesn't fix the other problem of being unable to checkin books from the list of open issues.
But I felt like the user should not be able to check-in books from that list, because there is no guarantee the user actually has the book. It is better to require them to read the barcode instead, so they wont accidentally checkin books they were not supposed to.
Comment 6 Olli-Antti Kivilahti 2024-08-27 11:00:51 UTC

*** This bug has been marked as a duplicate of bug 26479 ***
Comment 7 Oliver Behnke 2024-08-28 11:47:43 UTC
Hi Olli-Antti,

I don't think this is an obvious duplicate of 26479 as I tried to describe in https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26479#c3. That's why I opened this bug here in the first place. If our fix also does the job for Katrin's original issue than great, but that should be up to her to confirm.

Thanks,
Oliver
Comment 8 Oliver Behnke 2024-08-28 11:48:59 UTC
(I'd really love to be able to edit comments to fix typos...)
Comment 9 Olli-Antti Kivilahti 2024-08-29 08:04:02 UTC
Sorry for jumping ahead again and closing this.
Thank you for being considerate.

See Bug 26479 for Katrin's comments.
Comment 10 Oliver Behnke 2024-08-29 12:29:40 UTC
Created attachment 170870 [details] [review]
Patch to fix the regression

While at it again, turn this into a proper patch against 'main'.
Comment 11 Victor Grousset/tuxayo 2024-09-04 15:24:08 UTC
Created attachment 171034 [details] [review]
Bug 37525: [SCO] Regression in 32256: "Return this item" doesn't show up despite SCOAllowCheckin being allowed

There's a regression in bug 32256 and commit bb059abe respectively:

https://gitlab.com/koha-community/Koha/-/commit/bb059abe90#a22dc682a84c41672294c66e441329a451b5cd5a_137_137

The refactoring of "renew" -> "c.renew" missed this line (at least), so the condition is never met.

This fixes the issue and restores the original functionality.

Test plan:
1. log in with a patron at http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
2. check out an item
3. scan it again
4. see that the "return this item" choice isn't proposed. This is the
   bug
5. apply patches
6. rescan the item in the self-check out
7. returning is proposed
8. check it in and confirms that it's not checked out anymore

Signed-off-by: Oliver Behnke <oliver.behnke@aei.mpg.de>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 12 Victor Grousset/tuxayo 2024-09-04 15:37:13 UTC
Thanks for the fix Oliver. For proofreading, you can see in the above attachment the added test plan that would correspond to the fix.
Comment 13 Oliver Behnke 2024-09-05 08:15:44 UTC
Thanks Victor!
Comment 14 Olli-Antti Kivilahti 2024-09-27 05:24:29 UTC
(In reply to Victor Grousset/tuxayo from comment #12)
> Thanks for the fix Oliver. For proofreading, you can see in the above
> attachment the added test plan that would correspond to the fix.

Thanks Victor!
Comment 15 Kyle M Hall (khall) 2024-09-27 14:54:54 UTC
Created attachment 172118 [details] [review]
Bug 37525: [SCO] Regression in 32256: "Return this item" doesn't show up despite SCOAllowCheckin being allowed

There's a regression in bug 32256 and commit bb059abe respectively:

https://gitlab.com/koha-community/Koha/-/commit/bb059abe90#a22dc682a84c41672294c66e441329a451b5cd5a_137_137

The refactoring of "renew" -> "c.renew" missed this line (at least), so the condition is never met.

This fixes the issue and restores the original functionality.

Test plan:
1. log in with a patron at http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
2. check out an item
3. scan it again
4. see that the "return this item" choice isn't proposed. This is the
   bug
5. apply patches
6. rescan the item in the self-check out
7. returning is proposed
8. check it in and confirms that it's not checked out anymore

Signed-off-by: Oliver Behnke <oliver.behnke@aei.mpg.de>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 16 Martin Renvoize (ashimema) 2024-09-30 14:34:14 UTC
Thanks for all the hard work!

Pushed to main for the next 24.11.00 release as RM Assistant
Comment 17 Oliver Behnke 2024-10-01 09:50:32 UTC
Thanks everyone!

Given that this is a trivial fix for regression, is this going to be backported to older releases? If so, who is the next assignee in the process?

Cheers
Comment 18 Victor Grousset/tuxayo 2024-10-08 20:52:33 UTC
(In reply to Oliver Behnke from comment #17)
> is this going to be
> backported to older releases? If so, who is the next assignee in the process?

Don't worry, backporting is a systematic process. See the criteria for general guidelines of how far down patches are backported:
https://wiki.koha-community.org/wiki/Release_maintenance#Deciding_on_what_to_back_port

And the process is explicit. So there should be in the coming weeks messages here on each backporting by the release maintainers until the explicit end of the process. If there is no activity before the 22th of this month (date of the monthly minor releases), then you can head over to the chat or the development mailing list. To ask for more info about the status of this and who to ask confirmation that it's still on their todo list.
Comment 19 Victor Grousset/tuxayo 2024-10-08 22:54:48 UTC
> backporting is a systematic process

(systematic evaluation of backportability, in addition of the guidelines, there are case by case consideration and eventual complications)
Comment 20 Oliver Behnke 2024-10-09 07:41:33 UTC
Thanks. I just wanted to make sure that I (as the current assignee) am not the bottleneck :)
Comment 21 Lucas Gass (lukeg) 2024-11-18 22:26:40 UTC
Backported to 24.05.x for upcoming 24.05.06