Bug 25029 - AllowPatronToControlAutorenewal needs to have a disable setting
Summary: AllowPatronToControlAutorenewal needs to have a disable setting
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement with 5 votes (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 24476
Blocks:
  Show dependency treegraph
 
Reported: 2020-03-31 15:17 UTC by Christopher Brannon
Modified: 2022-04-21 00:48 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 25029: Add autorenew_checkouts column to preferences json (1.67 KB, patch)
2020-10-13 15:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25029: Add autorenew_checkouts column to preferences json (1.73 KB, patch)
2021-01-20 16:17 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 29591: Add autorenew_checkouts column to preferences json (1.77 KB, patch)
2021-11-29 14:40 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Brannon 2020-03-31 15:17:30 UTC
The AllowPatronToControlAutorenewal has a Staff or Stand and Patron setting, but it needs to be a master disable switch as well.  With it in the staff only position, staff still see a switch for auto renewal in the patron settings, despite that nothing is set in the circulation rules.

It would be ideal if AllowPatronToControlAutorenewal had a Disable setting that:
* hid the "Allow auto-renewal of items" choice in the Library Management settings for the patron.
* hid the three columns in the circulation rules that deal with Auto Renewal.
Comment 1 Nick Clemens 2020-10-13 15:45:55 UTC
Created attachment 111566 [details] [review]
Bug 25029: Add autorenew_checkouts column to preferences json

Bug 24476 added autorenew_checkouts as a borrower column, intending it to be able to be
hidden if a library was not using the feature.

When the column chooser was added, this column was not included.

To test:
1 - Search system preferences for "BorrowerUnwantedFields"
2 - Click the box to change the value
3 - "autorenew_checkouts" is not an option
4 - Confirm that you see the option to opt patron out of autorenewal when editing patron on staff side
5 - Apply patch and restart everythign
6 - Search for the syspref again
7 - Note that now you can select "autorenew_checkouts"
8 - Check the box and save
9 - Confirm the opt out option is no longer visible during patron editing
Comment 2 PTFS Europe Sandboxes 2021-01-20 16:17:54 UTC
Created attachment 115446 [details] [review]
Bug 25029: Add autorenew_checkouts column to preferences json

Bug 24476 added autorenew_checkouts as a borrower column, intending it to be able to be
hidden if a library was not using the feature.

When the column chooser was added, this column was not included.

To test:
1 - Search system preferences for "BorrowerUnwantedFields"
2 - Click the box to change the value
3 - "autorenew_checkouts" is not an option
4 - Confirm that you see the option to opt patron out of autorenewal when editing patron on staff side
5 - Apply patch and restart everythign
6 - Search for the syspref again
7 - Note that now you can select "autorenew_checkouts"
8 - Check the box and save
9 - Confirm the opt out option is no longer visible during patron editing

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 3 Sally 2021-01-20 16:18:13 UTC
Great idea.  Works as described.
Comment 4 Christopher Brannon 2021-01-20 16:33:22 UTC
What about the three auto renew columns in the Circulation rules?
Comment 5 Andrew Fuerste-Henry 2021-01-21 16:14:34 UTC
(In reply to Christopher Brannon from comment #4)
> What about the three auto renew columns in the Circulation rules?

I'd argue that's a separate question/issue - removing an option in the patron record vs entirely hiding an option from your circ rules. Maybe file a separate bug for the option to completely hide auto-renew from the circ rules?
Comment 6 Katrin Fischer 2021-01-21 19:48:16 UTC
I really think this is the way to go, but there is a side effect in the option showing up for all patron preferences where they don't any effect (example: PatronSelfModificationBorrowerUnwantedField). I think there is another patch around adding a limit to the display for another option at the moment.
Comment 7 Christopher Brannon 2021-01-22 16:12:54 UTC
(In reply to PTFS Europe Sandboxes from comment #2)
> Created attachment 115446 [details] [review] [review]
> Bug 25029: Add autorenew_checkouts column to preferences json
> 
> Bug 24476 added autorenew_checkouts as a borrower column, intending it to be
> able to be
> hidden if a library was not using the feature.
> 
> When the column chooser was added, this column was not included.
> 
> To test:
> 1 - Search system preferences for "BorrowerUnwantedFields"
> 2 - Click the box to change the value
> 3 - "autorenew_checkouts" is not an option
> 4 - Confirm that you see the option to opt patron out of autorenewal when
> editing patron on staff side
> 5 - Apply patch and restart everythign
> 6 - Search for the syspref again
> 7 - Note that now you can select "autorenew_checkouts"
> 8 - Check the box and save
> 9 - Confirm the opt out option is no longer visible during patron editing
> 
> Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

I don't know that I agree with this approach.  This patch provides a way to clean up the mess left behind when a feature is not used.  A straight forward approach would be to have an option to turn off this feature and have all related pieces go away.  Instead, we have various options to hide it in different places.

It doesn't make sense that if a library doesn't want a feature that we have to go to other controls and hide it.  It should either be on and in use, or off, and not shown anywhere.
Comment 8 Christopher Brannon 2021-01-22 16:15:10 UTC
(In reply to Andrew Fuerste-Henry from comment #5)
> (In reply to Christopher Brannon from comment #4)
> > What about the three auto renew columns in the Circulation rules?
> 
> I'd argue that's a separate question/issue - removing an option in the
> patron record vs entirely hiding an option from your circ rules. Maybe file
> a separate bug for the option to completely hide auto-renew from the circ
> rules?

I would counter argue that this is not a separate question/issue.  The request of this bug was a master off switch, not a request to independently hide individual parts of a feature.
Comment 9 Nick Clemens 2021-01-25 11:15:40 UTC
(In reply to Christopher Brannon from comment #8)
> (In reply to Andrew Fuerste-Henry from comment #5)
> > (In reply to Christopher Brannon from comment #4)
> > > What about the three auto renew columns in the Circulation rules?
> > 
> > I'd argue that's a separate question/issue - removing an option in the
> > patron record vs entirely hiding an option from your circ rules. Maybe file
> > a separate bug for the option to completely hide auto-renew from the circ
> > rules?
> 
> I would counter argue that this is not a separate question/issue.  The
> request of this bug was a master off switch, not a request to independently
> hide individual parts of a feature.

I think the title of this bug confused me, I read at as needing a way to disable AllowPatronToControlAutorenewal.

What I am hearing from these comments is 'Auto-Renewal features need a master on/off switch' - much like ILLModule

The patches here do address a bug and I think are a separate issue, would it be acceptable to move this larger request to a new bug?
Comment 10 Katrin Fischer 2021-01-25 12:12:17 UTC
I think some granularity is good and should not stop us from having a master switch as well. Some libraries will want to use the auto-renew feature, but not want to have individual settings per user.
Comment 11 Caroline Cyr La Rose 2021-04-22 14:16:37 UTC
(In reply to Christopher Brannon from comment #0)
> The AllowPatronToControlAutorenewal has a Staff or Stand and Patron setting,
> but it needs to be a master disable switch as well.  With it in the staff
> only position, staff still see a switch for auto renewal in the patron
> settings, despite that nothing is set in the circulation rules.
> 
> It would be ideal if AllowPatronToControlAutorenewal had a Disable setting
> that:
> * hid the "Allow auto-renewal of items" choice in the Library Management
> settings for the patron.
> * hid the three columns in the circulation rules that deal with Auto Renewal.

In my opinion (for what it is worth, probably two cents only!), AllowPatronToControlAutorenewal is ok as it is, because it does what the title of the syspref says it does. I do think we need a master switch for this feature, like the ILLModule syspref. 

The syspref could be called simply Autorenewal. 
Enable/Disable the auto-renewal feature.
NOTE: This system preference requires the misc/cronjobs/automatic_renewals.pl cronjob. Ask your system administrator to schedule it. 
NOTE: You must configure the auto-renewal settings in your circulation rules.

This system preference would do what Chris said above
> * hid[e] the "Allow auto-renewal of items" choice in the Library Management
> settings for the patron.
> * hid[e] the three columns in the circulation rules that deal with Auto Renewal.

A note should be added to AllowPatronToControlAutorenewal and OPACFineNoRenewalsBlockAutoRenew
NOTE: This system preference has no effect if Autorenewal is set to Disable.
Comment 12 Christopher Brannon 2021-04-22 14:57:26 UTC
(In reply to Caroline Cyr La Rose from comment #11)
> In my opinion (for what it is worth, probably two cents only!),
> AllowPatronToControlAutorenewal is ok as it is, because it does what the
> title of the syspref says it does. I do think we need a master switch for
> this feature, like the ILLModule syspref. 
> 
> The syspref could be called simply Autorenewal. 
> Enable/Disable the auto-renewal feature.
> NOTE: This system preference requires the
> misc/cronjobs/automatic_renewals.pl cronjob. Ask your system administrator
> to schedule it. 
> NOTE: You must configure the auto-renewal settings in your circulation rules.
> 
> This system preference would do what Chris said above
> > * hid[e] the "Allow auto-renewal of items" choice in the Library Management
> > settings for the patron.
> > * hid[e] the three columns in the circulation rules that deal with Auto Renewal.
> 
> A note should be added to AllowPatronToControlAutorenewal and
> OPACFineNoRenewalsBlockAutoRenew
> NOTE: This system preference has no effect if Autorenewal is set to Disable.

Wondering why a completely new switch would be better than a disable option in this one.  That would be like having an on/off switch for a light, and a completely separate dimmer to control it.  Creating a whole new switch makes more for admins to sift through and one more thing that we have to look at to make something work and an added failure point.  Unless I am missing something, it would be ideal to have the disable option in the same pref.  Adding a disable option to the pref would not change the nature of the pref, and it would still do what the name of the pref says.  Making admins chase down another pref to disable this pref is inefficient.  ANYWHERE we can put a disable in a pref rather than having a separate pref is an improvement, in my opinion.
Comment 13 Caroline Cyr La Rose 2021-04-22 15:35:23 UTC
For me, AllowPatronToControlAutorenewal is a switch to add this option to the OPAC, not to enable/disable the whole feature. Which is why I thought another syspref would be the way to go.

But that's just the way I understand it, it may be wrong.

I honestly don't care much how it is implemented, I would have been happy with the BorrowerUnwantedFields method. But since we started upgrading clients to 20.05, I've had multiple comments on the yes/no option in the patron file. So I thought I'd comment with my thoughts and experience.
Comment 14 Katrin Fischer 2021-04-22 19:41:55 UTC
(In reply to Caroline Cyr La Rose from comment #13)
> For me, AllowPatronToControlAutorenewal is a switch to add this option to
> the OPAC, not to enable/disable the whole feature. Which is why I thought
> another syspref would be the way to go.
> 
> But that's just the way I understand it, it may be wrong.
> 
> I honestly don't care much how it is implemented, I would have been happy
> with the BorrowerUnwantedFields method. But since we started upgrading
> clients to 20.05, I've had multiple comments on the yes/no option in the
> patron file. So I thought I'd comment with my thoughts and experience.

I totally agree with you on the master switch, that would be nice to remove complexity from the GUI for libraries not interested in automatic renewals at all.

I can also imagine a disable setting for AllowPatronToControlAutoRenewal, as not everyone will probably want this type of granular per person setting. 
BUT BIG BUT: I don't think it should hide it from circulation rules, but only control the option in he patron record.

Having auto-renewals scheduled automatically and the options to intervene for some patrons are different things.
Comment 15 Fridolin Somers 2021-08-11 07:00:28 UTC
I jump into the discussion.

Template contains : [% UNLESS noautorenew_checkouts %]
Surely this column must be added to borrowers.json.

Switching the all feature off is in my opinion another problem.

PS : I think patch need rebase because of :
Bug 11879: (follow up) Some fixes and code cleanup
Comment 16 Lucy Vaux-Harvey 2021-10-13 13:52:28 UTC
(In reply to Caroline Cyr La Rose from comment #13)
> For me, AllowPatronToControlAutorenewal is a switch to add this option to
> the OPAC, not to enable/disable the whole feature. Which is why I thought
> another syspref would be the way to go.
> 
> But that's just the way I understand it, it may be wrong.
> 
> I honestly don't care much how it is implemented, I would have been happy
> with the BorrowerUnwantedFields method. But since we started upgrading
> clients to 20.05, I've had multiple comments on the yes/no option in the
> patron file. So I thought I'd comment with my thoughts and experience.

Just adding further to Caroline's comment we are also getting frequent queries about this option at upgrades, it is causing confusion.  It would be great to have a disable option so that it didn't display.  I'd also agree that a master switch for auto renewals is a good idea.
Comment 17 Koha Team University Lyon 3 2021-10-18 13:53:46 UTC
I agree that we whould have a way to set the auto-renewal on a "off" mode with the syspref.
It really surprising that this option is set to "yes" when you subscribe a new patron.
At least, it should be a choice in the patron category setting.
Sonia
Comment 18 Nick Clemens 2021-11-29 14:40:16 UTC Comment hidden (obsolete)
Comment 19 Nick Clemens 2021-11-29 14:41:46 UTC
Patch moved to 29591 - discussion here should not prevent adding of the column to controls