Created attachment 105889 [details] [review]
Bug 25761: Implementation of too_many_overdue has unintended consequences

Bug 24449 implemented the too_many_overdue SIP flag ( position 6 of the
patron status flags ). This has had unintended consequences as
many library use SIP2 for validation to eresource vendors ( Overdrive,
Hoopla, etc ), but do not want overdue checkouts to block the
use of these resources.

Test Plan:
1) Apply this patch
2) Restart all the things!
3) Find a patron with overdue items
4) Run a SIP2 patron information request
5) Note the "too many overdue" flag is set
6) Set the new SIP config option "disable_too_many_overdue" for the
account you used for testing
7) Restart the SIP server
8) Run the same patron information request
9) Notice the "too many overdue" flag is no longer set!

Thank you for making it an option.  Our library WANTS sip users blocked when they have overdues.  Especially now that we don't charge fines.

Created attachment 109747 [details] [review]
Bug 25761: Implementation of too_many_overdue has unintended consequences

Bug 24449 implemented the too_many_overdue SIP flag ( position 6 of the
patron status flags ). This has had unintended consequences as
many library use SIP2 for validation to eresource vendors ( Overdrive,
Hoopla, etc ), but do not want overdue checkouts to block the
use of these resources.

Test Plan:
1) Apply this patch
2) Restart all the things!
3) Find a patron with overdue items
4) Run a SIP2 patron information request
5) Note the "too many overdue" flag is set
6) Set the new SIP config option "disable_too_many_overdue" for the
account you used for testing
7) Restart the SIP server
8) Run the same patron information request
9) Notice the "too many overdue" flag is no longer set!

With patch applied, changing the flag in the SIP config file has no impact on response.

Created attachment 109786 [details] [review]
Bug 25761: Implementation of too_many_overdue has unintended consequences

Bug 24449 implemented the too_many_overdue SIP flag ( position 6 of the
patron status flags ). This has had unintended consequences as
many library use SIP2 for validation to eresource vendors ( Overdrive,
Hoopla, etc ), but do not want overdue checkouts to block the
use of these resources.

Test Plan:
1) Apply this patch
2) Restart all the things!
3) Find a patron with overdue items
4) Run a SIP2 patron information request
5) Note the "too many overdue" flag is set
6) Set the new SIP config option "disable_too_many_overdue" for the
account you used for testing
7) Restart the SIP server
8) Run the same patron information request
9) Notice the "too many overdue" flag is no longer set!

For consistency with bug 25541, I wonder if we ought to name the flag 'overdues_block_circ' or something along those lines?

As an aside, debian/templates/SIPconfig.xml is getting really out of sync these days.. I think there's a bug open to bring it back in sync.. but we should probably be catching cases where we add new config options and ensuring we add them to those templates too ;)

Hi, not enough experience with SIP so I have two questions

> 5) Note the "too many overdue" flag is set

> READ: 24              00120201112    201630AEEdna Acosta|AA23529001000463|BLY|CQN|AFGreetings from Koha. |AOCPL|

Which one is it? I couldn't in the spec where is the patron status field.

> 6) Set the new SIP config option "disable_too_many_overdue" for the
> account you used for testing

Where does one do that?

(In reply to Victor Grousset/tuxayo from comment #8)
> Hi, not enough experience with SIP so I have two questions
> 
> > 5) Note the "too many overdue" flag is set
> 
> > READ: 24              00120201112    201630AEEdna Acosta|AA23529001000463|BLY|CQN|AFGreetings from Koha. |AOCPL|
> 
> Which one is it? I couldn't in the spec where is the patron status field.

It is the 6th character of the patron status flags, the 8th character of the entire message.

24              00120201112    201630AEEdna
       ^
       This one. Y means patron has too many overdues, blank means they don't.
> 
> > 6) Set the new SIP config option "disable_too_many_overdue" for the
> > account you used for testing
> 
> Where does one do that?

Find your SIPconfig.xml, usually in /etc/koha/sites/<instance>/SIPconfig.xml, then add as you can see here at the bottom of the page for the SIP account you want to use for testing: https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109786&action=diff

Created attachment 113595 [details] [review]
Bug 25761: Implementation of too_many_overdue has unintended consequences

Bug 24449 implemented the too_many_overdue SIP flag ( position 6 of the
patron status flags ). This has had unintended consequences as
many library use SIP2 for validation to eresource vendors ( Overdrive,
Hoopla, etc ), but do not want overdue checkouts to block the
use of these resources.

Test Plan:
1) Apply this patch
2) Restart all the things!
3) Find a patron with overdue items
4) Run a SIP2 patron information request
5) Note the "too many overdue" flag is set
6) Set the new SIP config option "disable_too_many_overdue" for the
account you used for testing
7) Restart the SIP server
8) Run the same patron information request
9) Notice the "too many overdue" flag is no longer set!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 113596 [details] [review]
Bug 25761: (QA follow-up) Consistent flag names

This follow-up updates the config flag to be consistent with bug 24449
and updates the debian template file to match too.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

(In reply to Martin Renvoize from comment #11)
> Created attachment 113596 [details] [review] [review]
> Bug 25761: (QA follow-up) Consistent flag names
> 
> This follow-up updates the config flag to be consistent with bug 24449
> and updates the debian template file to match too.
> 
> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Thanks for the followup Martin!

Created attachment 113598 [details] [review]
Bug 25761: (QA follow-up) Consistent flag names

This follow-up updates the config flag to be consistent with bug 24449
and updates the debian template file to match too.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

(In reply to Kyle M Hall from comment #9)
> It is the 6th character of the patron status flags, the 8th character of the
> entire message.

READ: 24              00120201113    154013AEEdna Acosta|AA23529001000463|BLY|CQN|AFGreetings from Koha. |AOCPL|

overdue item +  OverduesBlockCirc == "Block" + add disable_too_many_overdue="0" to account koha + patches + restart_all

READ: 24Y             00120201113    163742AEEdna Acosta|AA23529001000463|BLY|CQN|AFGreetings from Koha.  -- Patron has overdues|AOCPL|


Somehow I get the charge privileges denied flag, is that expected?
And I don't get the disable_too_many_overdue flag.

Is the right config file this one? /etc/koha/sites/kohadev/SIPconfig.xml
(in koha-testing-docker)

(In reply to Victor Grousset/tuxayo from comment #14)
> (In reply to Kyle M Hall from comment #9)
> > It is the 6th character of the patron status flags, the 8th character of the
> > entire message.
> 
> READ: 24              00120201113    154013AEEdna
> Acosta|AA23529001000463|BLY|CQN|AFGreetings from Koha. |AOCPL|
> 
> overdue item +  OverduesBlockCirc == "Block" + add
> disable_too_many_overdue="0" to account koha + patches + restart_all
> 
> READ: 24Y             00120201113    163742AEEdna
> Acosta|AA23529001000463|BLY|CQN|AFGreetings from Koha.  -- Patron has
> overdues|AOCPL|
> 
> 
> Somehow I get the charge privileges denied flag, is that expected?
> And I don't get the disable_too_many_overdue flag.
> 
> Is the right config file this one? /etc/koha/sites/kohadev/SIPconfig.xml
> (in koha-testing-docker)

Sorry Victor, I should have made that clearer. My followup changes "disable_too_many_overdues" to "overdues_block_circ" to make it more consistent with some other configs we've introduced.

When enabled (the default) having too many overdues  (according to the circulation rules) should set that return flag to 'Y' as your seeing.. if you disable it by passing in that config Param with '0' then you should not see the 'Y'.

You are looking at the right file, but only if that after having run make to refresh it from the templates in the source directories.

Hope that helps

Created attachment 113630 [details] [review]
Bug 25761: Implementation of too_many_overdue has unintended consequences

Bug 24449 implemented the too_many_overdue SIP flag ( position 6 of the
patron status flags ). This has had unintended consequences as
many library use SIP2 for validation to eresource vendors ( Overdrive,
Hoopla, etc ), but do not want overdue checkouts to block the
use of these resources.

Test Plan:
1) Apply this patch
2) Restart all the things!
3) Find a patron with overdue items
4) Run a SIP2 patron information request
5) Note the "too many overdue" flag is set
6) Set the new SIP config option "disable_too_many_overdue" for the
account you used for testing
7) Restart the SIP server
8) Run the same patron information request
9) Notice the "too many overdue" flag is no longer set!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Created attachment 113631 [details] [review]
Bug 25761: (QA follow-up) Consistent flag names

This follow-up updates the config flag to be consistent with bug 24449
and updates the debian template file to match too.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

> having too many overdues  (according to the circulation rules)

Ok!!!!

Wait which setting of the circulation rules? I can't find it.

Anyway, I managed to get the expected result with OverduesBlockCirc



Thanks I learned useful stuff about SIP and the config files :)

BTW why do we have lpl-sc-beacock user in the config file if it's not in the sample data?

(In reply to Victor Grousset/tuxayo from comment #18)
> > having too many overdues  (according to the circulation rules)
> 
> Ok!!!!
> 
> Wait which setting of the circulation rules? I can't find it.
> 
> Anyway, I managed to get the expected result with OverduesBlockCirc
> 
> 
> 
> Thanks I learned useful stuff about SIP and the config files :)
> 
> BTW why do we have lpl-sc-beacock user in the config file if it's not in the
> sample data?

Oops, indeed, it's OverduesBlockCirc syspref, my mistake.

No idea about the user in the config.. likely a legacy thing.

> No idea about the user in the config.. likely a legacy thing.

Shall I do a followup and swap it with the koha user? or maybe with koha_sip (login term1)
As we add config to lpl-sc-beacock, it might as well match an account of sample data.

Pushed to master for 20.11, thanks to everybody involved!

Merge conflicts for 20.05.x, please rebase if needed