]> git.koha-community.org Git - koha.git/log
koha.git
7 years agoBug 17855: Redirect to the installer if installation process is not done yet
Jonathan Druart [Mon, 17 Apr 2017 15:16:17 +0000 (12:16 -0300)]
Bug 17855: Redirect to the installer if installation process is not done yet

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>
7 years agoBug 17855: Do not create a new dbh, get it from C4::Context->dbh
Jonathan Druart [Mon, 17 Apr 2017 15:13:45 +0000 (12:13 -0300)]
Bug 17855: Do not create a new dbh, get it from C4::Context->dbh

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>
7 years agoBug 17855: perltidy onboarding.pl
Jonathan Druart [Mon, 17 Apr 2017 15:05:21 +0000 (12:05 -0300)]
Bug 17855: perltidy onboarding.pl

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>
7 years agoBug 17855 - Followup patch following requests in comments 176 and 177
Alex Buckley [Sun, 9 Apr 2017 05:30:05 +0000 (05:30 +0000)]
Bug 17855 - Followup patch following requests in comments 176 and 177

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>
7 years agoBug 17855 - Reverted back to original step 3 in the web installer so there is no...
Alex Buckley [Sat, 18 Mar 2017 18:19:17 +0000 (18:19 +0000)]
Bug 17855 - Reverted back to original step 3 in the web installer so there is no basic and advanced setup mode and consequently no need for SQL files to be moved to a 'default' directory for all languages.

Onboarding tool and the rest of the web installer still exists in this
commit

How I tested in my VM (Current master):
- Dropped database
- Recreated database
- Went through installer
- Language en, Marc21
- Installed mandatory data only
- Went through Onboarding tool
- After finishing, logged in as superlibrarian
  that was created by onboarding tool
Everything worked fine.

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>
7 years agoBug 17855: Onboarding tool
Alex Buckley [Thu, 9 Feb 2017 16:32:29 +0000 (17:32 +0100)]
Bug 17855: Onboarding tool

Tested 3 patches together on current master
- Dropped database
- Recreated database
- Went through installer
- Language en, Marc21
- Installed mandatory data only
- Went through Onboarding tool
- After finishing, logged in as superlibrarian
  that was created by onboarding tool
Everything worked fine.

Signed-off-by: Marc Véron <veron@veron.ch>
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>
7 years agoBug 13835: Popup with searches: results hidden by language menu in footer
Marc Véron [Fri, 21 Apr 2017 19:22:44 +0000 (21:22 +0200)]
Bug 13835: Popup with searches: results hidden by language menu in footer

Language menus in pop up windows are not necessary and can hide the contents
(especially search results) on a narrow screen.

For an example, see screenshot in comment #3

This patch allows to mark pop p menus not to display the language footer.

To test:

- Reproduce issue from comment #3
- Apply patch
- Try to reproduce issue from comment #3
  -> language menu should no longer display
- Verify that language menu is suppresed in 'Add to ist' as well
  (from catalog search results, select an item, Add to:...)

Note: There will be more pop-ups with unwnated language selector.
      That can be resolved in follow up bugs.

Followed test plan which worked as intended
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>
7 years agoUpdate Italian installer sample files for 17.05
Zeno Tajoli [Wed, 19 Apr 2017 12:51:19 +0000 (14:51 +0200)]
Update Italian installer sample files for 17.05

Update and translate sample SQL file of the Italian installer.

To test:
- Start with an empty database
- Install it-IT
- Run the web installer in it-IT/Italian
- Verify all files, especially sample notices, sample_frequencies,
  sample_numberpatterns load without any problems.

https://bugs.koha-community.org/show_bug.cgi?id=18456

While applying I got a 3-way-merge. Verified in staff client places
mentioned above, notices, frequencies, sample_numberpatterns, HSBND_FREQ
Works as expected.
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>
7 years agoBug 18479 - Holds 'Placed on' column in opac-user.pl not sorting correctly
Owen Leonard [Mon, 24 Apr 2017 12:36:58 +0000 (12:36 +0000)]
Bug 18479 - Holds 'Placed on' column in opac-user.pl not sorting correctly

Two columns in the user's holds table in the OPAC do not sort correctly
because they are not marked up correctly to enable date sorting: 'Placed
on' and 'Expires on.' This patch corrects it.

This patch also removes a stray </td> which was causing validation
errors.

To test, apply the patch and log into the OPAC with an account which has
multiple holds with differnt hold dates and expiration dates. Confirm
that sorting on these columns works correctly.

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>
7 years agoBug 18453: Remove exports_enabled in circulation_batch_checkouts.tt
Jonathan Druart [Wed, 19 Apr 2017 15:32:06 +0000 (12:32 -0300)]
Bug 18453: Remove exports_enabled in circulation_batch_checkouts.tt

This is a c/p from circulation.tt, this variable is not needed in this
template

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18453: Hide the export column when needed
Jonathan Druart [Wed, 19 Apr 2017 15:29:26 +0000 (12:29 -0300)]
Bug 18453: Hide the export column when needed

If exports_enabled is equal to "0" it will be evaluated to true.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
7 years agoBug 17916 - "Delete MARC modification template" fails to actually delete it
Owen Leonard [Tue, 25 Apr 2017 13:15:13 +0000 (13:15 +0000)]
Bug 17916 - "Delete MARC modification template" fails to actually delete it

The MARC modification templates' 'Delete' button fails because the event
handler has a "preventDefault" but the outcome of the confirmation
function must return true or false.

This patch removes the "preventDefault" from the .delete_template click
handler as well as a redundant onclick attribute from the delete link
itself.

To test, apply the patch and go to Tools -> MARC modification templates.
Clicking the delete button for any existing template should work
correctly.

Followed test plan, worked as intended

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>
7 years agoBug 18419 - Broken patron-blank image in viewlog.tt
Owen Leonard [Tue, 25 Apr 2017 16:16:49 +0000 (16:16 +0000)]
Bug 18419 - Broken patron-blank image in viewlog.tt

Because of a variable name collision the blank patron image doesn't
display in viewlog.tt. This patch moves the image from the template to
the stylesheet to avoid this problem.

This patch also replaces the blank patron image PNG file with an SVG
file. SVG support is wide enough to begin using whereever possible.

This patch also removes some inline CSS from circ-menu.inc and puts it
in the global stylesheet.

To test, apply the patch and clear your browser cache if necessary.

- Enable the patronimages system preference.
- View a patron account which lacks a patron image. All views (details,
  fines, notices, etc) should show the "blank" patron image, including
  the modification log view.
- View a patron account which has a patron image and check that it still
  displays correctly in all views.

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>
7 years agoBug 15738: Show rental fees on OPAC summary page
Aleisha Amohia [Thu, 5 Jan 2017 02:01:57 +0000 (02:01 +0000)]
Bug 15738: Show rental fees on OPAC summary page

This patch adds a few lines that check for a rental fee on an item. If
yes, it will show in brackets as a rental fee on the OPAC summary page.

To test:
1) Have a borrower with an overdue item accruing fines, a lost item and
an item with a rental fee. Confirm the Fines column on the OPAC summary
page now shows you what you may expect to see for each item.

Sponsored-by: Catalyst IT
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>
7 years agoBug 18482 False duplicates detected on adding a batch from a stage file
Olivier Crouzet [Sat, 22 Apr 2017 16:20:48 +0000 (18:20 +0200)]
Bug 18482 False duplicates detected on adding a batch from a stage file

When adding a batch from a stage file with defaut matching on
title/author, if a duplicate is detected, all following records
treated in the batch are discarded from import even if they are not duplicates

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>
7 years agoBug 18179: Update existing calls
Jonathan Druart [Tue, 18 Apr 2017 16:50:36 +0000 (13:50 -0300)]
Bug 18179: Update existing calls

This patch updates the existing occurrences of ->find called in a list
context.
There are certainly others that are not easy to catch with git grep.
Test plan:
Confirm that the 4 modified scripts still works as expected.

We need this one ASAP in master to make sure we will not get other
side-effects of this kind and to catch possible uncaught occurrences
before the release.

Tested scripts changed by this patch, they work as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18179: Forbid list context calls for Koha::Objects->find
Jonathan Druart [Tue, 18 Apr 2017 16:49:18 +0000 (13:49 -0300)]
Bug 18179: Forbid list context calls for Koha::Objects->find

Reading https://perlmaven.com/how-to-return-undef-from-a-function
this sound like the more correct behaviour.

Considering:
$template->param(
    stuff => Koha::Stuffs->find( $id ),
    foo   => 1,
);
without this patch, if the $id does not represent any rows in the DB,
stuff will be assigned to 'foo' and $foo will be undef in the template.
That can lead to very bad side-effects.

With this patch we make sure that it will never happen again.

Test plan:
  prove t/db_dependent/Koha/Objects.t
should return green

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18467: Handle orders with deleted biblio when viewing a basket
Jonathan Druart [Fri, 21 Apr 2017 16:05:47 +0000 (13:05 -0300)]
Bug 18467: Handle orders with deleted biblio when viewing a basket

If the bibliographic record of an order has been removed, $order->{bibionumber}
is undefined. We need to handle this specific case correctly.

To test:
1 - Create a basket
2 - Order a bib
3 - Cancel order and delete record
4 - You cannot view basket
5 - Apply patch
6 - View basket
7 - There should not be an error
r calling count on undefined bib in basket.pl if order cancelled and
record deleted

Followed test plan, works as intended

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>
7 years agoBug 18471 - Receiving order with unitprice greater than 1000 processing incorrectly
Kyle M Hall [Fri, 21 Apr 2017 14:10:37 +0000 (10:10 -0400)]
Bug 18471 - Receiving order with unitprice greater than 1000 processing incorrectly

Receiving orders process the comma as a decimal point
Invoices are displaying incorrectly when formatting total

Test Plan:
1. Open a basket
2. Place an order for an item with price > 1000, $4367.00 for example
3. Close basket
4. Receive order
5. Note on orderreceive.pl the price is populate as "4,367.00"
6. Receive/Save
7. Note the 'Actual Cost' is now $4.00, verify db contains 4 as well
8. Cancel receipt
9. Receive again, this time enter price as "4367"
10. Receive/save
11. Note actual cost is correct
12. Finish receiving
13. Note invoice reads total as $4.00
14. Check db. price in aqorders is correct but displaying incorrectly
15. Apply this patch
16. Repeat step2 1. 14, note errors are fixed

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18461: (bug 13757 follow-up) Do not mix decode_json and to_json
Jonathan Druart [Mon, 24 Apr 2017 16:53:00 +0000 (13:53 -0300)]
Bug 18461: (bug 13757 follow-up) Do not mix decode_json and to_json

Internal server error while approving OPAC-edited patron attributes
containing umlauts (äöü)

If a OPAC user enters text containing umlauts (äöü) in a patron atribute
and saves it, the Staff client will get an inernal server error in
members-update.pl (approving).

E.g. a text like 111ä222 leads to the following in plack error log:

malformed UTF-8 character in JSON string, at character offset 14

To test:
- Reproduce problem above
- Apply patch
- Verify that Home > Patron > Update records displays without error
and 'New value' displays properly
- Approve!

Also, run the tests:
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/db_dependent/Koha/Patron/Modifications.t
=> FAIL: Tests explode due to encoding problems in JSON handling
- Apply the patch
- Run the tests again:
 k$ prove t/db_dependent/Koha/Patron/Modifications.t
=> SUCCESS: Tests pass!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18461: Make tests break on wrong UTF8 data handilng
Tomas Cohen Arazi [Mon, 24 Apr 2017 18:45:17 +0000 (15:45 -0300)]
Bug 18461: Make tests break on wrong UTF8 data handilng

This patch makes the tests fail if extended attributes handling fails due to
wrong UTF-8 data handling.

To test:
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/db_dependent/Koha/Patron/Modifications.t
=> FAIL: Tests explode

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18066: Remove warnings from translate script
Jonathan Druart [Mon, 24 Apr 2017 19:26:31 +0000 (16:26 -0300)]
Bug 18066: Remove warnings from translate script

This patch removes the following warnings:
Use of uninitialized value $pref_name in concatenation (.) or string at
LangInstaller.pm

7 years agoBug 18452: Correcting 'url' to say 'URL' in catalog detail
Aleisha Amohia [Wed, 19 Apr 2017 01:03:15 +0000 (01:03 +0000)]
Bug 18452: Correcting 'url' to say 'URL' in catalog detail

To test:
1) Edit a record, put a URL in 856u and hit save
2) Confirm that url shows as URL in OPAC and staff client

Sponsored-by: Catalyst IT
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 15815: Reword confirm message when removing patrons from card batch
Aleisha Amohia [Tue, 18 Apr 2017 23:18:31 +0000 (23:18 +0000)]
Bug 15815: Reword confirm message when removing patrons from card batch

This patch rewords the confirm message when removing patrons from a card
batch.
From: "Are you sure you want to remove card number(s):1 from this
batch?"
To: "Are you sure you want to remove the selected patron(s) from this
batch?"

To test:
1) Go to Tools -> Patron Card Creator -> Manage batches
2) Edit a batch
3) Select one or more patrons and click 'Remove selected patrons' (not
individual Delete buttons)
4) Confirm the message is worded better and easier to understand
5) Click OK and confirm the patrons are deleted as expected

Sponsored-by: Catalyst IT
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18466: article requests box outline in opac
phette23 [Fri, 21 Apr 2017 00:15:57 +0000 (17:15 -0700)]
Bug 18466: article requests box outline in opac

To test:
1) apply patch
2) ensure ArticleRequests is set to Enable
3) sign in as a user with no article requests on the OPAC side
4) click the 'article requests' tab & see the box & message

Sponsored-By: California College of the Arts
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>
7 years agoBug 15702: Recommended Counter-patch
Mark Tompsett [Fri, 20 Jan 2017 02:13:47 +0000 (21:13 -0500)]
Bug 15702: Recommended Counter-patch

As per comment #7, this patch affects AddMember and ModMember.
The test plan should be the same as comment #6.
Secondary patch with tests still to come.

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>
7 years agoBug 15702: Add test cases for modified code
Mark Tompsett [Fri, 21 Apr 2017 12:58:03 +0000 (08:58 -0400)]
Bug 15702: Add test cases for modified code

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>
7 years agoBug 18457: Add tests
Jonathan Druart [Mon, 24 Apr 2017 17:16:34 +0000 (14:16 -0300)]
Bug 18457: Add tests

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18457 - process_message_queue.pl will die if a patron has no sms_provider_id...
Kyle M Hall [Wed, 19 Apr 2017 13:52:38 +0000 (09:52 -0400)]
Bug 18457 - process_message_queue.pl will die if a patron has no sms_provider_id set but sms via email is enabled for that patron

If SMS via Email is enabled, and a patron has opted for SMS messages, but has not selected a service provider, the cronjob will die with the error
Can't call method "domain" on an undefined value at /usr/share/koha/lib/C4/Letters.pm line 1055.
This will cause all messages that come after the error to not be sent!

Test Plan:
1) Enable SMS via Email
2) Enable SMS for a patron, but don't set a provider
3) Perform an action that will trigger an sms message to go into
   the holds queue ( item due, item checkout, etc )
4) Run process_message_queue.pl, note the error
5) Apply the patch
4) Run process_message_queue.pl, no error this time!

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
7 years agoBug 18435 - Language entered in Materials Specified does not display when items are...
Nick Clemens [Fri, 14 Apr 2017 17:07:39 +0000 (13:07 -0400)]
Bug 18435 - Language entered in Materials Specified does not display when items are checked out and checked in

This problem stems from bug 17642, if items.materials is not linked to
an authorized value the contents are being tossed out. We shuold display
them instead

To test:
1 - Ensure items.materials 952$3 is not mapped to an authorized value
2 - Add a value to an item
3 - Check the item out, note the materials does not display
4 - Check the item in, note the materials does not display
5 - Apply patch
6 - Check the item out, note the materials does display
7 - Check the item in, note the materials does display

Followed test plan, message about material displays as expected
whil checking out and in.
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>
7 years agoBug 18468 - When adding from a staged file order discounts are not passed into C4...
Nick Clemens [Fri, 21 Apr 2017 01:36:44 +0000 (21:36 -0400)]
Bug 18468 - When adding from a staged file order discounts are not passed into C4::Acquisitions::populate_order_with_prices

To test:
1 - Setup a vendor with a discount
2 - Stage a file
3 - Create a basket
4 - Order from staged file
5 - Add a price but no discount
6 - Save order
7 - Note ecost is not discounted
8 - Apply patch
9 - Repeate 2-6
10 - Note ecost is discounted

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>
7 years agoBug 18409: Make the controller for holds use Koha::Holds
Tomas Cohen Arazi [Fri, 21 Apr 2017 23:34:00 +0000 (20:34 -0300)]
Bug 18409: Make the controller for holds use Koha::Holds

Recently, there's been a major fix on the REST api swagger spec,
which involved fixing boolean values so they are actually booleans
and Koha::Object was extended to handle that.
While the swagger spec for this endpoint got fixed, such is not the case
with the implementation (the controller class).

This patch fixes this situation by:
- Specifying boolean properties as boolean in the schema file
- Fixes the controller so it returns Koha::Hold objects instead of the
  hashref returned by GetReserve, which is wrong.
- Better (than empty) descriptions are added to 'suspend',
  'suspend_until' and 'lowestPriority' on the spec.

To test:
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/db_dependent/api/v1/holds.t
=> FAIL: Tests fail, mostly due to error 500 results.
- Apply this patch
- Run:
 k$ prove t/db_dependent/api/v1/holds.t
=> SUCCESS: Tests pass!
- Sign off :-D

This can also be tested using any interface for REST apis.

Note: This endpoint lacks several of the new guidelines and is not
complete (there's no GET for single holds, etc). It is also missing
exception handling. There are probably
other bug reports for that, just thought it was worth mentioning.

Followed test plan and this patch worked as intended
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18300: [QA Follow-up] Fix return value inconsistency
Marcel de Rooy [Mon, 3 Apr 2017 19:43:48 +0000 (21:43 +0200)]
Bug 18300: [QA Follow-up] Fix return value inconsistency

As noted on bug report 17669, the return values of delete (both singular
and plural), delete_missing and delete_temporary should be consistent.

Removed the if-construction around full_path. We do not need it; in the
very exceptional case that full_path would be empty, we should delete
the record since the file is missing.

Adjusted POD and unit tests accordingly.

Test plan:
Run t/db_dependent/Upload.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18300: Delete missing upload records
Marcel de Rooy [Sun, 19 Mar 2017 16:25:01 +0000 (17:25 +0100)]
Bug 18300: Delete missing upload records

If you deleted files from the upload directories manually, or you rebooted
with files in the temporary upload folder, or for some other reason have
records without a file, you may want to cleanup your records in the
uploaded_files table.

This patch adds the method delete_missing to Koha::UploadedFiles. It also
supports a keep_record parameter to do a dry run (count the missing files
first).

Also, we add an option --uploads-missing to cleanup_database. If you add
the flag 1 after this option, you will delete missing files. If you add the
flag 0 or only use the option, you will count missing files.

A subtest is added to Upload.t for delete_missing tests.

Test plan:
[1] Run t/db_dependent/Upload.t
[2] Upload a file and delete the file manually.
[3] Run cleanup_database.pl --uploads-missing
    It should report at least one missing file now.
    Check that the record has not been deleted.
[4] Run cleanup_database.pl --uploads-missing 1
    It should report that it removed at least one file.
    Check that the record is gone.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18182: Fix Koha::RefundLostItemFeeRules
Jonathan Druart [Wed, 12 Apr 2017 18:20:10 +0000 (15:20 -0300)]
Bug 18182: Fix Koha::RefundLostItemFeeRules

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18182: Tests all Koha::Objects-based modules
Jonathan Druart [Wed, 12 Apr 2017 18:19:33 +0000 (15:19 -0300)]
Bug 18182: Tests all Koha::Objects-based modules

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Added test description on line 387.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18182: [QA Follow-up] Fix a few typos, consistent rollback
Marcel de Rooy [Fri, 31 Mar 2017 11:32:55 +0000 (13:32 +0200)]
Bug 18182: [QA Follow-up] Fix a few typos, consistent rollback

Since we here only use one rollback and this test does not care, it is
more consistent to keep that pattern.
Additionally, promoting two globals to our.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18182: Make TestBuilder capable of returning Koha::Object
Tomas Cohen Arazi [Tue, 28 Feb 2017 12:08:38 +0000 (09:08 -0300)]
Bug 18182: Make TestBuilder capable of returning Koha::Object

This patch adds a new method to t::lib::TestBuilder so it can return
Koha::Object-derived objects. The new method is called ->build_object
and requires the plural of the target class to be passed.

'class' is a mandatory param, and a warning is raised and undef is
returned if absent.

It accepts 'value' as the original ->build() method, and that is passed as-is
to ->build().

To test:
- Apply the patches
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/db_dependent/TestBuilder.t
=> SUCCESS: Tests pass!
- Sign off :-D

Sponsored-by: ByWater Solutions
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18459: Add the Koha::Item->biblioitem method
Julian Maurice [Wed, 19 Apr 2017 15:01:54 +0000 (17:01 +0200)]
Bug 18459: Add the Koha::Item->biblioitem method

Test plan:
  prove t/db_dependent/Koha/Items.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>
7 years agoBug 18448: Fix a few db_dependent tests
Marcel de Rooy [Sun, 16 Apr 2017 16:50:58 +0000 (18:50 +0200)]
Bug 18448: Fix a few db_dependent tests

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>
7 years agoBug 18427 - DBRev 16.12.00.026
Kyle M Hall [Fri, 21 Apr 2017 18:03:42 +0000 (14:03 -0400)]
Bug 18427 - DBRev 16.12.00.026

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18427: Add a primary key to serialitems
Marcel de Rooy [Thu, 13 Apr 2017 11:34:36 +0000 (13:34 +0200)]
Bug 18427: Add a primary key to serialitems

We drop the unique index serialitemsidx and add a primary key consisting
of itemnumber just as the unique key did.

Test plan:
Run the db revision.
Apply bug 18182. The TestBuilder.t should no longer fail on serialitems.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18427: DBIx schema changes
Marcel de Rooy [Thu, 13 Apr 2017 11:53:37 +0000 (13:53 +0200)]
Bug 18427: DBIx schema changes

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17669 - DBRev 16.12.00.025
Kyle M Hall [Fri, 21 Apr 2017 17:59:52 +0000 (13:59 -0400)]
Bug 17669 - DBRev 16.12.00.025

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17669: Remove warning 'variable $kohaobj masks earlier declaration in same scope'
Jonathan Druart [Thu, 20 Apr 2017 04:10:14 +0000 (01:10 -0300)]
Bug 17669: Remove warning 'variable $kohaobj masks earlier declaration in same scope'

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17669: [QA Follow-up] More consistency in return values of delete
Marcel de Rooy [Mon, 3 Apr 2017 17:14:52 +0000 (19:14 +0200)]
Bug 17669: [QA Follow-up] More consistency in return values of delete

See Bugzilla comment36 (QA request).

Koha::UploadedFile->delete
Returns 1, 0E0 or -1 (unknown).

Koha::UploadedFiles->delete and delete_temporary
Returns number of deleted records, 0E0 or -1.

POD lines are corrected accordingly. Unit tests adjusted too.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Added a note that Koha::Object->delete itself is not completely consistent
with Koha::Objects->delete (DBIx). The singular may return 1 when it
gets 0E0 from DBIx, while the plural one may return 0E0. But should be
handled somewhere else.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17669: [QA Follow-up] Allow zero in temp-uploads-days
Marcel de Rooy [Fri, 31 Mar 2017 06:31:10 +0000 (08:31 +0200)]
Bug 17669: [QA Follow-up] Allow zero in temp-uploads-days

As requested by QA on comment33.
If the pref is 0 or the overriding command line parameter is 0, all
temporary files will be deleted. But if the pref is NULL or empty string,
we will not delete files.

Also adjusted the description of the preference in this regard.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17669: [QA Follow-up] Rename --temp-uploads-override parameter
Marcel de Rooy [Thu, 30 Mar 2017 06:51:17 +0000 (08:51 +0200)]
Bug 17669: [QA Follow-up] Rename --temp-uploads-override parameter

As requested by QA on comment25.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17669: [QA Follow-up] Rename preference by removing underscores
Marcel de Rooy [Thu, 30 Mar 2017 06:42:38 +0000 (08:42 +0200)]
Bug 17669: [QA Follow-up] Rename preference by removing underscores

Requested by QA on comment25.
Result of:
git grep -l Upload_PurgeTemporaryFiles_Days | xargs sed -i -e "s/Upload_PurgeTemporaryFiles_Days/UploadPurgeTemporaryFilesDays/g"

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17669: Add new switch to crontab example and debian daily cron file
Marcel de Rooy [Wed, 14 Dec 2016 12:46:06 +0000 (13:46 +0100)]
Bug 17669: Add new switch to crontab example and debian daily cron file

The new switch for deleting temporary uploads in cleanup_database can
be added to cron.

Note: Since the option --temp-uploads does only purge temporary uploads
when triggered by the preference Upload_PurgeTemporaryFiles_Days, it can
be safely added here.

Test plan:
There is actually nothing to test here if you followed the preceding test
plans. Just verify that the switch is inserted ocrrectly.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17669: Incorporate the changes in cleanup_database
Marcel de Rooy [Tue, 13 Dec 2016 15:44:26 +0000 (16:44 +0100)]
Bug 17669: Incorporate the changes in cleanup_database

Two command line options are added:
[1] -temp-uploads to indicate that you want to purge these uploads,
[2] -temp-uploads-override DAYS to (optionally) tell that you want to
    override the corresponding pref value.

Test plan:
[1] Check the modified usage statement.
[2] If needed, backup your temporary uploads :)
    In case you do not have one, add a temporary one with Tools/Upload.
    Note: Do not choose an upload category.
[3] Set pref to 0, and run cleanup_database with only --temp-uploads.
    No files should be deleted.
[4] Check number of "old" temp uploads. Set pref to nonzero value.
    Verify that the oldest are gone (depending on the value chosen).
[5] Set pref to 0 again.
    If all uploads are gone now, add a new one with Tools/Upload.
    Run cleanup_database with --temp-uploads --temp-uploads-override -1
    All temporary files are gone.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17669: Add delete_temporary method with unit tests
Marcel de Rooy [Tue, 13 Dec 2016 13:04:29 +0000 (14:04 +0100)]
Bug 17669: Add delete_temporary method with unit tests

Test plan:
Run t/db_dependent/Upload.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17669: Small change to sub delete
Marcel de Rooy [Mon, 20 Mar 2017 07:43:32 +0000 (08:43 +0100)]
Bug 17669: Small change to sub delete

Instead of looping through _resultset here and wrapping the results,
we should use a Koha::Objects instance. Since the method may be called
as a class method, we create an instance if needed.

Test plan:
Run t/db_dependent/Upload.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17669: Introduce new preference for purging temp uploads
Marcel de Rooy [Tue, 13 Dec 2016 13:03:27 +0000 (14:03 +0100)]
Bug 17669: Introduce new preference for purging temp uploads

The new preference is: Upload_PurgeTemporaryFiles_Days.
It controls if the cron job will delete temporary uploaded files and if so,
how many days should have passed before doing so.

Since the default value is empty, it will not start deleting these uploads
until you change the pref. No behavior change. But the dbrev will print a
message if you already have temporary uploads.

Test plan:
[1] Check Administration, Preferences, Tools for the new pref.
[2] Run the db rev.
    If you have temporary uploads, check the reported count.
[3] Bonus: Run sysprefs.sql in a safe environment.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17964: Replace next with last
Jonathan Druart [Fri, 24 Mar 2017 01:13:17 +0000 (22:13 -0300)]
Bug 17964: Replace next with last

We certainly always want to retrieve the last row of a given search.
If it not sufficient we will need to rethink this code.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17964: [QA Follow-up] Test descriptions, typo
Marcel de Rooy [Fri, 24 Mar 2017 08:05:44 +0000 (09:05 +0100)]
Bug 17964: [QA Follow-up] Test descriptions, typo

Typo: have been checkin => checked.
And five test descriptions ;)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17964: Support date formatting
Jonathan Druart [Sat, 21 Jan 2017 15:02:50 +0000 (16:02 +0100)]
Bug 17964: Support date formatting

Using the KohaDates plugin will allow us to format dates as we want,
using the same filters as the ones defined in the KohaDates TT plugin.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17964: Add old_issues
Jonathan Druart [Sat, 21 Jan 2017 14:48:57 +0000 (15:48 +0100)]
Bug 17964: Add old_issues

If it's a CHECKIN, C4::Circulation::SendCirculationAlert set a
"old_issues" key instead of "issues".

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17964: TT syntax for notices - Prove that CHECKIN and CHECKOUT are compatible
Jonathan Druart [Mon, 16 Jan 2017 15:35:18 +0000 (16:35 +0100)]
Bug 17964: TT syntax for notices - Prove that CHECKIN and CHECKOUT are compatible

From the CHECKIN and CHECKOUT templates you should be able to access the
following information: item, biblio, biblioitem, patron and library

Test plan:
Define CHECKIN and CHECKOUT notice templates.
You should be able to define them using the TT syntax to produce the
same generated notice messages as with the historical syntax.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16895 [QA Followup] - Use q{} for empty string as per PBP
Kyle M Hall [Fri, 21 Apr 2017 15:01:30 +0000 (11:01 -0400)]
Bug 16895 [QA Followup] - Use q{} for empty string as per PBP

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16895: [QA Follow-up] Resolve warning on $payment_type_writeoff
Marcel de Rooy [Wed, 19 Apr 2017 14:25:17 +0000 (16:25 +0200)]
Bug 16895: [QA Follow-up] Resolve warning on $payment_type_writeoff

Resolves:
  Use of uninitialized value $payment_type_writeoff in string eq at C4/SIP/Sip/MsgType.pm line 1072, <STDIN> line 2.
Would be raised if this parameter is not found in SIPconfig.xml.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16895 [QA Followup] - Don't require fee payment to be exact full payment
Kyle M Hall [Wed, 19 Apr 2017 13:29:13 +0000 (09:29 -0400)]
Bug 16895 [QA Followup] - Don't require fee payment to be exact full payment

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16895 - Allow writeoffs via SIP2
Kyle M Hall [Mon, 11 Jul 2016 13:55:50 +0000 (13:55 +0000)]
Bug 16895 - Allow writeoffs via SIP2

Many SIP2 services such as those by Comprise Technologies are able to or
require that an ILS be able to accept writeoffs via SIP2. The SIP2
protocol specifies that payment type be a two digit number, but does not
specify a code for writeoffs. To this end we should allow the write-off
code to be specified in the SIP2 config on a per-account basis so that
if different vendors use different fixed codes for write-offs we can
handle that gracefully.

Test Plan:
1) Apply this patch
2) Modify your SIP2 config to include
      payment_type_writeoff="06"
  in the login portion of the account you will be using for the test.
3) Restart your SIP2 server
4) Create a fee for a patron
5) Send a SIP2 fee paid message specifying the payment type code we
   defined earlier, with a payment amount that is *not* equal to the
   amount outstanding for the fee.
6) Note the fee paid response indicates the payment failed
7) Repeat step 5, but this time send the amount outstanding as the
   payment amount
8) Note that the fee paid response indicates a successful payment
9) Note in Koha that the fee has been written off!

Signed-off-by: Rhonda Kuiper <kuiper@roundrocktexas.gov>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
7 years agoBug 10357: Do not change the alerttype
Jonathan Druart [Tue, 18 Apr 2017 18:13:18 +0000 (15:13 -0300)]
Bug 10357: Do not change the alerttype

Previous patch modified the alerttype from "issue" to "issue_det" or
"issue_ser". This is wrong, we do not want to modify this value,
especially because it's used in C4::Letters::SendAlerts
This patch uses a $referer variable instead, like it is already used in
other scripts for redirection.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 10357: Adding link for email notification for new subscription issues to opac...
Aleisha Amohia [Mon, 6 Feb 2017 21:26:54 +0000 (21:26 +0000)]
Bug 10357: Adding link for email notification for new subscription issues to opac-detail

This patch adds the link to the opac-detail.pl page so it is less hard
to find.
Update: Fixing link to look like button, adding span ID around element
Update: Changing ID to Class
Update: Comment 14 fixes

To test:
1) Add a subscription, attach it to a record and select a notice under
'Patron notification'. Hit Next, fill in some fields and save the
subscription
2) Go to that record detail page in the OPAC. Go to the subscriptions
tab, click 'More details'. Notice how many clicks it takes to see the
'Subscribe' button
3) Apply patch and go back to the details page for the record
4) Notice now there is a 'subscribe' button
5) Confirm this button works as expected and you are redirected to
the detail page
6) Confirm 'Cancel' works and redirect works too
7) Go down to 'More details' and confirm the buttons work and you are
redirected back to the serial-issues plage from here

Sponsored-by: Catalyst IT
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Re-tested, works as expected.
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>
7 years agoBug 18443: Get rid of warning 'uninitialized value $user' in C4/Auth.pm
Marc Véron [Tue, 18 Apr 2017 07:21:45 +0000 (09:21 +0200)]
Bug 18443: Get rid of warning 'uninitialized value $user' in C4/Auth.pm

When logging out from OPAC, plack-error.log log and/or opac-error.log
complain about 'uninitialized value $user' in C4/Auth.pm line 187. The
warning is not necessary, this patch removes it.

To test:
- try to trigger warning
- apply patch
- verify that warning no longer occurs
- prove t/db_dependent/Auth.t
- verifiy that SCO still behaves like before (especially if
  you break out from sco path)

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18432: Replace 2 'he or she' with 'they'
Jonathan Druart [Thu, 20 Apr 2017 16:00:00 +0000 (13:00 -0300)]
Bug 18432: Replace 2 'he or she' with 'they'

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18432 : Follow up - Updating to use they/them
Chris Cormack [Tue, 18 Apr 2017 21:32:59 +0000 (09:32 +1200)]
Bug 18432 : Follow up - Updating to use they/them

Updating to use they/them and skipping the ones changed to it

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18432: code comments assume male gender
phette23 [Thu, 13 Apr 2017 21:26:07 +0000 (14:26 -0700)]
Bug 18432: code comments assume male gender

Comments throughout the Koha codebase assume that
all librarians or borrowers are male by using the
pronoun 'he' universally. This patch changes to
'he or she' / 'him or hers'.

Testing plan:

- ensuring modifying tests still pass:
+ C4/SIP/t/06patron_enable.t
+ t/db_dependent/Circulation.t
+ t/db_dependent/Koha/Patrons.t
+ t/db_dependent/Reserves.t

Sponsored-By: California College of the Arts
No code changes detected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18429 - Receiving an item should update the datelastseen
Nick Clemens [Thu, 13 Apr 2017 13:55:15 +0000 (09:55 -0400)]
Bug 18429 - Receiving an item should update the datelastseen

To test:
0 - Ensure AcqCreateItem is set to 'placing an order'
1 - Order some items, note entrydate and datelastseen match
2 - Alter those dates to be earlier than today (or wait some days)
3 - Recieve the item and note datelast seen not updated
4 - Apply patch
5 - Repeat 1-3
6 - Date last seen should be updated.

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Works as expected.
Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18420: Fix HoldFulfillmentPolicy.t and Passwordrecovery.t
Marcel de Rooy [Fri, 14 Apr 2017 10:46:13 +0000 (12:46 +0200)]
Bug 18420: Fix HoldFulfillmentPolicy.t and Passwordrecovery.t

We are going out of scope here, but these tests need a branch/item.

Test plan:
Run the adjusted tests.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18420: Replace hardcoded EUR in OrderUsers.t
Marcel de Rooy [Thu, 13 Apr 2017 07:02:55 +0000 (09:02 +0200)]
Bug 18420: Replace hardcoded EUR in OrderUsers.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18420: Do not use 'S' as patron category code in other tests
Jonathan Druart [Tue, 11 Apr 2017 10:48:54 +0000 (07:48 -0300)]
Bug 18420: Do not use 'S' as patron category code in other tests

Test plan:
prove all these tests, they must all pass

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18420: Use TestBuilder to create a patron category in Suggestions.t
Jonathan Druart [Tue, 11 Apr 2017 10:33:32 +0000 (07:33 -0300)]
Bug 18420: Use TestBuilder to create a patron category in Suggestions.t

No need to do it that way, let's use TestBuilder.

Test plan:
  prove t/db_dependent/Suggestions.t
should still return green

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18420: Fix Members.t when no patron category 'S' exists
Jonathan Druart [Wed, 12 Apr 2017 14:10:57 +0000 (11:10 -0300)]
Bug 18420: Fix Members.t when no patron category 'S' exists

No need to create Staff users here.

Test plan:
  prove t/db_dependent/Members.t
should return green, even if no categories.categorycode 'S' exists

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18420: Fix Passwordrecovery.t when no patron category 'S' exists
Jonathan Druart [Wed, 12 Apr 2017 14:06:43 +0000 (11:06 -0300)]
Bug 18420: Fix Passwordrecovery.t when no patron category 'S' exists

No need to create Staff users here.

Test plan:
  prove t/db_dependent/Passwordrecovery.t
should return green, even if no categories.categorycode 'S' exists

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18420: Fix Budgets.t when no patron category 'S' exists
Jonathan Druart [Wed, 12 Apr 2017 14:03:10 +0000 (11:03 -0300)]
Bug 18420: Fix Budgets.t when no patron category 'S' exists

No need to create Staff users here.

Test plan:
  prove t/db_dependent/Budgets.t
should return green, even if no categories.categorycode 'S' exists

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18335 - Check in: Make patron info in hold messages obey syspref AddressFormat
Marc Véron [Sun, 26 Mar 2017 15:26:22 +0000 (17:26 +0200)]
Bug 18335 - Check in: Make patron info in hold messages obey syspref AddressFormat

When checking in, information about patrons with a hold always display
in an US style. Address information should opey the system preference
AddressFormat

To reproduce:
- Set syspref AddressFormat to German style
- Check out an item to a patron A
- Put a hold on this item for patron B
- Check in item using the Check in field in page header
- Result: In message 'Hold found', address does not display in German
  style

To test:
- Apply patch
- Repeat steps above
- Verify that address displays as expected

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18460: Fix undefined itemtype warning in Serials.t
Tomas Cohen Arazi [Wed, 19 Apr 2017 15:15:46 +0000 (12:15 -0300)]
Bug 18460: Fix undefined itemtype warning in Serials.t

This patch makes the test create an itemtype, and use it for the created item so there's no warning.

To test:
- Run:
  $ prove t/db_dependent/Serials.t
=> FAIL: item-level_itypes set but no itemtype set... warning raised
- Apply the patch
- Run:
  $ prove t/db_dependent/Serials.t
=> SUCCESS: Tests pass and no warning is raised
- Sign off :-D

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>
7 years agoBug 12021 - SIP2 checkin should alert on transfer and use CT for return branch
Benjamin Rokseth [Tue, 1 Apr 2014 12:07:59 +0000 (14:07 +0200)]
Bug 12021 - SIP2 checkin should alert on transfer and use CT for return branch

This small patch corrects the SIP2 checkin response if branch transfer is needed.
It depends on bug 7981 and also removes an obsolete alert check in SIP2.

Test plan:
 - apply bug 7981
 - make sure syspref AutomaticItemReturn is false
 - check 'Return policy' in 'Default checkout, hold and return policy'
 - make a SIP2 checkout and checkin from a branch different than item's home branch.
 - verify that SIP2 response gives alert CV04 (=send to different branch)
 - also verify that response field CT gives correct branch according to 'Return policy'

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18423 - Followup - enable Add child button for institutional borrowers
Nick Clemens [Tue, 18 Apr 2017 18:03:37 +0000 (14:03 -0400)]
Bug 18423 - Followup - enable Add child button for institutional borrowers

If they can have guarantors and we shwo the button on one page I think
we should in fact show it on all.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18423 - Add 'Add child' button to files, housebound, and deletemem pages
Nick Clemens [Tue, 18 Apr 2017 17:53:54 +0000 (13:53 -0400)]
Bug 18423 - Add 'Add child' button to files, housebound, and deletemem pages

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18423 Follow-up - Add 'Add child' button to statistics tab
Nick Clemens [Mon, 17 Apr 2017 16:32:56 +0000 (12:32 -0400)]
Bug 18423 Follow-up - Add 'Add child' button to statistics tab

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>
7 years agoBug 18423 follow up - fix display off add child on members/notices.pl
Nick Clemens [Wed, 12 Apr 2017 15:32:11 +0000 (11:32 -0400)]
Bug 18423 follow up - fix display off add child on members/notices.pl

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>
7 years agoBug 18423 - Add child button not always appearing - problem in template variable
Nick Clemens [Wed, 12 Apr 2017 14:23:56 +0000 (10:23 -0400)]
Bug 18423 - Add child button not always appearing - problem in template variable

This patch removes the retrieval of the syspref borrowerRelationship
from the scripts and moves it to a check using the Template Toolkit
plugin

To test:
1 - Apply patch
2 - Ensure 'Add child' button displays appropriately on all member pages.

Tested 3 pateches together, works as expected.
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>
7 years agoBug 17911: Message and timeout mismatch
Mark Tompsett [Mon, 16 Jan 2017 04:26:33 +0000 (23:26 -0500)]
Bug 17911: Message and timeout mismatch

The installation step 3 has a meta refresh of 10 seconds,
but the message says 5 seconds. People could get impatient.

TEST PLAN
---------
1) Fresh install
   -- notice the discrepancy on the web installation.
2) drop and create the DB
3) apply the patch
4) run the web install
   -- notice it refreshes faster (5 seconds vs 10 seconds)
5) run koha qa test tools

Works as intended and passes qa test tool
 OK koha-tmpl/intranet-tmpl/prog/en/modules/installer/step3.tt
  OK    forbidden patterns
  OK     git manipulation
  OK      spelling
  OK   tt_valid
  OK    valid_template

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>
7 years agoBug 17814: koha-plack --stop should make sure that Plack really stop
pongtawat [Mon, 26 Dec 2016 10:50:50 +0000 (17:50 +0700)]
Bug 17814: koha-plack --stop should make sure that Plack really stop

koha-plack --stop doesn't ensure that Plack was really stopped before
returning

So in case that koha-plack --stop was quickly follows by koha-plack
--start (e.g. logrotate), it could leave Plack in stop state. This is
due to koha-plack --start think that Plack was already started, while it
actually is being stopped.

For me I think this is the cause why I got random Plack stop when
logrotate is run.

It should be similar to the case of Zebra in Bug #16885, and the
solution might be the same: adding "--retry=TERM/30/KILL/5;" to
start-stop-daemon command.

TEST PLAN
---------
1) Login to staff client and do something that will hold connection for
a long time, e.g. a batch import or a slow report.
2) sudo koha-plack --stop mykoha
3) ps aux | grep plack   <-- a Plack process will still running work in
1)
4) wait for 1) to finish and all Plack processes exit
5) sudo koha-plack --start mykoha
6) apply the patch
7) repeat step 1)-2)
8) ps aux | grep plack   <-- There should be no Plack process running
now
9) Note that work in step 1) might get terminated midway. Since we force
Plack to stop after some wait.

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>
7 years agoBug 17502: Add info when throwing the exception
Jonathan Druart [Wed, 19 Apr 2017 18:34:17 +0000 (15:34 -0300)]
Bug 17502: Add info when throwing the exception

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17502: Throw some exceptions in output_pref
Marcel de Rooy [Thu, 23 Mar 2017 08:35:57 +0000 (09:35 +0100)]
Bug 17502: Throw some exceptions in output_pref

Test plan:
Run the adjusted t/DateUtils.t

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
NOTE: This patch is amended in QA. The exceptions are moved from a separate
module to the general section of Exceptions.pm.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17502: Add type check to output_pref
Marcel de Rooy [Thu, 27 Oct 2016 07:11:03 +0000 (09:11 +0200)]
Bug 17502: Add type check to output_pref

This patch makes the following changes:
[1] In Koha/DateUtils.pm, sub output_pref:
    Add a test if $dt is really a DateTime before calling method ymd.
    Preventing a crash like:
    Can't locate object method "ymd" via package "dateonly".
    See also BZ 17502/15822.
[2] Adds a few unit tests in t/DateUtils.t.

Test plan:
[1] Run the adjusted unit test t/DateUtils.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
7 years agoBug 18364: [Follow-up] Also add an environment variable to prevent locking
Marcel de Rooy [Thu, 20 Apr 2017 10:52:47 +0000 (12:52 +0200)]
Bug 18364: [Follow-up] Also add an environment variable to prevent locking

The test in SendCirculationAlert is extended by adding an env var
called KOHA_NO_TABLE_LOCKS. If this var is set to a true value,
the table locking is skipped too.

This is useful when running a test without prove. The variable could be
set in a shell profile.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18364: Do not LOCK/UNLOCK tables from tests
Jonathan Druart [Fri, 31 Mar 2017 16:43:38 +0000 (13:43 -0300)]
Bug 18364: Do not LOCK/UNLOCK tables from tests

From the MySQL doc:
"LOCK TABLES is not transaction-safe and implicitly commits any active transaction before attempting to lock the tables."
If the LOCK/UNLOCK statements are executed from tests, the current transaction will be committed.
To avoid that we need to guess if this code is execute from testsa or not (yes it is a bit hacky)

Better ideas are welcome!

Another fix would have been to revert
  commit be156d9ad9e5bcfadab34d44f90e04fd61e256ad
  Bug 15854: Use a READ and WRITE LOCK on message_queue
but theorically a race is still possible.

Existing tests seem to be safe, to test this patch you will need new
tests from bug 17964.
Test plan:
  prove t/db_dependent/Letters/TemplateToolkit.t
twice, and notice that changes have been comitted.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18439 - Resend button for notices being hidden by CSS and never unhidden
Nick Clemens [Mon, 17 Apr 2017 17:22:24 +0000 (13:22 -0400)]
Bug 18439 - Resend button for notices being hidden by CSS and never unhidden

Current jQuery is not unhiding resend button, this patch adds an id
and a toggle command

To test:
1 - Enable EnhancedMessagingPreferences
2 - Find a patron with sent or failed messages
3 - Note you can expand message, but not resend
4 - Apply patch
5 - Note that on expanding message a 'Resend' button appears
6 - Ensure resend button operates as expected

Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18001 - LocalHoldsPriority can cause multiple holds queue lines for same hold...
Kyle M Hall [Thu, 26 Jan 2017 14:31:35 +0000 (14:31 +0000)]
Bug 18001 - LocalHoldsPriority can cause multiple holds queue lines for same hold request

If LocalHoldsPriority is enabled, and a record level request has more
than one item that could fill that hold, there is a possibility that a
holds queue row will be generated for the request for each of the
available items!

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18001 - Unit Test
Kyle M Hall [Tue, 18 Apr 2017 11:38:42 +0000 (07:38 -0400)]
Bug 18001 - Unit Test

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17835: Re-add the use Koha in orderreceive
Jonathan Druart [Thu, 20 Apr 2017 12:56:09 +0000 (09:56 -0300)]
Bug 17835: Re-add the use Koha in orderreceive

It is still used for getitemtypeinfo, at least.

7 years agoBug 18372 - transits are not created at check in despite user responsing Yes to the...
Owen Leonard [Tue, 11 Apr 2017 16:45:40 +0000 (16:45 +0000)]
Bug 18372 - transits are not created at check in despite user responsing Yes to the prompt

If the AutomaticItemReturn system preference is set to "Don't" and you
check in an item which belongs to another library, a dialog will ask you
if you want to transfer. This patch corrects a bug to enable the "Yes"
buttons to work, triggering the transfer.

To test, apply the patch and check in an item which belongs to another
library. In the dialog, click the 'Yes' button and confirm that the item
has been made 'in transit' to the correct library.

Perform the same test with the 'Yes, print slip' and 'No' buttons and
confirm they are working correctly.

Since this patch changes the behavior of many other popups triggered
from this template, test other operations which trigger a 'print slip'
option like:

- Checking in an item which is part of a rotating collection
- Checking in an item which is on hold for a patron

Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17835: (followup) Make TestBuilder skip views
Tomas Cohen Arazi [Wed, 19 Apr 2017 14:19:26 +0000 (11:19 -0300)]
Bug 17835: (followup) Make TestBuilder skip views

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
7 years agoBug 18321: Correctly calculate switch_onsite_checkout
Jonathan Druart [Fri, 31 Mar 2017 03:24:09 +0000 (00:24 -0300)]
Bug 18321: Correctly calculate switch_onsite_checkout

Silly error here, the $switch_onsite_checkout is not correctly
calculated, it should not only be set to the value of the syspref, but
meet all the conditions.

Test plan:
Set a circ rule with number of on-site checkout = 2 and number of
checkouts = 2.
Switch on both ConsiderOnSiteCheckoutsAsNormalCheckouts and SwitchOnSiteCheckouts

Do a regular checkout
Do another regular checkout
At this point you have reach the maximum number of checkouts allowed.
Not try another one.
=> Without this patch you will not get the warning
=> With this patch you will see it

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18321: Add tests
Jonathan Druart [Fri, 31 Mar 2017 03:23:19 +0000 (00:23 -0300)]
Bug 18321: Add tests

Clean a bit existing tests by adding default values and add a test to
highlight the issue.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>