|
Description
Lari Taskula
2016-04-07 11:38:15 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 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. 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 (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) Should this be "patch doesn't apply"? Created attachment 99697 [details] [review] Bug 16223: Automatically remove any borrower debarments after a payment Rebased patch. Created attachment 99698 [details] [review] Bug 16223: Enable regex match for rules of debarment Rebased patch. Created attachment 104399 [details] [review] Bug 16223: Automatically remove any borrower debarments after a payment Rebased patch. Created attachment 104400 [details] [review] Bug 16223: Enable regex match for rules of debarment removals Rebased patch. 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> Created attachment 114273 [details] [review] Bug 16223: Enable regex match for rules of debarment removals Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi> Rebased patches and removed dependency to bug 14698. 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> Switching this one to Signed off as well noticing the sandbox patches. 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? 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> 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> 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> Setting back to signed offed. 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> 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> Oops, forgot to set this back to "Needs Signoff"... This is actually already signed off. 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 :)
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. 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' (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. 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 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 (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? It's bug 23051 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 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 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> 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> 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 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 Rebased patches. Couldn't find trailing whitespaces after applying patches, but found a extra =cut on last patch. 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. (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. 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 Created attachment 143389 [details] [review] Bug 16223: DO NOT PUSH! Schema changes Sponsored-by: Koha-Suomi Oy 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 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. (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. 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 Test plan updated. Setting this back to "Needs Signoff" since patches seem to apply nicely apart from ByWaters sandbox. I got the same error on the BibLibre sandboxes for some reason. Spinning one up on PTFS-Europe's site now. 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> 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> 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> 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! 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. 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> 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> 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> 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> 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> 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> Created attachment 143768 [details] [review] Bug 16223: (QA follow-up) Fix kohastructure bug Created attachment 143769 [details] [review] Bug 16223: (QA follow-up) Remove GetDebarments We remove the GetDebarments routine in 31095. 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. 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 ? 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. Thank you Martin and Andrew for looking into this. I'll try to work on this again tomorrow. Hmm, it seems code returns only "MANUAL" restriction. Tests however return correct type. So for some reason Koha saves restrictions incorrectly from UI. 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 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> 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> 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> Created attachment 146054 [details] [review] Bug 16223: (QA follow-up) Fix kohastructure bug Created attachment 146055 [details] [review] Bug 16223: (QA follow-up) Remove GetDebarments We remove the GetDebarments routine in 31095. 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. 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 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? 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> 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> 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> Created attachment 148310 [details] [review] Bug 16223: (QA follow-up) Remove GetDebarments We remove the GetDebarments routine in 31095. 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. 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 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 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> 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> 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> 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> 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> 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> First patch has been resolved in bug 33574. 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> 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> 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> 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> 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 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> 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> Rebased patches, setting status back to "Signed off". Looking here 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> 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> 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> 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> 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> 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> 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> 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.
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 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> 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> 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> 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> 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> 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> Created attachment 156847 [details] [review] Bug 16223: (QA follow-up) Adjust DB changes Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 156848 [details] [review] Bug 16223: DBIC schema Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 156849 [details] [review] Bug 16223: Mark lift_after_payment as boolean in schema Created attachment 156850 [details] [review] Bug 16223: Tidy Pushed to master for 23.11. Nice work everyone, thanks! Enhancement not pushed to 23.05.x This was already documented it seems. |