Bug 37525 - [SCO] "Return this item" doesn't show up in scan confirmation screen despite SCOAllowCheckin being allowed
Summary: [SCO] "Return this item" doesn't show up in scan confirmation screen despite ...
Status: Signed Off
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: Testopia
URL:
Keywords:
Depends on: 32256
Blocks:
  Show dependency treegraph
 
Reported: 2024-07-30 11:59 UTC by Oliver Behnke
Modified: 2024-09-12 23:00 UTC (History)
2 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
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

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!