Bug 25147 - AllowSelfCheckReturns is in the wrong system preference section
Summary: AllowSelfCheckReturns is in the wrong system preference section
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Self checkout (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: David Roberts
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-04-14 20:20 UTC by Caroline Cyr La Rose
Modified: 2020-05-05 22:50 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00


Attachments
Bug 25147: AllowSelfCheckReturns is in the wrong system preference section (2.09 KB, patch)
2020-04-15 16:38 UTC, David Roberts
Details | Diff | Splinter Review
Bug 25147: AllowSelfCheckReturns is in the wrong system preference section (2.14 KB, patch)
2020-04-16 08:36 UTC, Devinim
Details | Diff | Splinter Review
Bug 25147: AllowSelfCheckReturns is in the wrong system preference section (2.22 KB, patch)
2020-04-16 10:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25147: Rename preference (3.50 KB, patch)
2020-04-17 07:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25147: Clarify section name (1.47 KB, patch)
2020-04-17 07:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25147: (RM follow-up) Update display (1.10 KB, patch)
2020-04-17 08:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25147: (RM follow-up) Update pref references. (5.67 KB, patch)
2020-04-17 08:09 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Caroline Cyr La Rose 2020-04-14 20:20:37 UTC
Unless I misunderstand the purpose of the AllowSelfCheckReturns system preference, I think it should be in the "Self check-in module" section of the Circulation system preferences. It is currently in the "Self Checkout" section.
Comment 1 David Roberts 2020-04-15 16:38:07 UTC
Created attachment 103022 [details] [review]
Bug 25147: AllowSelfCheckReturns is in the wrong system preference section

This patch moves the AllowSelfCheckReturns system preference to the "Self check-in module" section of the Circulation system preferences.

To test:

1) Check that the AllowSelfCheckReturns system preference is currently
in the "Self Checkout" section of the Circulation system preferences.
2) Apply the patch
3) The preference should now be in the "Self check-in module" section of
the Circulation system preferences.
Comment 2 Devinim 2020-04-16 08:36:07 UTC
Created attachment 103059 [details] [review]
Bug 25147: AllowSelfCheckReturns is in the wrong system preference section

This patch moves the AllowSelfCheckReturns system preference to the "Self check-in module" section of the Circulation system preferences.

To test:

1) Check that the AllowSelfCheckReturns system preference is currently
in the "Self Checkout" section of the Circulation system preferences.
2) Apply the patch
3) The preference should now be in the "Self check-in module" section of
the Circulation system preferences.

Signed-off-by: Devinim <kohadevinim@devinim.com.tr>
Comment 3 Jonathan Druart 2020-04-16 10:33:56 UTC
Created attachment 103069 [details] [review]
Bug 25147: AllowSelfCheckReturns is in the wrong system preference section

This patch moves the AllowSelfCheckReturns system preference to the "Self check-in module" section of the Circulation system preferences.

To test:

1) Check that the AllowSelfCheckReturns system preference is currently
in the "Self Checkout" section of the Circulation system preferences.
2) Apply the patch
3) The preference should now be in the "Self check-in module" section of
the Circulation system preferences.

Signed-off-by: Devinim <kohadevinim@devinim.com.tr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 4 Phil Ringnalda 2020-04-16 15:17:48 UTC
Everything currently in the chunk labelled "Self Checkout" is preferences used to control sco-main.pl, a feature that does checkout and renewal, and optionally with the preference AllSelfCheckReturns, also checkin.

Everything currently in the chunk labelled "Self check-in module" is preferences used to control sci-main.pl, a separate feature that does only checkin, and does checkin whether or not AllowSelfCheckReturns is set.

I'd think a better solution would be to either rename the preference, which is actually AllowReturnsInSelfCheckout, or rename the chunks to make it more clear that they are preferences for two different features.
Comment 5 Martin Renvoize 2020-04-16 18:28:17 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 6 Martin Renvoize 2020-04-17 06:44:48 UTC
(In reply to Phil Ringnalda from comment #4)
> Everything currently in the chunk labelled "Self Checkout" is preferences
> used to control sco-main.pl, a feature that does checkout and renewal, and
> optionally with the preference AllSelfCheckReturns, also checkin.
> 
> Everything currently in the chunk labelled "Self check-in module" is
> preferences used to control sci-main.pl, a separate feature that does only
> checkin, and does checkin whether or not AllowSelfCheckReturns is set.
> 
> I'd think a better solution would be to either rename the preference, which
> is actually AllowReturnsInSelfCheckout, or rename the chunks to make it more
> clear that they are preferences for two different features

Fair comment, I'd accept a followup to clarify the situation further.
Comment 7 Martin Renvoize 2020-04-17 06:50:51 UTC
On further consideration, I'm going to revert this patch from master and replace it with a quick followup to clarify the preference name as suggested by Phil.
Comment 8 Martin Renvoize 2020-04-17 07:21:13 UTC
Created attachment 103116 [details] [review]
Bug 25147: Rename preference

This patch renames the preference from the ambiguous 'AllowSelfCheckReturns'
to 'SCOAllowCheckin' which is more consistent with other preferences
affecting this module.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2020-04-17 07:21:16 UTC
Created attachment 103117 [details] [review]
Bug 25147: Clarify section name

Clarify the preference section headings for Self-checkout and
Self-checkin.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize 2020-04-17 07:26:31 UTC
Alternatives pushed :)
Comment 11 Martin Renvoize 2020-04-17 08:09:35 UTC
Created attachment 103120 [details] [review]
Bug 25147: (RM follow-up) Update display

I missed the display change in the preference name change.. oops.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2020-04-17 08:09:39 UTC
Created attachment 103121 [details] [review]
Bug 25147: (RM follow-up) Update pref references.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2020-04-17 08:10:01 UTC
I made a right hash of that.. should all be good now.
Comment 14 Joy Nelson 2020-05-05 22:50:55 UTC
enhancement not backported to 19.11.x