Bug 16223 - Automatically remove any borrower debarments after a payment
Summary: Automatically remove any borrower debarments after a payment
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Emmi Takkinen
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 33574
Blocks:
  Show dependency treegraph
 
Reported: 2016-04-07 11:38 UTC by Lari Taskula
Modified: 2023-10-14 12:10 UTC (History)
14 users (show)

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


Attachments
Bug 16223: Automatically remove any borrower debarments after a payment (12.00 KB, patch)
2016-04-08 12:43 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16223: Enable regex match for rules of debarment removals (4.18 KB, patch)
2016-04-08 13:49 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 16223: Automatically remove any borrower debarments after a payment (10.70 KB, patch)
2020-02-27 11:24 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Enable regex match for rules of debarment (4.14 KB, patch)
2020-02-27 11:25 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Automatically remove any borrower debarments after a payment (10.80 KB, patch)
2020-05-06 06:52 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Enable regex match for rules of debarment removals (4.16 KB, patch)
2020-05-06 06:53 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Automatically remove any borrower debarments after a payment (11.02 KB, patch)
2020-12-09 07:53 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Enable regex match for rules of debarment removals (4.26 KB, patch)
2020-12-09 07:54 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 26940: Take empty strings into account (1.29 KB, text/plain)
2021-03-19 12:54 UTC, ByWater Sandboxes
Details
Bug 16223: Enable regex match for rules of debarment removals (4.29 KB, patch)
2021-03-22 08:00 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 16223: Automatically remove any borrower debarments after a payment (11.10 KB, patch)
2021-03-22 08:03 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 16223: Enable regex match for rules of debarment removals (4.35 KB, patch)
2021-03-22 08:03 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 16223: Automatically remove any borrower debarments after a payment (11.21 KB, patch)
2021-07-12 05:56 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Enable regex match for rules of debarment removals (4.35 KB, patch)
2021-07-12 05:57 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Fix QA issues (4.31 KB, patch)
2021-11-03 13:27 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Call DelDebarmentsAfterPayment from Koha::Account::Line::apply (4.59 KB, patch)
2021-11-25 12:36 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Automatically remove any borrower debarments after a payment (11.03 KB, patch)
2022-03-28 06:16 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Enable regex match for rules of debarment removals (4.35 KB, patch)
2022-03-28 06:17 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Fix QA issues (4.31 KB, patch)
2022-03-28 06:17 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Call DelDebarmentsAfterPayment from Koha::Account::Line::apply (4.56 KB, patch)
2022-03-28 06:18 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Add new columns to table restriction_types (2.44 KB, patch)
2022-11-08 09:44 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: DO NOT PUSH! Schema changes (1.67 KB, patch)
2022-11-08 09:45 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Add ability to define liftable restrictions (13.35 KB, patch)
2022-11-08 09:45 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Add ability to define liftable restrictions (13.38 KB, patch)
2022-11-09 08:26 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Add new columns to table restriction_types (2.59 KB, patch)
2022-11-10 14:49 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 16223: DO NOT PUSH! Schema changes (1.82 KB, patch)
2022-11-10 14:50 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 16223: Add ability to define liftable restrictions (13.49 KB, patch)
2022-11-10 14:50 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 16223: Add new columns to table restriction_types (2.59 KB, patch)
2022-11-10 16:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 16223: DO NOT PUSH! Schema changes (1.82 KB, patch)
2022-11-10 16:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 16223: Add ability to define liftable restrictions (13.57 KB, patch)
2022-11-10 16:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 16223: Add new columns to table restriction_types (2.58 KB, patch)
2022-11-11 10:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 16223: DO NOT PUSH! Schema changes (1.82 KB, patch)
2022-11-11 10:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 16223: Add ability to define liftable restrictions (13.57 KB, patch)
2022-11-11 10:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Fix kohastructure bug (956 bytes, patch)
2022-11-11 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Remove GetDebarments (4.63 KB, patch)
2022-11-11 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Move trigger to apply (1.74 KB, patch)
2022-11-11 10:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 16223: Add correct value attributes to restriction options in select element (2.27 KB, patch)
2022-11-16 07:30 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Add new columns to table restriction_types (2.63 KB, patch)
2023-02-03 06:42 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: DO NOT PUSH! Schema changes (1.83 KB, patch)
2023-02-03 06:43 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Add ability to define liftable restrictions (13.58 KB, patch)
2023-02-03 06:43 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Fix kohastructure bug (1006 bytes, patch)
2023-02-03 06:44 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Remove GetDebarments (4.63 KB, patch)
2023-02-03 06:44 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Move trigger to apply (1.75 KB, patch)
2023-02-03 06:45 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Add correct value attributes to restriction options in select element (2.27 KB, patch)
2023-02-03 06:45 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Add new columns to table restriction_types (1.94 KB, patch)
2023-03-17 08:31 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: DO NOT PUSH! Schema changes (1.83 KB, patch)
2023-03-17 08:32 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Add ability to define liftable restrictions (13.58 KB, patch)
2023-03-17 08:32 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Remove GetDebarments (4.63 KB, patch)
2023-03-17 08:33 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Move trigger to apply (1.75 KB, patch)
2023-03-17 08:34 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Add correct value attributes to restriction options in select element (2.27 KB, patch)
2023-03-17 08:34 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Require fee limit only if "Lift after payment" is set as "Yes" (5.18 KB, patch)
2023-03-17 08:35 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Add new columns to table restriction_types (2.00 KB, patch)
2023-04-21 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 16223: DO NOT PUSH! Schema changes (1.88 KB, patch)
2023-04-21 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 16223: Add ability to define liftable restrictions (13.63 KB, patch)
2023-04-21 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Remove GetDebarments (4.69 KB, patch)
2023-04-21 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Move trigger to apply (1.81 KB, patch)
2023-04-21 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 16223: Require fee limit only if "Lift after payment" is set as "Yes" (5.23 KB, patch)
2023-04-21 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 16223: Add new columns to table restriction_types (2.01 KB, patch)
2023-07-05 06:36 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: DO NOT PUSH! Schema changes (1.89 KB, patch)
2023-07-05 06:37 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Add ability to define liftable restrictions (13.62 KB, patch)
2023-07-05 06:37 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Remove GetDebarments (4.70 KB, patch)
2023-07-05 06:38 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Move trigger to apply (1.81 KB, patch)
2023-07-05 06:38 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Require fee limit only if "Lift after payment" is set as "Yes" (5.24 KB, patch)
2023-07-05 06:41 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 16223: Add new columns to table restriction_types (2.10 KB, patch)
2023-09-15 09:18 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16223: DO NOT PUSH! Schema changes (1.98 KB, patch)
2023-09-15 09:19 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16223: Add ability to define liftable restrictions (13.73 KB, patch)
2023-09-15 09:19 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Remove GetDebarments (4.79 KB, patch)
2023-09-15 09:19 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Move trigger to apply (1.90 KB, patch)
2023-09-15 09:19 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16223: Require fee limit only if "Lift after payment" is set as "Yes" (5.33 KB, patch)
2023-09-15 09:19 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Add filter, remove superfluous condition (1.98 KB, patch)
2023-09-15 09:19 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16223: Add new columns to table restriction_types (2.11 KB, patch)
2023-10-11 15:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 16223: Add ability to define liftable restrictions (13.75 KB, patch)
2023-10-11 15:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Remove GetDebarments (4.81 KB, patch)
2023-10-11 15:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Move trigger to apply (1.92 KB, patch)
2023-10-11 15:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 16223: Require fee limit only if "Lift after payment" is set as "Yes" (5.35 KB, patch)
2023-10-11 15:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Add filter, remove superfluous condition (2.00 KB, patch)
2023-10-11 15:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 16223: (QA follow-up) Adjust DB changes (3.02 KB, patch)
2023-10-11 15:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 16223: DBIC schema (1.68 KB, patch)
2023-10-11 15:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 16223: Mark lift_after_payment as boolean in schema (971 bytes, patch)
2023-10-11 15:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 16223: Tidy (4.83 KB, patch)
2023-10-11 15:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lari Taskula 2016-04-07 11:38:15 UTC
Some libraries debar Borrowers at the end of the year for having unpaid fines, like in Bug 15157. Currently librarians have to manually remove this type of debarments after Borrower has paid his/her fines. Add a system preference to define debarments that should be automatically removed after a payment is made, and add functionality to actually remove the defined debarments from Borrower. Also let libraries to define the amount of outstanding fines after payment after which the debarment will be removed.
Comment 1 Lari Taskula 2016-04-08 12:43:45 UTC
Created attachment 50078 [details] [review]
Bug 16223: Automatically remove any borrower debarments after a payment

Some libraries debar Patrons at the end of the year for having unpaid fines,
like in Bug 15157. Currently librarians have to manually remove this type of
debarments after Patron has paid his/her fines. Add a system preference to
define debarments that should be automatically removed after a payment is made,
and add functionality to actually remove the defined debarments from Patron.
Also let libraries to define the amount of outstanding fines after payment
after which the debarment will be removed.

This patch introduces a system preference DebarmentsToLiftAfterPayment, which
allows libraries to define rules for removing debarments after paying fines.
The system preference uses YAML and is defined as follows:

Debarment with this comments will be removed:
  outstanding: 5

Which means that if a Patron has a debarment "Debarment with this comment will
be removed", and he pays his fines and charges until his outstanding fees are
equal or less than 5.00, this debarment will be lifted. The parameter outstanding
is optional - if not given, the debarment will be lifted until Patron has paid
all of his outstanding fees (in other words, equal to "outstanding: 0").

To test:
1. Set a debarment to a Patron
2. Set a fine for Patron
3. Define rule(s) for removing debarment(s) in system preference
   DebarmentsToLiftAfterPayment
4. Pay the fine you set in step 2
5. Note that the debarment is now lifted
Comment 2 Lari Taskula 2016-04-08 13:49:40 UTC
Created attachment 50084 [details] [review]
Bug 16223: Enable regex match for rules of debarment removals

This patch adds possibility to use regex to match debarments with dynamic comment.
Comment 3 Emmi Takkinen 2019-10-22 11:13:58 UTC
Tried to test this, got error:
error: short SHA1 bb92094 is ambiguous
hint: The candidates are:
hint:   bb92094b8c blob
hint:   bb92094f25 blob
error: sha1 information is lacking or useless (t/db_dependent/Patron/Borrower_Debarments.t).
error: could not build fake ancestor
Comment 4 Lari Taskula 2019-10-22 11:24:06 UTC
(In reply to Emmi Takkinen from comment #3)
> Tried to test this, got error:
> error: short SHA1 bb92094 is ambiguous
> hint: The candidates are:
> hint:   bb92094b8c blob
> hint:   bb92094f25 blob
> error: sha1 information is lacking or useless
> (t/db_dependent/Patron/Borrower_Debarments.t).
> error: could not build fake ancestor

Do you see this after your error:

> Patch failed at 0001 Bug 16223: Automatically remove any borrower debarments after a payment
> Use 'git am --show-current-patch' to see the failed patch
> When you have resolved this problem run "git bz apply --continue".
> If you would prefer to skip this patch, instead run "git bz apply --skip".
> To restore the original branch and stop patching run "git bz apply --abort".
> Patch left in /tmp/Bug-16223-Automatically-remove-any-borrower-debarm-EzxzUo.patch

Git fails to merge changes and is unable to continue. You need to continue manually. 

You can do the following:
1. git apply --reject /tmp/Bug-16223-Automatically-remove-any-borrower-debarm-EzxzUo.patch
2. It generates .rej file for every original file that failed, e.g. C4/Accounts.pm.rej
3. In this .rej file you can see the changes that are not yet applied
4. Merge the changes manually (by copypasting the lines to appropriate locations, if they are still valid)
Comment 5 Katrin Fischer 2019-11-24 12:48:54 UTC
Should this be "patch doesn't apply"?
Comment 6 Emmi Takkinen 2020-02-27 11:24:14 UTC
Created attachment 99697 [details] [review]
Bug 16223: Automatically remove any borrower debarments  after a payment

Rebased patch.
Comment 7 Emmi Takkinen 2020-02-27 11:25:03 UTC
Created attachment 99698 [details] [review]
Bug 16223: Enable regex match for rules of debarment

Rebased patch.
Comment 8 Emmi Takkinen 2020-05-06 06:52:42 UTC
Created attachment 104399 [details] [review]
Bug 16223: Automatically remove any borrower debarments  after a payment

Rebased patch.
Comment 9 Emmi Takkinen 2020-05-06 06:53:28 UTC
Created attachment 104400 [details] [review]
Bug 16223: Enable regex match for rules of debarment removals

Rebased patch.
Comment 10 Emmi Takkinen 2020-12-09 07:53:15 UTC
Created attachment 114272 [details] [review]
Bug 16223: Automatically remove any borrower debarments after a payment

Some libraries debar Patrons at the end of the year for having unpaid fines,
like in Bug 15157. Currently librarians have to manually remove this type of
debarments after Patron has paid his/her fines. Add a system preference to
define debarments that should be automatically removed after a payment is made,
and add functionality to actually remove the defined debarments from Patron.
Also let libraries to define the amount of outstanding fines after payment
after which the debarment will be removed.

This patch introduces a system preference DebarmentsToLiftAfterPayment, which
allows libraries to define rules for removing debarments after paying fines.
The system preference uses YAML and is defined as follows:

Debarment with this comments will be removed:
  outstanding: 5

Which means that if a Patron has a debarment "Debarment with this comment will
be removed", and he pays his fines and charges until his outstanding fees are
equal or less than 5.00, this debarment will be lifted. The parameter outstanding
is optional - if not given, the debarment will be lifted until Patron has paid
all of his outstanding fees (in other words, equal to "outstanding: 0").

To test:
1. Set a debarment to a Patron
2. Set a fine for Patron
3. Define rule(s) for removing debarment(s) in system preference
   DebarmentsToLiftAfterPayment
4. Pay the fine you set in step 2
5. Note that the debarment is now lifted

Also prove t/db_dependent/Patron/Borrower_Debarments.t

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Comment 11 Emmi Takkinen 2020-12-09 07:54:01 UTC
Created attachment 114273 [details] [review]
Bug 16223: Enable regex match for rules of debarment removals

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Comment 12 Emmi Takkinen 2020-12-09 07:55:18 UTC
Rebased patches and removed dependency to bug 14698.
Comment 13 ByWater Sandboxes 2021-03-19 12:54:25 UTC
Created attachment 118526 [details]
Bug 26940: Take empty strings into account

Signed-off-by: Katariina Hanhisalo <katariina.hanhisalo@xamk.fi>

Signed-off-by: Katariina Hanhisalo <katariina.hanhisalo@xamk.fi>
Comment 14 Katrin Fischer 2021-03-21 13:45:51 UTC
Switching this one to Signed off as well noticing the sandbox patches.
Comment 15 Emmi Takkinen 2021-03-22 06:41:17 UTC
Comment on attachment 118526 [details]
Bug 26940: Take empty strings into account

It seems one patch from bug 26940 has somehow slipped here. Actual patches still need sign off. Obsoleted patch and changing status back to Needs sign off. Katariina could you sign off again?
Comment 16 ByWater Sandboxes 2021-03-22 08:00:24 UTC
Created attachment 118572 [details] [review]
Bug 16223: Enable regex match for rules of debarment removals

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>

Signed-off-by: Katariina Hanhisalo <katariina.hanhisalo@xamk.fi>
Comment 17 ByWater Sandboxes 2021-03-22 08:03:07 UTC
Created attachment 118574 [details] [review]
Bug 16223: Automatically remove any borrower debarments after a payment

Some libraries debar Patrons at the end of the year for having unpaid fines,
like in Bug 15157. Currently librarians have to manually remove this type of
debarments after Patron has paid his/her fines. Add a system preference to
define debarments that should be automatically removed after a payment is made,
and add functionality to actually remove the defined debarments from Patron.
Also let libraries to define the amount of outstanding fines after payment
after which the debarment will be removed.

This patch introduces a system preference DebarmentsToLiftAfterPayment, which
allows libraries to define rules for removing debarments after paying fines.
The system preference uses YAML and is defined as follows:

Debarment with this comments will be removed:
  outstanding: 5

Which means that if a Patron has a debarment "Debarment with this comment will
be removed", and he pays his fines and charges until his outstanding fees are
equal or less than 5.00, this debarment will be lifted. The parameter outstanding
is optional - if not given, the debarment will be lifted until Patron has paid
all of his outstanding fees (in other words, equal to "outstanding: 0").

To test:
1. Set a debarment to a Patron
2. Set a fine for Patron
3. Define rule(s) for removing debarment(s) in system preference
   DebarmentsToLiftAfterPayment
4. Pay the fine you set in step 2
5. Note that the debarment is now lifted

Also prove t/db_dependent/Patron/Borrower_Debarments.t

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>

Signed-off-by: Katariina Hanhisalo <katariina.hanhisalo@xamk.fi>
Comment 18 ByWater Sandboxes 2021-03-22 08:03:09 UTC
Created attachment 118575 [details] [review]
Bug 16223: Enable regex match for rules of debarment removals

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>

Signed-off-by: Katariina Hanhisalo <katariina.hanhisalo@xamk.fi>

Signed-off-by: Katariina Hanhisalo <katariina.hanhisalo@xamk.fi>
Comment 19 Emmi Takkinen 2021-03-22 10:42:40 UTC
Setting back to signed offed.
Comment 20 Emmi Takkinen 2021-07-12 05:56:31 UTC
Created attachment 122768 [details] [review]
Bug 16223: Automatically remove any borrower debarments after a payment

Some libraries debar Patrons at the end of the year for having unpaid fines,
like in Bug 15157. Currently librarians have to manually remove this type of
debarments after Patron has paid his/her fines. Add a system preference to
define debarments that should be automatically removed after a payment is made,
and add functionality to actually remove the defined debarments from Patron.
Also let libraries to define the amount of outstanding fines after payment
after which the debarment will be removed.

This patch introduces a system preference DebarmentsToLiftAfterPayment, which
allows libraries to define rules for removing debarments after paying fines.
The system preference uses YAML and is defined as follows:

Debarment with this comments will be removed:
  outstanding: 5

Which means that if a Patron has a debarment "Debarment with this comment will
be removed", and he pays his fines and charges until his outstanding fees are
equal or less than 5.00, this debarment will be lifted. The parameter outstanding
is optional - if not given, the debarment will be lifted until Patron has paid
all of his outstanding fees (in other words, equal to "outstanding: 0").

To test:
1. Set a debarment to a Patron
2. Set a fine for Patron
3. Define rule(s) for removing debarment(s) in system preference
   DebarmentsToLiftAfterPayment
4. Pay the fine you set in step 2
5. Note that the debarment is now lifted

Also prove t/db_dependent/Patron/Borrower_Debarments.t

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>

Signed-off-by: Katariina Hanhisalo <katariina.hanhisalo@xamk.fi>
Comment 21 Emmi Takkinen 2021-07-12 05:57:04 UTC
Created attachment 122769 [details] [review]
Bug 16223: Enable regex match for rules of debarment removals

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>

Signed-off-by: Katariina Hanhisalo <katariina.hanhisalo@xamk.fi>

Signed-off-by: Katariina Hanhisalo <katariina.hanhisalo@xamk.fi>
Comment 22 Emmi Takkinen 2021-07-20 12:43:54 UTC
Oops, forgot to set this back to "Needs Signoff"...
Comment 23 Emmi Takkinen 2021-10-19 05:32:18 UTC
This is actually already signed off.
Comment 24 Katrin Fischer 2021-11-01 13:04:36 UTC
Hi Emmi,

my apply script shows a bit of ending whitespace and the QA test tools show an error I haven't seen before:

1) QA tools error

 FAIL	Koha/Patron/Debarments.pm
   OK	  critic
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   OK	  pod coverage
   OK	  spelling
   FAIL	  valid
		Inconsistent hierarchy during C3 merge of class 'Koha::Patron::Debarments':
			current merge results [
				Koha::Patron::Debarments,
			]
			merging failed on 'Exporter' 

Could you please check this out?

I've also done a first code review:

2) Please always use INSERT IGNORE when adding system preferences. This helps to keep them idempotent.

+    $dbh->do("INSERT INTO systempreferences (variable, value, options, explanation, type) VALUES ('DebarmentsToLiftAfterPayment', '', '', 'Lift these debarments after Borrower has paid his/her fees', 'textarea')");

3) Gender neutral code comments etc.

In English, you can use they/them/their to make your text gender neutral (wish it was that easy in German...)

Like here:
Lift these debarments after Borrower has paid his/her fees

You can say:
Lift these debarments after patron has paid their charges.

See: https://wiki.koha-community.org/wiki/Terminology
- prefer patron instead of borrower
- prefer charges instead of fines/fees (not added yet, but changed in GUI)

Similar for the written out pref text in the .yml file.

4) Unit tests pass :)
Comment 25 Emmi Takkinen 2021-11-02 09:38:59 UTC
Thanks Katrin for feedback! Points 2 and 3 are easy fix, but I don't really know what that QA tool error means or how to fix it (seems like similar error occurred in bug 23271, something about circular dependency). I'll look more into it, if someone knows how to fix this feel free to do so.
Comment 26 Katrin Fischer 2021-11-02 13:38:43 UTC
Adding Joubu und Tomas, maybe one of them can help with this one:

 FAIL	Koha/Patron/Debarments.pm
   OK	  critic
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   OK	  pod coverage
   OK	  spelling
   FAIL	  valid
		Inconsistent hierarchy during C3 merge of class 'Koha::Patron::Debarments':
			current merge results [
				Koha::Patron::Debarments,
			]
			merging failed on 'Exporter'
Comment 27 Jonathan Druart 2021-11-02 16:20:20 UTC
(In reply to Katrin Fischer from comment #26)
> Adding Joubu und Tomas, maybe one of them can help with this one:
> 
>  FAIL	Koha/Patron/Debarments.pm
>    OK	  critic
>    OK	  forbidden patterns
>    OK	  git manipulation
>    OK	  pod
>    OK	  pod coverage
>    OK	  spelling
>    FAIL	  valid
> 		Inconsistent hierarchy during C3 merge of class 'Koha::Patron::Debarments':
> 			current merge results [
> 				Koha::Patron::Debarments,
> 			]
> 			merging failed on 'Exporter'

Ignore it if it does not break the UI or tests. It should be fixed by bug 29395.
Comment 28 Emmi Takkinen 2021-11-03 13:27:28 UTC
Created attachment 127252 [details] [review]
Bug 16223: Fix QA issues

To test repeat original test plan and
prove t/db_dependent/Patron/Borrower_Debarments.t

Sponsored-by: Koha-Suomi Oy
Comment 29 Martin Renvoize 2021-11-03 16:18:10 UTC
Sorry guys.. this is a fail for me.

I think you need to move the function call from Koha::Account::pay to Koha::Account::Line::apply.. that way you'll catch any and all calls to apply payments (where you have it now, you'll miss any calls directly to apply, or calls to payin_amount.. and you'll miss cases where there's any additional outstanding credit to be applied from the payment).

Also, when autorenewal of items upon payment was added (a similar feature in my opinion).. we required that the user was given feedback of the change.. I'd love to see that here too.

I realise this is an old development being worked through.. but at least the first issue needs resolving prior to push in my opinion.. though I'd love to see the second dealt with.. I'd personally be OK with that being in a follow-up bug.

Failed QA
Comment 30 Emmi Takkinen 2021-11-24 11:22:05 UTC
(In reply to Martin Renvoize from comment #29)
> Also, when autorenewal of items upon payment was added (a similar feature in
> my opinion).. we required that the user was given feedback of the change..
> I'd love to see that here too.
> 

I tried to find which feature you mean with this but alas, I failed :D Could you point me the bug number of this so I could take a look?
Comment 31 Martin Renvoize 2021-11-25 05:52:16 UTC
It's bug 23051
Comment 32 Emmi Takkinen 2021-11-25 12:36:11 UTC
Created attachment 128005 [details] [review]
Bug 16223: Call DelDebarmentsAfterPayment from Koha::Account::Line::apply

Calling function from Koha::Account::pay missed several payment cases.
Moving it to Koha::Account::Line::apply should cover all payments. Also
reworked tests no just to test DelDebarmentsAfterPayment but to test
if debartments are lifted after pay.

To test repeat test plan from previous patches.
Also prove t/db_dependent/Patron/Borrower_Debartments.t

Sponsored-by: Koha-Suomi Oy
Comment 33 Katrin Fischer 2022-03-26 23:56:46 UTC
Hi Emmi, patches no longer apply, could you please rebase?

The script notes some trailing whitespace also, maybe worth checking. 

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 16223: Automatically remove any borrower debarments after a payment
.git/rebase-apply/patch:28: trailing whitespace.
    
.git/rebase-apply/patch:160: trailing whitespace.
     - 
warning: 2 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	Koha/Account.pm
M	Koha/Patron/Debarments.pm
M	installer/data/mysql/mandatory/sysprefs.sql
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
M	t/db_dependent/Patron/Borrower_Debarments.t
.git/rebase-apply/patch:28: trailing whitespace.
    
.git/rebase-apply/patch:160: trailing whitespace.
     - 
warning: 2 lines applied after fixing whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Patron/Borrower_Debarments.t
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
Auto-merging installer/data/mysql/mandatory/sysprefs.sql
Auto-merging Koha/Patron/Debarments.pm
CONFLICT (content): Merge conflict in Koha/Patron/Debarments.pm
Auto-merging Koha/Account.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 16223: Automatically remove any borrower debarments after a payment
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-16223-Automatically-remove-any-borrower-debarm-iuWbtz.p
Comment 34 Emmi Takkinen 2022-03-28 06:16:44 UTC
Created attachment 132284 [details] [review]
Bug 16223: Automatically remove any borrower debarments after a payment

Some libraries debar Patrons at the end of the year for having unpaid fines,
like in Bug 15157. Currently librarians have to manually remove this type of
debarments after Patron has paid his/her fines. Add a system preference to
define debarments that should be automatically removed after a payment is made,
and add functionality to actually remove the defined debarments from Patron.
Also let libraries to define the amount of outstanding fines after payment
after which the debarment will be removed.

This patch introduces a system preference DebarmentsToLiftAfterPayment, which
allows libraries to define rules for removing debarments after paying fines.
The system preference uses YAML and is defined as follows:

Debarment with this comments will be removed:
  outstanding: 5

Which means that if a Patron has a debarment "Debarment with this comment will
be removed", and he pays his fines and charges until his outstanding fees are
equal or less than 5.00, this debarment will be lifted. The parameter outstanding
is optional - if not given, the debarment will be lifted until Patron has paid
all of his outstanding fees (in other words, equal to "outstanding: 0").

To test:
1. Set a debarment to a Patron
2. Set a fine for Patron
3. Define rule(s) for removing debarment(s) in system preference
   DebarmentsToLiftAfterPayment
4. Pay the fine you set in step 2
5. Note that the debarment is now lifted

Also prove t/db_dependent/Patron/Borrower_Debarments.t

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>

Signed-off-by: Katariina Hanhisalo <katariina.hanhisalo@xamk.fi>
Comment 35 Emmi Takkinen 2022-03-28 06:17:16 UTC
Created attachment 132285 [details] [review]
Bug 16223: Enable regex match for rules of debarment removals

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>

Signed-off-by: Katariina Hanhisalo <katariina.hanhisalo@xamk.fi>

Signed-off-by: Katariina Hanhisalo <katariina.hanhisalo@xamk.fi>
Comment 36 Emmi Takkinen 2022-03-28 06:17:41 UTC
Created attachment 132286 [details] [review]
Bug 16223: Fix QA issues

To test repeat original test plan and
prove t/db_dependent/Patron/Borrower_Debarments.t

Sponsored-by: Koha-Suomi Oy
Comment 37 Emmi Takkinen 2022-03-28 06:18:10 UTC
Created attachment 132287 [details] [review]
Bug 16223: Call DelDebarmentsAfterPayment from  Koha::Account::Line::apply

Calling function from Koha::Account::pay missed several payment cases.
Moving it to Koha::Account::Line::apply should cover all payments. Also
reworked tests no just to test DelDebarmentsAfterPayment but to test
if debartments are lifted after pay.

To test repeat test plan from previous patches.
Also prove t/db_dependent/Patron/Borrower_Debartments.t

Sponsored-by: Koha-Suomi Oy
Comment 38 Emmi Takkinen 2022-03-28 06:20:20 UTC
Rebased patches. Couldn't find trailing whitespaces after applying patches, but found a extra =cut on last patch.
Comment 40 Katrin Fischer 2022-04-03 13:20:05 UTC
Hi Emmi, 

thanks for the rebase, starting to dig a little deeper into this now:

1) DelDebarmentsAfterPayment 

According to coding guidelines the methods in Koha namespace should always be snake case.

2) Terminology

debarment --> restriction

In general, and that possibly applies to above as well, we should use the term "restrictions". See: bug 5268 for an explanation.

borrower --> patron

I know you can't change all spots as the table is called borrowers_debarments, but I think it would be good to change whereever possible.

Example:
Lift these debarments after Borrower has paid their charges >
Lift these restrictions after a patron has paid their charges

See also: https://wiki.koha-community.org/wiki/Terminology

3) Number of tests

The number of tests needs to be updated to 37 for tests to pass :)

4) Functionality

A thing I stumbled on is that this works on the restriction comment and not the type of restriction. I believe this is done, because we can't have different types of restrictions currently and this makes it more flexible. But I am not sure if it will not also create issues.

Currently the comment can only be set, if the type of restriction is MANUAL, for DISCHARGE it will be empty, possibly for OVERDUES and SUSPENSION as well. We cannot edit restrictions to add a certain comment or fix a typo.

I recently reviewed bug 23681 and I think this goes in a more maintainable direction. It will allow to define restriction types in a new table with some being Koha-internal and others being added in addition. 

And what you do with the YAML pref could then be moved to be columns in the new table, which would make it easy to query, easy to handle for staff, etc. and remove any issues with variations of spelling/whitespace by having a list to pick from.

I know this is the older dev, but maybe have a look at bug 23681?

It's currently waiting for some more work, but I think it would be still achievable for the next version, especially with a little help.
Comment 41 Emmi Takkinen 2022-09-14 11:41:21 UTC
(In reply to Katrin Fischer from comment #40)
> Hi Emmi, 
> 
> I recently reviewed bug 23681 and I think this goes in a more maintainable
> direction. It will allow to define restriction types in a new table with
> some being Koha-internal and others being added in addition. 
> 
> And what you do with the YAML pref could then be moved to be columns in the
> new table, which would make it easy to query, easy to handle for staff, etc.
> and remove any issues with variations of spelling/whitespace by having a
> list to pick from.
> 

Bug 23681 is in master now, so I'll take a look at this.
Comment 42 Emmi Takkinen 2022-11-08 09:44:42 UTC
Created attachment 143388 [details] [review]
Bug 16223: Add new columns to table restriction_types

This patch adds two new columns, lift_after_payment and fee_limit,
to table restriction_types. These colums are used to control
lifting patrons restrictions after paying their fees.

To test:
1. Apply this patch.
2. Update your database via updatedatabase.pl.
=> Confirm your table restriction_types has two new columns.

Sponsored-by: Koha-Suomi Oy
Comment 43 Emmi Takkinen 2022-11-08 09:45:14 UTC
Created attachment 143389 [details] [review]
Bug 16223: DO NOT PUSH! Schema changes

Sponsored-by: Koha-Suomi Oy
Comment 44 Emmi Takkinen 2022-11-08 09:45:46 UTC
Created attachment 143390 [details] [review]
Bug 16223: Add ability to define liftable restrictions

Some libraries debar Patrons at the end of the year for having unpaid fines,
like in Bug 15157. Currently librarians have to manually remove this type of
debarments after Patron has paid his/her fines.

This patch adds ability to create restrictions which are lifted after
patron pays ceratain amount of fines.

To test:
1. Apply this patch.
2. Restart your services if needed.
3. Navigate to page restrictions.pl.
=> Note that table has two new colums in it, "Lift after payment?" and "Fee limit".
4. Add new restriction which has "Lift after payment?" set
as Yes and fee limit as 5.
5. Create fees for a patron so they exceed fee limit e.g. 10
6. Add restriction made in step 2. for the patron
7. Pay patrons fees partially so that they go under fee limit
=> Note that patrons restriction should now be lifted.

Also prove t/db_dependent/Patron/Borrower_Debarments.t.

Sponsored-by: Koha-Suomi Oy
Comment 45 Andrew Fuerste-Henry 2022-11-08 14:42:16 UTC
The third patch here just failed to apply for me on a sandbox. Not sure what might have changed, given how recently you posted it.
Comment 46 Emmi Takkinen 2022-11-09 08:17:43 UTC
(In reply to Andrew Fuerste-Henry from comment #45)
> The third patch here just failed to apply for me on a sandbox. Not sure what
> might have changed, given how recently you posted it.

That's odd, rebasing or applying patches via bz-tool doesn't give merge conflict on my test environment. I also asked one our librarians to test this on sandbox (she used ptfs-europen sandbox) and she had no issues. But in Bywaters sandbox she got following error when she opened restrictions.pl: 

Template process failed: undef error - The method Koha::Patron::Restriction::Type->lift_after_payment is not covered by tests!

Seems like schema update problem to me.

Also there seems to be some typos and need for improvement on last patches test plan. Fixing that asap.
Comment 47 Emmi Takkinen 2022-11-09 08:26:02 UTC
Created attachment 143462 [details] [review]
Bug 16223: Add ability to define liftable restrictions

Some libraries debar Patrons at the end of the year for having unpaid fines,
like in Bug 15157. Currently librarians have to manually remove this type of
debarments after Patron has paid his/her fines.

This patch adds ability to create restrictions which are lifted after
patron pays ceratain amount of fines.

To test:
1. Apply this patch.
2. Restart your services if needed.
3. Navigate to page restrictions.pl, Administration->Patron restrictions.
=> Note that table has two new colums in it, "Lift after payment?" and "Fee limit".
4. Add new restriction which has "Lift after payment?" set
as Yes and fee limit as 5.
5. Create fees for a patron so they exceed fee limit e.g. 10
6. Add restriction made in step 4. for the patron
7. Pay patrons fees partially so that they go under fee limit
=> Note that patrons restriction should now be lifted.

Also prove t/db_dependent/Patron/Borrower_Debarments.t.

Sponsored-by: Koha-Suomi Oy
Comment 48 Emmi Takkinen 2022-11-09 08:29:06 UTC
Test plan updated. Setting this back to "Needs Signoff" since patches seem to apply nicely apart from ByWaters sandbox.
Comment 49 Andrew Fuerste-Henry 2022-11-09 21:19:31 UTC
I got the same error on the BibLibre sandboxes for some reason. Spinning one up on PTFS-Europe's site now.
Comment 50 PTFS Europe Sandboxes 2022-11-10 14:49:58 UTC
Created attachment 143670 [details] [review]
Bug 16223: Add new columns to table restriction_types

This patch adds two new columns, lift_after_payment and fee_limit,
to table restriction_types. These colums are used to control
lifting patrons restrictions after paying their fees.

To test:
1. Apply this patch.
2. Update your database via updatedatabase.pl.
=> Confirm your table restriction_types has two new columns.

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Comment 51 PTFS Europe Sandboxes 2022-11-10 14:50:04 UTC
Created attachment 143671 [details] [review]
Bug 16223: DO NOT PUSH! Schema changes

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Comment 52 PTFS Europe Sandboxes 2022-11-10 14:50:10 UTC
Created attachment 143672 [details] [review]
Bug 16223: Add ability to define liftable restrictions

Some libraries debar Patrons at the end of the year for having unpaid fines,
like in Bug 15157. Currently librarians have to manually remove this type of
debarments after Patron has paid his/her fines.

This patch adds ability to create restrictions which are lifted after
patron pays ceratain amount of fines.

To test:
1. Apply this patch.
2. Restart your services if needed.
3. Navigate to page restrictions.pl.
=> Note that table has two new colums in it, "Lift after payment?" and "Fee limit".
4. Add new restriction which has "Lift after payment?" set
as Yes and fee limit as 5.
5. Create fees for a patron so they exceed fee limit e.g. 10
6. Add restriction made in step 2. for the patron
7. Pay patrons fees partially so that they go under fee limit
=> Note that patrons restriction should now be lifted.

Also prove t/db_dependent/Patron/Borrower_Debarments.t.

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Comment 53 Martin Renvoize 2022-11-10 15:54:39 UTC
Oh wow, Emmi, you star!

Thanks for the full rework here.. my apologies I missed the action had taken place.. the code it looking great..

I'm going to work through it now.. I'm afraid it's just missed 22.11 freezes but hopefully we can get it pushed as one of the first features for 23.05!
Comment 54 Martin Renvoize 2022-11-10 16:13:26 UTC
Hmm... I've not actually tested this yet.. just applied it and done a minor rebase.. reading the code it look like it won't work quite yet and it also looks like the follow-up to move from 'pay' to 'apply' is missing.

I'll test and provide a follow-up myself now.
Comment 55 Martin Renvoize 2022-11-10 16:19:02 UTC
Created attachment 143688 [details] [review]
Bug 16223: Add new columns to table restriction_types

This patch adds two new columns, lift_after_payment and fee_limit,
to table restriction_types. These colums are used to control
lifting patrons restrictions after paying their fees.

To test:
1. Apply this patch.
2. Update your database via updatedatabase.pl.
=> Confirm your table restriction_types has two new columns.

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Comment 56 Martin Renvoize 2022-11-10 16:19:08 UTC
Created attachment 143689 [details] [review]
Bug 16223: DO NOT PUSH! Schema changes

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Comment 57 Martin Renvoize 2022-11-10 16:19:13 UTC
Created attachment 143690 [details] [review]
Bug 16223: Add ability to define liftable restrictions

Some libraries debar Patrons at the end of the year for having unpaid fines,
like in Bug 15157. Currently librarians have to manually remove this type of
debarments after Patron has paid his/her fines.

This patch adds ability to create restrictions which are lifted after
patron pays ceratain amount of fines.

To test:
1. Apply this patch.
2. Restart your services if needed.
3. Navigate to page restrictions.pl.
=> Note that table has two new colums in it, "Lift after payment?" and "Fee limit".
4. Add new restriction which has "Lift after payment?" set
as Yes and fee limit as 5.
5. Create fees for a patron so they exceed fee limit e.g. 10
6. Add restriction made in step 2. for the patron
7. Pay patrons fees partially so that they go under fee limit
=> Note that patrons restriction should now be lifted.

Also prove t/db_dependent/Patron/Borrower_Debarments.t.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Comment 58 Martin Renvoize 2022-11-11 10:46:44 UTC
Created attachment 143765 [details] [review]
Bug 16223: Add new columns to table restriction_types

This patch adds two new columns, lift_after_payment and fee_limit,
to table restriction_types. These colums are used to control
lifting patrons restrictions after paying their fees.

To test:
1. Apply this patch.
2. Update your database via updatedatabase.pl.
=> Confirm your table restriction_types has two new columns.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Comment 59 Martin Renvoize 2022-11-11 10:46:49 UTC
Created attachment 143766 [details] [review]
Bug 16223: DO NOT PUSH! Schema changes

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Comment 60 Martin Renvoize 2022-11-11 10:46:55 UTC
Created attachment 143767 [details] [review]
Bug 16223: Add ability to define liftable restrictions

Some libraries debar Patrons at the end of the year for having unpaid fines,
like in Bug 15157. Currently librarians have to manually remove this type of
debarments after Patron has paid his/her fines.

This patch adds ability to create restrictions which are lifted after
patron pays ceratain amount of fines.

To test:
1. Apply this patch.
2. Restart your services if needed.
3. Navigate to page restrictions.pl.
=> Note that table has two new colums in it, "Lift after payment?" and "Fee limit".
4. Add new restriction which has "Lift after payment?" set
as Yes and fee limit as 5.
5. Create fees for a patron so they exceed fee limit e.g. 10
6. Add restriction made in step 2. for the patron
7. Pay patrons fees partially so that they go under fee limit
=> Note that patrons restriction should now be lifted.

Also prove t/db_dependent/Patron/Borrower_Debarments.t.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Comment 61 Martin Renvoize 2022-11-11 10:47:00 UTC
Created attachment 143768 [details] [review]
Bug 16223: (QA follow-up) Fix kohastructure bug
Comment 62 Martin Renvoize 2022-11-11 10:47:06 UTC
Created attachment 143769 [details] [review]
Bug 16223: (QA follow-up) Remove GetDebarments

We remove the GetDebarments routine in 31095.
Comment 63 Martin Renvoize 2022-11-11 10:47:12 UTC
Created attachment 143770 [details] [review]
Bug 16223: (QA follow-up) Move trigger to apply

This patch moves the del_restrictions_after_payment from before the
actual credit application call inside Koha::Account::pay to after the
application of credits to debits in Koha::Account::Line::apply.
Comment 64 Martin Renvoize 2022-11-11 10:49:11 UTC
I've rebased this and taken bug 31095 into account too.. 

I'd love to get an independent sign-off on this if your still interested in testing it  Andrew Fuerste-Henry ?
Comment 65 Andrew Fuerste-Henry 2022-11-14 16:11:39 UTC
With these new patches the restriction is no longer being lifting on payment. I see that my restriction type exists, is marked to lift on payment, with a fee limit of 5.000000. On two different patrons I added a $10 fee, created a restriction of my created type, and then paid the fine down to under $5. In both cases, the restriction was not removed.
Comment 66 Emmi Takkinen 2022-11-15 13:22:55 UTC
Thank you Martin and Andrew for looking into this. I'll try to work on this again tomorrow.
Comment 67 Emmi Takkinen 2022-11-16 07:07:57 UTC
Hmm, it seems code returns only "MANUAL" restriction. Tests however return correct type. So for some reason Koha saves restrictions incorrectly from UI.
Comment 68 Emmi Takkinen 2022-11-16 07:30:32 UTC
Created attachment 143936 [details] [review]
Bug 16223: Add correct value attributes to restriction options in select element

Restrictions weren't lift after payment because value
attributes for restrictions code were missing from
restriction select element in patron-restrictions-tab.inc.
Due this restrictions were always set as type "MANUAL".

To test:
1. Add new restriction which has "Lift after payment?" set
as Yes and fee limit as 5.
2. Create fees for a patron so they exceed fee limit e.g. 10
3. Add restriction made in step 1. for the patron
4. Pay patrons fees partially so that they go under fee limit
=> Note that patrons restriction should now be lifted.

Also prove t/db_dependent/Patron/Borrower_Debarments.t.

Sponsored-by: Koha-Suomi Oy
Comment 69 Emmi Takkinen 2023-02-03 06:42:30 UTC
Created attachment 146051 [details] [review]
Bug 16223: Add new columns to table restriction_types

This patch adds two new columns, lift_after_payment and fee_limit,
to table restriction_types. These colums are used to control
lifting patrons restrictions after paying their fees.

To test:
1. Apply this patch.
2. Update your database via updatedatabase.pl.
=> Confirm your table restriction_types has two new columns.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Comment 70 Emmi Takkinen 2023-02-03 06:43:03 UTC
Created attachment 146052 [details] [review]
Bug 16223: DO NOT PUSH! Schema changes

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Comment 71 Emmi Takkinen 2023-02-03 06:43:37 UTC
Created attachment 146053 [details] [review]
Bug 16223: Add ability to define liftable restrictions

Some libraries debar Patrons at the end of the year for having unpaid fines,
like in Bug 15157. Currently librarians have to manually remove this type of
debarments after Patron has paid his/her fines.

This patch adds ability to create restrictions which are lifted after
patron pays ceratain amount of fines.

To test:
1. Apply this patch.
2. Restart your services if needed.
3. Navigate to page restrictions.pl.
=> Note that table has two new colums in it, "Lift after payment?" and "Fee limit".
4. Add new restriction which has "Lift after payment?" set
as Yes and fee limit as 5.
5. Create fees for a patron so they exceed fee limit e.g. 10
6. Add restriction made in step 2. for the patron
7. Pay patrons fees partially so that they go under fee limit
=> Note that patrons restriction should now be lifted.

Also prove t/db_dependent/Patron/Borrower_Debarments.t.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Comment 72 Emmi Takkinen 2023-02-03 06:44:04 UTC
Created attachment 146054 [details] [review]
Bug 16223: (QA follow-up) Fix kohastructure bug
Comment 73 Emmi Takkinen 2023-02-03 06:44:38 UTC
Created attachment 146055 [details] [review]
Bug 16223: (QA follow-up) Remove GetDebarments

We remove the GetDebarments routine in 31095.
Comment 74 Emmi Takkinen 2023-02-03 06:45:12 UTC
Created attachment 146056 [details] [review]
Bug 16223: (QA follow-up) Move trigger to apply

This patch moves the del_restrictions_after_payment from before the
actual credit application call inside Koha::Account::pay to after the
application of credits to debits in Koha::Account::Line::apply.
Comment 75 Emmi Takkinen 2023-02-03 06:45:46 UTC
Created attachment 146057 [details] [review]
Bug 16223: Add correct value attributes to restriction options in select element

Restrictions weren't lift after payment because value
attributes for restrictions code were missing from
restriction select element in patron-restrictions-tab.inc.
Due this restrictions were always set as type "MANUAL".

To test:
1. Add new restriction which has "Lift after payment?" set
as Yes and fee limit as 5.
2. Create fees for a patron so they exceed fee limit e.g. 10
3. Add restriction made in step 1. for the patron
4. Pay patrons fees partially so that they go under fee limit
=> Note that patrons restriction should now be lifted.

Also prove t/db_dependent/Patron/Borrower_Debarments.t.

Sponsored-by: Koha-Suomi Oy
Comment 76 Emmi Takkinen 2023-03-16 06:59:15 UTC
I got a comment that "Fee limit" should probably not be always required. Which got me thinking that maybe it should be required only if "Lift after payment" is set to "Yes". Or then it should always default to zero. Does anyone have any other suggestions?
Comment 77 Emmi Takkinen 2023-03-17 08:31:42 UTC
Created attachment 148307 [details] [review]
Bug 16223: Add new columns to table restriction_types

This patch adds two new columns, lift_after_payment and fee_limit,
to table restriction_types. These colums are used to control
lifting patrons restrictions after paying their fees.

To test:
1. Apply this patch.
2. Update your database via updatedatabase.pl.
=> Confirm your table restriction_types has two new columns.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Comment 78 Emmi Takkinen 2023-03-17 08:32:10 UTC
Created attachment 148308 [details] [review]
Bug 16223: DO NOT PUSH! Schema changes

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Comment 79 Emmi Takkinen 2023-03-17 08:32:38 UTC
Created attachment 148309 [details] [review]
Bug 16223: Add ability to define liftable restrictions

Some libraries debar Patrons at the end of the year for having unpaid fines,
like in Bug 15157. Currently librarians have to manually remove this type of
debarments after Patron has paid his/her fines.

This patch adds ability to create restrictions which are lifted after
patron pays ceratain amount of fines.

To test:
1. Apply this patch.
2. Restart your services if needed.
3. Navigate to page restrictions.pl.
=> Note that table has two new colums in it, "Lift after payment?" and "Fee limit".
4. Add new restriction which has "Lift after payment?" set
as Yes and fee limit as 5.
5. Create fees for a patron so they exceed fee limit e.g. 10
6. Add restriction made in step 2. for the patron
7. Pay patrons fees partially so that they go under fee limit
=> Note that patrons restriction should now be lifted.

Also prove t/db_dependent/Patron/Borrower_Debarments.t.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Comment 80 Emmi Takkinen 2023-03-17 08:33:07 UTC
Created attachment 148310 [details] [review]
Bug 16223: (QA follow-up) Remove GetDebarments

We remove the GetDebarments routine in 31095.
Comment 81 Emmi Takkinen 2023-03-17 08:34:27 UTC
Created attachment 148311 [details] [review]
Bug 16223: (QA follow-up) Move trigger to apply

This patch moves the del_restrictions_after_payment from before the
actual credit application call inside Koha::Account::pay to after the
application of credits to debits in Koha::Account::Line::apply.
Comment 82 Emmi Takkinen 2023-03-17 08:34:59 UTC
Created attachment 148312 [details] [review]
Bug 16223: Add correct value attributes to restriction  options in select element

Restrictions weren't lift after payment because value
attributes for restrictions code were missing from
restriction select element in patron-restrictions-tab.inc.
Due this restrictions were always set as type "MANUAL".

To test:
1. Add new restriction which has "Lift after payment?" set
as Yes and fee limit as 5.
2. Create fees for a patron so they exceed fee limit e.g. 10
3. Add restriction made in step 1. for the patron
4. Pay patrons fees partially so that they go under fee limit
=> Note that patrons restriction should now be lifted.

Also prove t/db_dependent/Patron/Borrower_Debarments.t.

Sponsored-by: Koha-Suomi Oy
Comment 83 Emmi Takkinen 2023-03-17 08:35:27 UTC
Created attachment 148313 [details] [review]
Bug 16223: Require fee limit only if "Lift after payment" is set as "Yes"

Fee limit should only be required if restriction is
meant to be lifted after payment. This patch makes
"Fee limit" input field required based on value of
"Lift after payment" select field.

To test:
1. Create a new restriction.
2. "Lift after payment" should be set as "No".
3. Change value as "Yes".
=> "Fee limit" should now have "Required" text
after input field.
4. Try to save restriction.
=> Text "This field is required" should be
displayed after "Fee limit".
5. Change "Lift after payment" back to "No".
=> Both texts should now be lifted.

Also test with existing restriction and make sure
restrictions are still saved without problems.

Sponsored-by: Koha-Suomi Oy
Comment 84 Martin Renvoize 2023-04-21 10:40:09 UTC
Created attachment 150024 [details] [review]
Bug 16223: Add new columns to table restriction_types

This patch adds two new columns, lift_after_payment and fee_limit,
to table restriction_types. These colums are used to control
lifting patrons restrictions after paying their fees.

To test:
1. Apply this patch.
2. Update your database via updatedatabase.pl.
=> Confirm your table restriction_types has two new columns.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 85 Martin Renvoize 2023-04-21 10:40:13 UTC
Created attachment 150025 [details] [review]
Bug 16223: DO NOT PUSH! Schema changes

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 86 Martin Renvoize 2023-04-21 10:40:16 UTC
Created attachment 150026 [details] [review]
Bug 16223: Add ability to define liftable restrictions

Some libraries debar Patrons at the end of the year for having unpaid fines,
like in Bug 15157. Currently librarians have to manually remove this type of
debarments after Patron has paid his/her fines.

This patch adds ability to create restrictions which are lifted after
patron pays ceratain amount of fines.

To test:
1. Apply this patch.
2. Restart your services if needed.
3. Navigate to page restrictions.pl.
=> Note that table has two new colums in it, "Lift after payment?" and "Fee limit".
4. Add new restriction which has "Lift after payment?" set
as Yes and fee limit as 5.
5. Create fees for a patron so they exceed fee limit e.g. 10
6. Add restriction made in step 2. for the patron
7. Pay patrons fees partially so that they go under fee limit
=> Note that patrons restriction should now be lifted.

Also prove t/db_dependent/Patron/Borrower_Debarments.t.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 87 Martin Renvoize 2023-04-21 10:40:20 UTC
Created attachment 150027 [details] [review]
Bug 16223: (QA follow-up) Remove GetDebarments

We remove the GetDebarments routine in 31095.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 88 Martin Renvoize 2023-04-21 10:40:24 UTC
Created attachment 150028 [details] [review]
Bug 16223: (QA follow-up) Move trigger to apply

This patch moves the del_restrictions_after_payment from before the
actual credit application call inside Koha::Account::pay to after the
application of credits to debits in Koha::Account::Line::apply.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 89 Martin Renvoize 2023-04-21 10:40:28 UTC
Created attachment 150029 [details] [review]
Bug 16223: Require fee limit only if "Lift after payment" is set as "Yes"

Fee limit should only be required if restriction is
meant to be lifted after payment. This patch makes
"Fee limit" input field required based on value of
"Lift after payment" select field.

To test:
1. Create a new restriction.
2. "Lift after payment" should be set as "No".
3. Change value as "Yes".
=> "Fee limit" should now have "Required" text
after input field.
4. Try to save restriction.
=> Text "This field is required" should be
displayed after "Fee limit".
5. Change "Lift after payment" back to "No".
=> Both texts should now be lifted.

Also test with existing restriction and make sure
restrictions are still saved without problems.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 90 Emmi Takkinen 2023-07-05 06:26:14 UTC
First patch has been resolved in bug 33574.
Comment 91 Emmi Takkinen 2023-07-05 06:36:35 UTC
Created attachment 153024 [details] [review]
Bug 16223: Add new columns to table restriction_types

This patch adds two new columns, lift_after_payment and fee_limit,
to table restriction_types. These colums are used to control
lifting patrons restrictions after paying their fees.

To test:
1. Apply this patch.
2. Update your database via updatedatabase.pl.
=> Confirm your table restriction_types has two new columns.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 92 Emmi Takkinen 2023-07-05 06:37:07 UTC
Created attachment 153025 [details] [review]
Bug 16223: DO NOT PUSH! Schema changes

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 93 Emmi Takkinen 2023-07-05 06:37:38 UTC
Created attachment 153026 [details] [review]
Bug 16223: Add ability to define liftable restrictions

Some libraries debar Patrons at the end of the year for having unpaid fines,
like in Bug 15157. Currently librarians have to manually remove this type of
debarments after Patron has paid his/her fines.

This patch adds ability to create restrictions which are lifted after
patron pays ceratain amount of fines.

To test:
1. Apply this patch.
2. Restart your services if needed.
3. Navigate to page restrictions.pl.
=> Note that table has two new colums in it, "Lift after payment?" and "Fee limit".
4. Add new restriction which has "Lift after payment?" set
as Yes and fee limit as 5.
5. Create fees for a patron so they exceed fee limit e.g. 10
6. Add restriction made in step 2. for the patron
7. Pay patrons fees partially so that they go under fee limit
=> Note that patrons restriction should now be lifted.

Also prove t/db_dependent/Patron/Borrower_Debarments.t.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 94 Emmi Takkinen 2023-07-05 06:38:11 UTC
Created attachment 153027 [details] [review]
Bug 16223: (QA follow-up) Remove GetDebarments

We remove the GetDebarments routine in 31095.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 95 Emmi Takkinen 2023-07-05 06:38:45 UTC
Created attachment 153028 [details] [review]
Bug 16223: (QA follow-up) Move trigger to apply

Fee limit should only be required if restriction is
meant to be lifted after payment. This patch makes
"Fee limit" input field required based on value of
"Lift after payment" select field.

To test:
1. Create a new restriction.
2. "Lift after payment" should be set as "No".
3. Change value as "Yes".
=> "Fee limit" should now have "Required" text
after input field.
4. Try to save restriction.
=> Text "This field is required" should be
displayed after "Fee limit".
5. Change "Lift after payment" back to "No".
=> Both texts should now be lifted.

Also test with existing restriction and make sure
restrictions are still saved without problems.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 96 Emmi Takkinen 2023-07-05 06:40:13 UTC
Comment on attachment 153028 [details] [review]
Bug 16223: (QA follow-up) Move trigger to apply

This patch moves the del_restrictions_after_payment from before the
actual credit application call inside Koha::Account::pay to after the
application of credits to debits in Koha::Account::Line::apply.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 97 Emmi Takkinen 2023-07-05 06:41:10 UTC
Created attachment 153029 [details] [review]
Bug 16223: Require fee limit only if "Lift after payment" is set as "Yes"

Fee limit should only be required if restriction is
meant to be lifted after payment. This patch makes
"Fee limit" input field required based on value of
"Lift after payment" select field.

To test:
1. Create a new restriction.
2. "Lift after payment" should be set as "No".
3. Change value as "Yes".
=> "Fee limit" should now have "Required" text
after input field.
4. Try to save restriction.
=> Text "This field is required" should be
displayed after "Fee limit".
5. Change "Lift after payment" back to "No".
=> Both texts should now be lifted.

Also test with existing restriction and make sure
restrictions are still saved without problems.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 98 Emmi Takkinen 2023-07-05 06:41:57 UTC
Rebased patches, setting status back to "Signed off".
Comment 99 Marcel de Rooy 2023-09-15 08:31:18 UTC
Looking here
Comment 100 Marcel de Rooy 2023-09-15 09:18:59 UTC
Created attachment 155644 [details] [review]
Bug 16223: Add new columns to table restriction_types

This patch adds two new columns, lift_after_payment and fee_limit,
to table restriction_types. These colums are used to control
lifting patrons restrictions after paying their fees.

To test:
1. Apply this patch.
2. Update your database via updatedatabase.pl.
=> Confirm your table restriction_types has two new columns.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 101 Marcel de Rooy 2023-09-15 09:19:03 UTC
Created attachment 155645 [details] [review]
Bug 16223: DO NOT PUSH! Schema changes

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 102 Marcel de Rooy 2023-09-15 09:19:06 UTC
Created attachment 155646 [details] [review]
Bug 16223: Add ability to define liftable restrictions

Some libraries debar Patrons at the end of the year for having unpaid fines,
like in Bug 15157. Currently librarians have to manually remove this type of
debarments after Patron has paid his/her fines.

This patch adds ability to create restrictions which are lifted after
patron pays ceratain amount of fines.

To test:
1. Apply this patch.
2. Restart your services if needed.
3. Navigate to page restrictions.pl.
=> Note that table has two new colums in it, "Lift after payment?" and "Fee limit".
4. Add new restriction which has "Lift after payment?" set
as Yes and fee limit as 5.
5. Create fees for a patron so they exceed fee limit e.g. 10
6. Add restriction made in step 2. for the patron
7. Pay patrons fees partially so that they go under fee limit
=> Note that patrons restriction should now be lifted.

Also prove t/db_dependent/Patron/Borrower_Debarments.t.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 103 Marcel de Rooy 2023-09-15 09:19:10 UTC
Created attachment 155647 [details] [review]
Bug 16223: (QA follow-up) Remove GetDebarments

We remove the GetDebarments routine in 31095.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 104 Marcel de Rooy 2023-09-15 09:19:14 UTC
Created attachment 155648 [details] [review]
Bug 16223: (QA follow-up) Move trigger to apply

This patch moves the del_restrictions_after_payment from before the
actual credit application call inside Koha::Account::pay to after the
application of credits to debits in Koha::Account::Line::apply.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 105 Marcel de Rooy 2023-09-15 09:19:17 UTC
Created attachment 155649 [details] [review]
Bug 16223: Require fee limit only if "Lift after payment" is set as "Yes"

Fee limit should only be required if restriction is
meant to be lifted after payment. This patch makes
"Fee limit" input field required based on value of
"Lift after payment" select field.

To test:
1. Create a new restriction.
2. "Lift after payment" should be set as "No".
3. Change value as "Yes".
=> "Fee limit" should now have "Required" text
after input field.
4. Try to save restriction.
=> Text "This field is required" should be
displayed after "Fee limit".
5. Change "Lift after payment" back to "No".
=> Both texts should now be lifted.

Also test with existing restriction and make sure
restrictions are still saved without problems.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 106 Marcel de Rooy 2023-09-15 09:19:21 UTC
Created attachment 155650 [details] [review]
Bug 16223: (QA follow-up) Add filter, remove superfluous condition

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 107 Marcel de Rooy 2023-09-15 09:19:43 UTC
Looks good to me.

FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/admin/restrictions.tt
   FAIL   filters
                missing_filter at line 217 (                                [% IF restriction.fee_limit %][% restriction.fee_limit %][% END %])
Added follow-up.

-        if ($dupe->count) {
+        if ($dupe->count && $dupe->unblessed->{code} ne $code) {
        my $dupe = Koha::Patron::Restriction::Types->search(
            {
                code         => { '!=' => $code },
                display_text => $display_text,
            }
        );
        if ($dupe->count && $dupe->unblessed->{code} ne $code) {
Look above. This change is unneeded. Removed it.

I was just wondering a bit about the column name fee_limit. 
The crux to understanding what it does, is this test: $total_due <= $restriction->type->fee_limit
But I dont have a better name at hand. Could imagine that this field name is not immediately clear to people.
Comment 108 Marcel de Rooy 2023-09-15 09:19:53 UTC
Note for RM:
Please watch for conflicts:
Bug 29145 - Allow patrons to have overdue items that would not result in debarment when removing overdue debarments
Comment 109 Tomás Cohen Arazi 2023-10-11 15:10:42 UTC
Created attachment 156841 [details] [review]
Bug 16223: Add new columns to table restriction_types

This patch adds two new columns, lift_after_payment and fee_limit,
to table restriction_types. These colums are used to control
lifting patrons restrictions after paying their fees.

To test:
1. Apply this patch.
2. Update your database via updatedatabase.pl.
=> Confirm your table restriction_types has two new columns.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 110 Tomás Cohen Arazi 2023-10-11 15:10:47 UTC
Created attachment 156842 [details] [review]
Bug 16223: Add ability to define liftable restrictions

Some libraries debar Patrons at the end of the year for having unpaid fines,
like in Bug 15157. Currently librarians have to manually remove this type of
debarments after Patron has paid his/her fines.

This patch adds ability to create restrictions which are lifted after
patron pays ceratain amount of fines.

To test:
1. Apply this patch.
2. Restart your services if needed.
3. Navigate to page restrictions.pl.
=> Note that table has two new colums in it, "Lift after payment?" and "Fee limit".
4. Add new restriction which has "Lift after payment?" set
as Yes and fee limit as 5.
5. Create fees for a patron so they exceed fee limit e.g. 10
6. Add restriction made in step 2. for the patron
7. Pay patrons fees partially so that they go under fee limit
=> Note that patrons restriction should now be lifted.

Also prove t/db_dependent/Patron/Borrower_Debarments.t.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 111 Tomás Cohen Arazi 2023-10-11 15:10:51 UTC
Created attachment 156843 [details] [review]
Bug 16223: (QA follow-up) Remove GetDebarments

We remove the GetDebarments routine in 31095.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 112 Tomás Cohen Arazi 2023-10-11 15:10:55 UTC
Created attachment 156844 [details] [review]
Bug 16223: (QA follow-up) Move trigger to apply

This patch moves the del_restrictions_after_payment from before the
actual credit application call inside Koha::Account::pay to after the
application of credits to debits in Koha::Account::Line::apply.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 113 Tomás Cohen Arazi 2023-10-11 15:10:59 UTC
Created attachment 156845 [details] [review]
Bug 16223: Require fee limit only if "Lift after payment" is set as "Yes"

Fee limit should only be required if restriction is
meant to be lifted after payment. This patch makes
"Fee limit" input field required based on value of
"Lift after payment" select field.

To test:
1. Create a new restriction.
2. "Lift after payment" should be set as "No".
3. Change value as "Yes".
=> "Fee limit" should now have "Required" text
after input field.
4. Try to save restriction.
=> Text "This field is required" should be
displayed after "Fee limit".
5. Change "Lift after payment" back to "No".
=> Both texts should now be lifted.

Also test with existing restriction and make sure
restrictions are still saved without problems.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 114 Tomás Cohen Arazi 2023-10-11 15:11:03 UTC
Created attachment 156846 [details] [review]
Bug 16223: (QA follow-up) Add filter, remove superfluous condition

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 115 Tomás Cohen Arazi 2023-10-11 15:11:08 UTC
Created attachment 156847 [details] [review]
Bug 16223: (QA follow-up) Adjust DB changes

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 116 Tomás Cohen Arazi 2023-10-11 15:11:12 UTC
Created attachment 156848 [details] [review]
Bug 16223: DBIC schema

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 117 Tomás Cohen Arazi 2023-10-11 15:11:16 UTC
Created attachment 156849 [details] [review]
Bug 16223: Mark lift_after_payment as boolean in schema
Comment 118 Tomás Cohen Arazi 2023-10-11 15:11:20 UTC
Created attachment 156850 [details] [review]
Bug 16223: Tidy
Comment 119 Tomás Cohen Arazi 2023-10-11 15:13:56 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 120 Fridolin Somers 2023-10-13 00:16:45 UTC
Enhancement not pushed to 23.05.x