Test Plan:
1) Apply this patch
2) Note all references to reversing payments have been removed
3) Note ability to void payments remains unchanged
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Test plan:
0) Do not apply the patch
1) Enable ILS-DI
2) Go to
<opac_url>/cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=51&show_fines=1
use patron with some paid and unpaid fines
3) Save the page for later usage
4) Apply the patch
5) Repeat 2 with same patron and compare the saved result with the new
one, they should be same
6) git grep getcharges should return no occurance
7) prove t/db_dependent/Accounts.t
Co-authored-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Fix t/db_dependent/Accounts.t
Default value could be 0, or left as it.
It seems that the different initialisation we have in the code pass a
value for 'amount'
account_offsets.amount
`amount` decimal(26,6) NOT NULL,
DBIx::Class::Storage::DBI::_dbh_execute(): Field 'amount' doesn't
have a default value at /home/vagrant/kohaclone/Koha/Object.pm line 125
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
At this time, only payments may be voided. There is no reason to have this limitation. There are situations where a librarian may need to void an accidental writeoff, or perhaps void an automatic credit that was created by Koha. For illustration, this is directly from a partner library:
"For example a lost book refund becomes a credit on account. Presently the credit may be applied to a fine for a different item charged to patron. We perform a write off to clear the remaining credit, then add the fine back to the account and give the patron a refund for the lost/refunded amount. Our accounting system ask that we keep the Lost funds/refunds separate from all fines."
Test Plan:
1) Create a fine and write it off
2) Note there is no 'void' button for the writeoff
3) Apply this patch
4) Note the buttons now show
5) Test each button on a writeoff
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Test Plan:
1) Apply this patch and its' dependencies
2) Create and pay some various fees and fines
3) View the payments for fees, and fees paid by payments by
using the new 'Details' button available on boraccount.pl
Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Running updatedatabase.pl after patch application makes dependencies
happy. Passes QA Tools and works as intended.
Rebased (2017-12-21): Alex Arnaud <alex.arnaud@biblibre.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Bug 2696: (QA follow-up) - Remove use of GetMember
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test plan:
0) Do not apply the patch
1) Run these 3 commands
git grep ModNote
git grep getcredits
git grep getrefunds
all should return occurances only in C4/Accounts.pm - the sub definition
and in t/db_dependent/Accounts.t
2) Apply the patch
3) Run commands from 1) again, now there should be no occurence
4) prove t/db_dependent/Accounts.t
should return green
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We've had the ability to 'reverse' a payment for a long time, but it
would be much better if we had a true void payment function that
replaces the paid amounts into the fee so that it appears as if the
payment was never made.
Test Plan:
1) Apply this patch and dependent patches
2) Run updatedatabase.pl
3) Create some fines
4) Pay those fines
5) Use the new 'void' button to void the payments
6) Note the fines were restored to their pre-payment amounts
Signed-off-by: Koha-us conference <koha-us@koha-us.net>
Signed-off-by: BWS Sandboxes <ByWaterSandboxes@gmail.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Some libraries would like to be able to specify more specific payment types such as cash, check, credit card, or even canned food ( for food drives ). This feature will allow a library to specify payment types as authorised values of the type PAYMENT_TYPE.
Test Plan:
1) Apply patches
2) Update database
3) Add authorised value PAYMENT_TYPE with values 'Cash', 'Check', 'Credit card'
4) Go to the fines section of a patron who has several fines
5) Tab 'Fines'
6) Click on 'Pay' for an individual fine
7) Verify that in the form a 'Pay an individual fine' dropdown appears with payment types
8) Select a payment type, confirm payment
9) Verify that payment type appears in "Description of charges" in tab 'Account'
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Bug 18786: Remove use of Koha::Patron::Images
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Bug 18786: Remove duplicate block with bad IF
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
purge_zero_balance_fees is used in cleanup_database.pl to determine which fees can be cleaned up.
It uses a simple SQL query to determine which rows in accountlines need to be removed:
463 my $sth = $dbh->prepare(
464 q{
465 DELETE FROM accountlines
466 WHERE date < date_sub(curdate(), INTERVAL ? DAY)
467 AND ( amountoutstanding = 0 or amountoutstanding IS NULL );
468 }
The function comes with the following warning:
451 B<Warning:> Because fines and payments are not linked in accountlines, it is
452 possible for a fine to be deleted without the accompanying payment,
453 or vise versa. This won't affect the account balance, but might be
454 confusing to staff.
This was a reasonable solution prior to the addition of account_offsets in 17.11. The problem now is that rows in accountlines which are linked as credits in accountlines will *always* have amountoutstanding marked as 0. These are linked to debits via account_offsets. purge_zero_balance_fees will delete credits and leave rows in account_offsets which link to deleted credits.
Sites using the --fees option cleanup_database.pl which upgrade to 17.11 may have all of their credits removed without warning.
Test Plan:
1) Apply this patch
2) prove t/db_dependent/Accounts.t
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Yes, we need tests to make sure we understand what is going on in this
subroutine.
The different combination of HoldsInNoissuesCharge,
RentalsInNoissuesCharge and ManInvInNoissuesCharge are tested here with
the subroutine we are going to remove (GetMemberAccountBalance).
In one of the next patches the new methods will be used to make sure we
are not modifying the calculated values (except the formatting).
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Eivin Giske Skaaren <eivin@sysmystic.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The test files do not need to return 1
Patch generated with:
perl -p -i -e "s/^1;\n//xsm" t/**/*.t
Test plan:
git grep '^1;$' t/**/*.t
should not return any results
NOTE: does not fix C4/SIP/t, nor xt tests.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Tests in db_dependent may expect a Koha database, but should not rely on
hardcoded categories, currencies, branch codes, etc.
This patch fixes a bunch of those. But this is a continuous project. We also
need QA to closely watch new edits.
Accounts.t: hardcoded category PT replaced
Acquisition/OrderFromSubscription.t: hardcoded USD
Acquisition/StandingOrders.t: same
ArticleRequests.t: create itemtype, branch and category for testing
AuthorisedValues.t: remove $dbh, add two test branches
AuthoritiesMarc.t: add hardcoded GEOGR_NAME authtype
Bookseller.t: add test currency
Koha.t: add test itemtype instead of hardcoded BK
UsageStats.t: add test branch and category
Test plan:
Run the adjusted tests.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
All tests successful (see comment #9)
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
WriteOffFee is the last of the "payment" subroutines that need to be
merged into Koha::Account::pay ( as a writeoff is really just type of
payment ).
Test Plan:
1) Apply this patch
2) Verify the writeoff, and writeoff all buttons still work
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test Plan:
1) Apply this patch
2) prove t/db_dependent/Accounts.t
3) Test fine payment via the "Pay selected" button
Signed-off-by: Laura Slavin <lslavin@hmcpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test Plan:
1) Apply this patch
2) prove t/db_dependent/Accounts.t
3) Test fine payment via the "Pay" button,
but make the payment for less then the full amount
Signed-off-by: Laura Slavin <lslavin@hmcpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test plan:
prove t/db_dependent/Accounts.t
should return green
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This is the first patch in a series to unify all payment functions into
a single method.
Test Plan:
1) Apply this patch
2) prove t/db_dependent/Accounts.t
3) Test fine payment via the "Pay amount" button
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Lari Taskula <larit@student.uef.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test t/db_dependent/Accounts.t:
This test created a MPL branch, but did not use it any more.
Test t/db_dependent/Items/AutomaticItemModificationByAge.t:
This test still used CPL/MPL branches. Now it uses two new branches.
Test plan:
[1] Run t/db_dependent/Accounts.t.
[2] Run t/db_dependent/Items/AutomaticItemModificationByAge.t.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Both tests pass OK
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This test fails if you do not have a MPL branch, but a closer look shows
that we only need to replace one occurrence.
Test plan:
Run Accounts.t
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Accounts.t: Only added a line that ensures the MPL branch exists.
AnonymiseIssueHistory.t: Only add a branch to work with.
Barcodes.t: Replaced clear with delete_all.
CalcFine.t: Remove default issuing rule and add one instead of updating.
Holds.t: Add category S in case it would not exist.
Members.t: Replaced clear with delete_all.
MoveItemFromBiblio.t: Replace last _fk construction.
Test plan:
Run these tests. (See note).
Git grep for only_fk, {_fk} and TestBuilder::default_value.
Note: Holds.t does not pass. Tests 9 and 39 fail, but they did already.
not ok 9 - GetReservesFromItemnumber should return a valid borrowernumber
not ok 39 - Test AlterPriority(), move to bottom
So this test needs attention, but on another report please :)
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The 'borrower' should not be used anymore, especially for new code.
This patch move files and rename variables newly pushed (i.e. in the Koha
namespace).
Test plan:
1/
git grep Koha::Borrower
should not return code in use.
2/
Prove the different modified test files
3/ Do some clicks in the member^Wpatron module to be sure there is not
an obvious error.
Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as described. Tested with Circulation, Members/Patrons, Discharge,
Restrictions modules and the must common functionalities
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
Added notes to usage in misc/cronjobs/cleanup_database.pl that
--fees DAYS requires DAYS to be greater than or equal to 1.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Also, add notes to perldocs and usage that payments and
credits are not linked to fines and may be deleted
independently of the associated fine.
Signed-off-by: Nancy Keener <nkeener@washoecounty.us>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
The function C4::Accounts::purge_zero_balance_fees() should delete rows
in accountlines where amountoutstanding is 0 and accountlines.date is
less than the current date minus '$days', i.e a number of days passed
to the function. Tests were added to prove the following:
* accountlines.amountoutstanding is 0, and date is set to CURRENT_DATE.
The accountlines row should not be deleted. This is merely a sanity check,
because difference between today's date and the fee date cannot be
greater than $days.
* 'Before', 'On' and 'After' tests for accountlines.amountoutstanding = 0
* accountlines.amountoutstanding is 0, and date is set to $days - 1
days ago. The accountlines row should not be deleted.
* accountlines.amountoutstanding is 0, and date is set to $days
days ago. the accountlines row should not be deleted, because
the difference must be *greater* than $days.
* accountlines.amountoutstanding is 0, and date is set to $days + 1
days ago. The accountlines row *will* be deleted in this case.
* 'Before', 'On' and 'After' tests for accountlines.amountoutstanding > 0.
(3 tests). The accountlines row will never be deleted, because
amountoutstanding is not 0.
* 'Before', 'On' and 'After' tests for accountlines.amountoutstanding < 0.
(3 testes) This tests credits. Again, the accountlines row will never
be deleted, because amountoutstanding is not 0.
http://bugs.koha-community.org/show_bug.cgi?id=14402
Signed-off-by: Nancy Keener <nkeener@washoecounty.us>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Tests assume that the branchcodes CPL/MPL/etc. already exist in the DB.
If they need them, they should create them.
Test plan:
Execute the differente test files on a DB without any branchcode or
at least without CPL/MPL branches.
Confirm that the tests pass.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This trivial patch introduces the code needed on the test files so they handle
the DB transaction instead of relying on the (removed) transaction started/rolled back
by TestBuilder.
Tested all of the files before and after applying the patch, resultes are the same.
(Pass exept of t/db_dependent/Barcodes_ValueBuilder.t, this has the same error).
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This is not introduced by the previous patches of this patchset.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
This patch introduces tests for makepayment and makepartialpayment to check
they are correctly logging to the statistics table.
To test:
- Run
$ prove t/db_dependent/Accounts.t
=> FAIL: tests fail because statistics are not correctly logged.
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
for Kyle M Hall : Adding removal of the 'issues' table
TEST PLAN
---------
1. Apply patch
2. prove -v t/db_dependent/Accounts.t
-- All 15 tests should run successfully without any error or warning
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
