This is the second patch in a series to unify all payment functions into
a single mathod
Test Plan:
1) Apply this patch
2) prove t/db_dependent/Accounts.t
3) Test fine payment via the "Pay" button
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Followed test plan, works as expected.
Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The amount in accountlines should be saved as a negative amount, but
amount outstanding is correct.
Note: This routine is actually used only once (in paycollect.pl). And is
only cosmetically tested in Accounts.t.
Test plan:
[1] Create a manual invoice, Fine, say 20.
[2] Click on the Pay button next to this fine, and pay 1.
[3] Pay again, using 'Pay amount' button, and pay 2.
[4] Pay again, using 'Pay' on the same line, the full remaining amount.
[5] Verify that you see -1, -2 and -17 on the Accounts tab.
[6] Run t/db_dependent/Accounts.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test plan:
1) Apply this patch
2) Make a payment in full using the "Pay" button
3) Note payment succeeds
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch makes t/db_dependent/PatronLists.t run inside a transaction.
It also makes it generate its own data using t::lib::TestBuilder instead
of relying on sample patrons on the DB.
To test:
- Run:
$ prove t/db_dependent/PatronLists.t
=> SUCCESS: Tests pass
- Apply the patch
- Run:
$ prove t/db_dependent/PatronLists.t
=> SUCCESS: Tests pass
- Sign off :-D
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>
Tax rates are stored in decimal(6,4) fields which means that 4 decimals
are allowed.
If a tax rate is 8.42%, it is stored as 0.0842
If a tax rate has more precision than that, Koha won't deal correctly
with it. We will need to update the DB structure.
With this patch, the tax rate will be displayed with the same precision
as in the DB. So if you enter 8.42, you will see 8.42% instead of 8.4%
without this patch.
Test plan:
Do a full acquisition workflow with a tax rate like 8.42% and confirm
that it is correctly displayed.
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This typo has been introduced by
commit eed14b080d
Bug 13001: Refactor VAT and price calculation - parcel page
So before the tax rewrite (13321, 13323).
It sounded weird to me that testers did not catch the bug on this page.
To understand the logic:
Conditions are listincgst, invoiceincgst
Conditions | Before this patch | If we fix the typo | After this patch
0 0 | excluded | excluded | excluded
0 1 | included | included | included
1 0 | excluded | excluded | excluded
1 1 | included | included | included
Test plan:
Create 4 vendors with the difference combinations
Create a basket, add an order (with a tax) and receive it
Confirm that the different values displayed on the parcel page are correct
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The calls
output_pref({ dt => dt_from_string( $date ) })
are wrong and should be replaced with
output_pref({ str => $date })
For better error handling.
Here we fix the problem of items.onloan when searching
Test plan:
- Set items.onloan=0000-00-00 (UPDATE items SET onloan='0000-00-00')
This can come from old data or bad migration
- Execute a search
=> Without this patch you get
Can't locate object method "ymd" via package "dateonly" (perhaps you forgot to load "dateonly"?) at /home/vagrant/kohaclone/Koha/DateUtils.pm line 225.
=> With this patch you won't get the error
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Using a DateTime object with a timezone of America/Santiago
was causing fatal errors for Koha::Calendar->is_holiday
and Koha::Calendar->exception_holidays, when the objects
were truncated to an invalid local time.
Using a floating zone allows us to use the same day, month, year
for comparison purposes without running into the possibility of
creating an invalid local time and thus a fatal software error.
Edit:
While the changes to is_holiday and single_holiday make sense (Jonathan agrees too)
I didn't manage to have them fail, because truncate is not failing in my trials, but
days_between. So to me, it narrows down to have exception_holiday return floating tz
datetime objects so it doesn't break days_between.
Anyway, it is ok to push this patch, and the regression test I provide covers this scenario
I'm describing.
To test:
- Apply the regression tests patch
- Run:
$ prove t/db_dependent/Holidays.t
=> FAIL: Unexpected error due to bad timezone/date combination
- Apply this patch
- Run:
$ prove t/db_dependent/Holidays.t
=> SUCCESS: Tests pass
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch introduces a regression test for exception_holidays. This routine
returns a list of datetimes to be used in date comparison and some datetimes don't exist
in some timezones, so floating timezones should be used instead.
To test:
- Apply the patch on master
- Run:
$ prove t/db_dependent/Holidays.t
=> FAIL: The new test fails
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch removes the requirement for this tests for the DB to include
at least 10 borrowers to pass. Borrowers are now created on each run
using t::lib::TestBuilder and a loop.
Bonus: some tiny changes to tidy the file.
To test:
- Run:
$ prove t/db_dependent/CourseReserves.t
SUCCESS => Tests pass with and without the patch.
- Sign off :-D
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch makes t/db_dependent/CourseReserves.t create
good sample data for its tests. It does so by creating a random
itemtype.
To test:
- Run
$ prove t/db_dependent/CourseReserves.t
=> FAIL: lots of warnings about "item-level_itypes set but no itemtype
set for item"
- Apply the patch
- Run:
$ prove t/db_dependent/CourseReserves.t
=> SUCCESS: Tests are green, and no warnings.
- Sign off :-D
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch makes t/db_dependent/Holds/RevertWaitingStatus.t create
good sample data for its tests. It does so by creating a random
itemtype.
To test:
- Run
$ prove t/db_dependent/Holds/RevertWaitingStatus.t
=> FAIL: lots of warnings about "item-level_itypes set but no itemtype
set for item"
- Apply the patch
- Run:
$ prove t/db_dependent/Holds/RevertWaitingStatus.t
=> SUCCESS: Tests are green, and no warnings.
- Sign off :-D
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch makes t/db_dependent/Members/* create
good sample data for its tests. It does so by creating a random
itemtype.
To test:
- Run
$ prove t/db_dependent/Members/*
=> FAIL: lots of warnings about "item-level_itypes set but no itemtype
set for item"
- Apply the patch
- Run:
$ prove t/db_dependent/Members/*
=> SUCCESS: Tests are green, and no warnings.
- Sign off :-D
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
If the category_type is 'S', GetBorrowersToExpunge won't return the
patron.
Test plan:
t/db_dependent/Members.t
should always 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>
From the pod of Digest::MD5:
"""
Since the MD5 algorithm is only defined for strings of bytes, it can not
be used on strings that contains chars with ordinal number above 255
(Unicode strings). The MD5 functions and methods will croak if you try
to feed them such input data.
What you can do is calculate the MD5 checksum of the UTF-8
representation of such strings.
"""
Test plan:
- Set a MySQL/MariaDB password with unicode characters:
UPDATE user SET password=PASSWORD('❤') WHERE USER='koha_kohadev';
FLUSH PRIVILEGES
- Update your $KOHA_CONF file
- Restart Memcached
- Hit the files modified by this patch
=> Without this patch, you will get a software error (with "Wide
character in subroutine entry" in the logs).
=> With this patch, everything will go fine
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Edit: removed debugging leftover
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This method will be used by several patches later.
Test plan:
prove t/db_dependent/Koha/Patrons.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>
Number of tests in Patrons.t corrected.
Method is_going_to_expired (no english!) renamed to is_going_to_expire.
Adding a negative duration replaced by a subtract. Reads easier.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
In order to be consistent, we need to create this method as well.
Test plan:
Make sure the pref NotifyBorrowerDeparture works as expected
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 17579 introduces a Koha::Patron->is_expired method.
Let's use it from this script.
Test plan:
Confirm that you see the message on the interface from the circulation
page for an expired patron
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
I was reading through Reserves.pm trying to figure out a bug - found some
unused variables instead.
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Problem:
- two missing trailing periods in template
Test plan:
- configure MaxOpenSuggestions with a low limit (e.g. 1)
- visit opac-suggestions.pl?op=add, login if necessary
- note that the period is missing from the second sentence of the leading paragraph under the "Enter a new purchase suggestion" heading
- continue to submit suggestions until you've reached the MaxOpenSuggestions limit
- observe the missing period in the "TooManySuggestionsText" paragraph
- apply patch
- revisit opac-suggestions.pl?op=add
- note that the period is now present in the second sentence of the leading paragraph under the "Enter a new purchase suggestion" heading
- note that the period is also present at the end of the "TooManySuggestionsText" paragraph
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Article requsts from OPAC and from intranet are showing an 'internal
server error'. I am testing in 16.11 with Plack.
Test Plan:
1) Set circ rule to record only article requests
2) Attempt to place a request on a record with no items
3) Note the error
4) Apply this patch
5) You should now be able to place the request!
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
In two places:
1. Administration -> Preferences -> Circulation ->
AllowMultipleIssuesOnABiblio: says effect, should say affect
2. Reports -> Lost Items -> Help (top-right corner): says effect, should
say affect
To test, navigate to these two places and confirm charges have been made
and are correct.
Sponsored-by: Catalyst IT
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
- Adds "Dear..."
- Fixes capitalization
- Removes 'E-mail' from the description as you could also have
a print template
To test:
- Make sure the SQL has no errors
- Install the letters and proof read them
If you agree - sign off!
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
In checkouts table, the is an export form (when some exports syspref are enabled).
Export using a CSV profile will create a file with name koha.mrc (same as ISO2709 export).
It would be better with koha.csv.
Bug 14647 manages the export page, this but will only manage for export from checkouts table where file name is hard-coded.
Test plan :
- Enable checkouts exports by setting syspref ExportWithCsvProfile with a profile for record export
- Go to circ page of a patron with checkouts : /cgi-bin/koha/circ/circulation.pl?borrowernumber=xxx
- Show checkouts table
- Select some checkboxes in "Export" column
- Select "CSV" in export format combo-box
- Click on "Export"
=> Without patch, the generated file is koha.mrc
=> With patch, the generated file is koha.csv
- Check ISO2709 export generates a file named koha.mrc
Signed-off-by: Dani Elder <dani@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
When a patron is not allowed to renew from the self check module, the
only message displayed is "No renewals allowed".
It would be nicer to let him/her know that the renewal is not allowed
because it's a on-site checkout or automatic renewal.
To do so we can call CanBookBeRenewed instead of CanBookBeIssued and get
the renewal error.
Test plan:
0/ Switch off AllowSelfCheckReturns
1/ check out an item and tick "auto renewal"
2/ Go on the self check module
=> auto renewal message is displayed
3/ check out an item and tick "on-site checkout"
4/ Go on the self check module
=> on-site checkout message is displayed
5/ check out an item without ticking any checkboxes (regular checkout)
Renew it to reach the max renew allowed
6/ Go on the self check module
=> regular checkout message is displayed
7/ Switch on AllowSelfCheckReturns and repeat previous steps
=> "Return this item" button is displayed in addition of the renewal
error message
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
"Select all" -> "Select all visible rows"
"Clear all" -> "Clear selection on visible rows"
+ fix the bug where the Delete link was visually enabled only if you
checked a box in the first page (now every checkbox enable the link)
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
With the number displayed, it should remove the ambiguity about what
"Select all" and "Clear all" do.
The number is repeated in the "Delete" confirmation dialog.
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
DataTables removes hidden rows from the DOM. Because of that, "Select
all", "Clear all" and the form submission don't work correctly
(basically they act only on the currently displayed page).
This patch fixes just that.
Test plan:
1/ Go to your search history page
2/ Make you sure you have at least 21 entries in your search history. If
not, do some searches and come back here.
3/ Click "Select all" and change page. The checkboxes on the new page
should be checked.
4/ Check some checkboxes on this new page and click "Clear all". Go back
to the previous page, checkboxes shouldn't be checked.
5/ Check some checkboxes on at least 2 different pages and click
"Delete". All selected search history entries should be deleted.
Followed test plan. Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(This issue has been raised on bug 17216 comments 93-103)
On bug 11944 (3.19.00.006) we updated the default COLLATE for all our tables to
utf8_unicode_ci but not the marc_subfield_structure table.
Indeed we want to keep tagsubfield a utf8_bin (to allow lowercase and uppercase
of the same letter for subfields).
We should have set the default collate to utf8_unicode_ci for further changes.
This patch updates the DB entry 3.19.00.006 to set the default COLLATE to this
table (for people upgrading from prior to 3.19.00.006) and set this
default COLLATE on 16.06.00.033 (for people upgrading from after 3.19.00.006).
The error is:
DBD::mysql::db do failed: Can't create table `koha_kohadev`.`#sql-306_9f9` (errno: 150 "Foreign key constraint is incorrectly formed") [for Statement "
ALTER TABLE marc_subfield_structure
MODIFY COLUMN authorised_value VARCHAR(32) DEFAULT NULL,
ADD CONSTRAINT marc_subfield_structure_ibfk_1 FOREIGN KEY (authorised_value) REFERENCES authorised_value_categories (category_name) ON UPDATE CASCADE ON DELETE SET NULL;
"] at installer/data/mysql/updatedatabase.pl line 13175.
Upgrade to 16.06.00.033 done (Bug 17216 - Add a new table to store authorized value categories)
Test plan:
1/ git checkout v3.18.00
2/ Do an install
3/ git checkout master;
4/ perl installer/data/mysql/updatedatabase.pl
=> Without this patch, you get the error
=> With this patch applied you will not get it and the default COLLATE for
marc_subfield_structure will be correctly set. Make sure tagsubfield is
still utf8_bin
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Will put some notes on the Bugzilla report.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Add '#' selector to several tags
Fix some spacing
Remove duplicated line
To highlight that all is working set PatronQuickAddFields as below for testing:
streetnumber|address|address2|city|state|zipcode|country|branchcode
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch updates the guarantor form to work with the quick add form
To test:
1 - Quick add a 'Kid' patron (or other patron type that may have
guarantor)
2 - Verify form appears as expected
3 - Test that you can add a patron manually or using selector
4 - Test that deletion works as expected
5 - Test 3 and 4 from the full version of the form
6 - Save or cancel and add a new patron using the full form only
7 - Verify that adding/deleting guarantor works with no errors
8 - Sign off
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
- wrong assignation
- reference to 'serials' table rather than 'serial'
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This db rev reapplies dbrev 3.15.00.049 in case that one failed due to
existence of issn_idx.
Test plan:
If you do not have index issn_idx, recreate it:
ALTER TABLE biblioitems ADD INDEX issn_idx (issn (255))
Run this db rev. You should see: Removed issn_idx.
Rerun the dbrev. You should see: Everything is fine.
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
When dbrev 3.15.00.049 failed, the still existing issn_idx must have
been the cause.
This patch only adjusts the old db rev.
Still thinking if we also need a new db rev to fix isbn and issn for
those installs where 3.15.00.049 failed too. (In that case the fields
may not have been changed to mediumtext and no indexes were added.)
Test plan:
Ideally, run an upgrade from something older than 3.15.00.49.
But running the added sql command from the command line might convince
you too.
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Then we need to remove the "available" part from the query.
They are really awkward patches...
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
When a ccl query is used, the buildQuery subroutine does not handle
the available limit (not an index).
This available limit is handle later in the subroutine.
This affect the author links on the detail page for instance (an=xx).
A much better solution would be to keep an 'available' zebra index up-to-date.
Test plan:
(OPAC or staff interface, it does not matter)
- Launch a search, click on a result and then on an author link to
launch another query (an:xx)
- Limit to available items without the 'facet'
=> Without this patch you won't get any results
=> With this patch applied you should get relevant result (regarding the
known bugs 16970, 13715, 13658, 5463, etc.)
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
