Currently, the patron information returned by SIP only shows "PATRON EXPIRED" when the patron card has expired. This patch makes the display more consistant with the Opac display and also complies with the NotifyBorrowerDeparture system preference.
Created attachment 106057 [details] [review] Bug 25813: Enhance patron expiration in SIP display Currently, the patron information returned by SIP only shows "PATRON EXPIRED" when the patron card has expired. This patch makes the display more consistant with the Opac display and also complies with the NotifyBorrowerDeparture system preference. Test plan: - apply the patch - set NotifyBorrowerDeparture to 0 - check that nothing is ever displayed about the card expiration - set NotifyBorrowerDeparture to a value greater than 0 - check that the following message will be displayed for a card that will expire within NotifyBorrowerDeparture days: "Your card will expire on {correctly formatted date}" - check that the following message will be displayed for a card that has expired: "Your account has expired as of {correctly formatted date}" You can use src/C4/SIP/interactive_patron_dump.pl for easier testing.
Created attachment 154697 [details] [review] Bug 25813: Enhance patron expiration in SIP display Currently, the patron information returned by SIP only shows "PATRON EXPIRED" when the patron card has expired. This patch makes the display more consistant with the Opac display and also complies with the NotifyBorrowerDeparture system preference. Test plan: - apply the patch - set NotifyBorrowerDeparture to 0 - check that nothing is ever displayed about the card expiration - set NotifyBorrowerDeparture to a value greater than 0 - check that the following message will be displayed for a card that will expire within NotifyBorrowerDeparture days: "Your card will expire on {correctly formatted date}" - check that the following message will be displayed for a card that has expired: "Your account has expired as of {correctly formatted date}" You can use src/C4/SIP/interactive_patron_dump.pl for easier testing.
The patch has been rebased, and unit tests have been added.
I can't apply le patch Applying: Bug 25813: Enhance patron expiration in SIP display Using index info to reconstruct a base tree... M C4/SIP/ILS/Patron.pm M t/db_dependent/SIP/Patron.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/SIP/Patron.t Auto-merging C4/SIP/ILS/Patron.pm CONFLICT (content): Merge conflict in C4/SIP/ILS/Patron.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 25813: Enhance patron expiration in SIP display
Created attachment 160730 [details] [review] Bug 25813: Enhance patron expiration in SIP display Currently, the patron information returned by SIP only shows "PATRON EXPIRED" when the patron card has expired. This patch makes the display more consistant with the Opac display and also complies with the NotifyBorrowerDeparture system preference. Test plan: - apply the patch - set NotifyBorrowerDeparture to 0 - check that nothing is ever displayed about the card expiration - set NotifyBorrowerDeparture to a value greater than 0 - check that the following message will be displayed for a card that will expire within NotifyBorrowerDeparture days: "Your card will expire on {correctly formatted date}" - check that the following message will be displayed for a card that has expired: "Your account has expired as of {correctly formatted date}" You can use src/C4/SIP/interactive_patron_dump.pl for easier testing.
The patch has been rebased.
Works well, using sip_cli_emulator.pl to test.
Created attachment 160794 [details] [review] Bug 25813: Enhance patron expiration in SIP display Currently, the patron information returned by SIP only shows "PATRON EXPIRED" when the patron card has expired. This patch makes the display more consistant with the Opac display and also complies with the NotifyBorrowerDeparture system preference. Test plan: - apply the patch - set NotifyBorrowerDeparture to 0 - check that nothing is ever displayed about the card expiration - set NotifyBorrowerDeparture to a value greater than 0 - check that the following message will be displayed for a card that will expire within NotifyBorrowerDeparture days: "Your card will expire on {correctly formatted date}" - check that the following message will be displayed for a card that has expired: "Your account has expired as of {correctly formatted date}" You can use src/C4/SIP/interactive_patron_dump.pl for easier testing. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 161084 [details] [review] Bug 25813: Enhance patron expiration in SIP display Currently, the patron information returned by SIP only shows "PATRON EXPIRED" when the patron card has expired. This patch makes the display more consistant with the Opac display and also complies with the NotifyBorrowerDeparture system preference. Test plan: - apply the patch - set NotifyBorrowerDeparture to 0 - check that nothing is ever displayed about the card expiration - set NotifyBorrowerDeparture to a value greater than 0 - check that the following message will be displayed for a card that will expire within NotifyBorrowerDeparture days: "Your card will expire on {correctly formatted date}" - check that the following message will be displayed for a card that has expired: "Your account has expired as of {correctly formatted date}" You can use src/C4/SIP/interactive_patron_dump.pl for easier testing. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
All working as described, code is tidy, covered by tests and passes QA scripts. Passing QA
I'll push this, but I'd love a follow-up doing some terminology clean-up: * Debarred = Restricted * Borrower = Patron I know we have tons of each, but we decided to move one direction, so let's not move backwards :)
Hi Katrin, I agree with you about having consistent terminology. However, NotifyBorrowerDeparture is a pre-existing system preference and is used in other places in Koha. Renaming it to NotifyPatronDeparture should be done in a separate BZ in my opinion. As for Debarred/Restricted, it is only code comments and a log line, it is never displayed to the user. I tried to be consistent with the Patron object method, which is $patron->is_debarred (and not $patron->is_restricted). That being said, I would happily change code comments (borrower -> patron, debarred -> restricted) and the log line if you think it's better this way.
Maybe I am going a bit far today, but not as far as asking you to rename the pref at least :) I mostly meant the code comments as it also can get confusing to new devs with our all-over mix of things.
Pushed for 24.05! Well done everyone, thank you!
Not backported to 23.11.x
I don't think there is anything to add/edit in the Koha manual for this.