Bug 16517: Do not crash when creating a list with an existing name

A typo has been made during a merge conflict when rebasing bug 16517.

it should be
  eval { Koha::Object->new->store; };
instead of
  eval { Koha::Object->new; }->store;

Test plan:
From the result list (intranet side), check some entries and add them to
a new list
Use an existing list's name
=> Without this patch you will get a server error (exception 'Duplicate shelf
object' raised by Koha::Virtualshelf->new->store)
=> With this patch you must get a friendly error message

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16505: <collection> is missing the marc namespace and updates fail if -x is passed

Using rebuild_zerba.pl with the -x option switch, produces an incorrect output in
terms of what our XSLTs expect for indexing. This patch introduces the right namespace information
on the exported records so indexing succeeds.

To test:
- On current master, have some records on your db
- Run:
  $ sudo koha-shell kohadev
  $ cd kohaclone
  $ misc/migration_tools/rebuild_zebra.pl -r -b -k -x
=> you will get a message like this:

NOTHING cleaned : the export /tmp/NL5ufjUfpp has been kept.

- Run
  $ less /tmp/NL5ufjUfpp/biblio/exported_records
=> FAIL: The first line looks like this

<?xml version="1.0" encoding="UTF-8"?><collection><record

- Now run:
  $ xsltproc \
     /etc/koha/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl \
     /tmp/NL5ufjUfpp/biblio/exported_records
=> FAIL: No output
- Apply the patch
- Run:
  $ misc/migration_tools/rebuild_zebra.pl -r -b -k -x
- Take a look at the result file:
  $ less /tmp/asdiouqwiue/biblio/exported_records
=> SUCCESS: The start of the file looks like this:
<?xml version="1.0" encoding="UTF-8"?><collection xmlns="http://www.loc.gov/MARC21/slim">

- Run:
  $ xsltproc \
     /etc/koha/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl \
     /tmp/asdiouqwiue/biblio/exported_records
=> SUCCESS: There is actually indexing data :-D
- Sign off :-D

Edit: I changed qq{} for q{} as suggested by Jonathan.

Sponsored-by: American Numismatic Society
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Works as described following test plan
No errors

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16561 - Bug 15877 followup to fix tests

When I altered the tests to avoid accidental deletion of all items, I
didn't rollback the transaction, hence the tests now delete all items
from a db when run.

This patch rollsback the transaction so items are not deleted from your db

To test:
1 - Backup your db
2 - prove -v t/db_dependent/Barcodes.t
3 - Note that all items and issues are deleted from the DB
4 - Restore your DB
5 - Apply this patch
6 - prove -v t/db_dependent/Barcodes.t
7 - Your DB should eb unchanged

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16539: Add tests

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16539 - Koha::Cache is incorrectly caching single holidays

This makes the loop add holidays for each bracnh instead of the calling
branch

To test:
I used memcached-tools to view the cache, clear the cache, and wait for
holidays to regnerate

Have a branches with different holidays
Clear the cache
Do a checkout
View the cache - one libraries holidays are repeated for all branches
Apply patch
Clear the cache
Do a checkout
Library holidays are correctly cached

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 13669: Catch the errors to have them to the logs

This patch redirect STDERR to a variable to retrieve the errors raised
by the DBMS when loading a sql file, it could be useful to debug errors.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
It's better of course, trying to load a failed fiel
it outputs mysl errors
DBD::mysql::st execute failed: You have an error in your SQL syntax...
No errors

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 13669: Re-adds error handling to load_sql

This patch prevents crashing in case an
error is detected when loading a file

To test:
1) Apply patch
2) Mangle kohastructure.sql or any sample
file adding and invalid SQL line
3) Run webinstaller and check that the error
is handled gracefully

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 13669: (alternative) use DBIx::RunSQL

Alternative POC solution, on top of
previous patches, feel free to obsolte.

This patch use DBIx::RunSQL->run_sql_file
to procees each sql file.

To test:
1) Apply all patches
2) Same test plan of patch 1

Timing test running web installer, marc21,
all sample data, time in seconds

a) without patch
structure   data
59.7        66.5
58.6        66.0

b) SQL::SplitStatement (patch 1+2)
59.4        101
59.7        102

c) DBIx::RunSQL (patch 3)
60.7        66.8
59.4        66.2

Tomas' version is a bit slower loading sample data,
all give similar results processing kohastructure.

New dependency to package: DBIx::RunSQL

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 13669: (followup) Fix kohastructure.sql so FK dependencies are sorted

The current table creation order is left to mysql's strategy, which is not
suitable for parsing the SQL files and passing one statement at a time in
the current order.

This patch just moves table creation statements around so FK constraints are
defined for previously created tables.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 13669: Use perl to load SQL statements from sample data

This patch changes C4::Installer::load_sql so it uses File::Slurp [1] to read
the SQL files, and SQL::SplitStatement to extract the statements from the full SQL
file so they can be passed to $dbh->do.

To test:
- On Mysql 5.5, run the webinstaller
=> SUCCESS: Everything works as expected
- Apply the patch
- Run
  $ mysql -uroot
  > DROP DATABASE koha_kohadev ; CREATE DATABASE koha_kohadev;
- Run the webinstaller
=> SUCCESS: Everything works as expected
- On Mysql 5.6+ (5.7 is implied)
- Run
  $ mysql -uroot
  > DROP DATABASE koha_kohadev ; CREATE DATABASE koha_kohadev;
- Run the webinstaller
=> FAIL: It cannot load the sql files due to a password-in-command-line error
- Apply the patch
- Make sure everything is clean (it should):
  $ mysql -uroot
  > DROP DATABASE koha_kohadev ; CREATE DATABASE koha_kohadev;
- Run the webinstaller
=> SUCCESS: EVerything works as expected
- Sign off :-D

[1] Note: This is a POC patch, in the sense that it does the job, fixes a nasty problem
but using File::Slurp to load the SQL files in memory comes with a big runtime penalty.
You will notice the install procedure is now much slower, for instance.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16470: Update MARC21 es-ES frameworks to Update 22 (April 2016)

Same test plan of Bug 16460

To test:
1) Remove all your frameworks
2) Check mandatory bibliographic and authority files load without problem, for es-ES files
3) Check new fields/subfields

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
es-ES frameworks updated

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16460: Update MARC21 frameworks to Update No. 22 (April 2016)

Update No. 22
Bibliographic
    New 382$r, 382$t, 753$0, 753$2
    Modified 382$e

Authority
    New 382$r, 382$t
    Modified 382$e

Also added final 'S' on 654 description

To test:
1) Remove all your frameworks
2) Check mandatory bibliographic and authority files load without problem
3) Check new fields/subfields

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as advertised

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
For comparison:
https://www.loc.gov/marc/bibliographic/bdapndxg.html
https://www.loc.gov/marc/authority/adapndxf.html

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16382: Update German web installer sample files for 16.05

Easy this time, only sample notices have been changed:
- Changed discharge to match the English sample notice.
- Translated new PASSWORD_RESET e-mail

To test:
- Start with an empty database
- Install de-DE
- Run the web installer in de-DE/German
- Verify all files, especially sample notices, load
  without any problems.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Tested removing all from letter table and loading file.
File loads Ok, notices look good.
No errors

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16501: Remove some unneeded warns in Upload.t

At some point I saw some gain in warning about a missing config line.
But these warns are actually not needed here.
Removing them here.

Test plan:
Run t/db_dependent/Upload.t while having upload_path in your koha-conf and
also without it.

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>

Bug 16471: Translatability: Fix issues in opac-password-recovery.tt

This patch fixes issues with the translatability of opac-password-recovery.tt

To test:
- Apply patch
- Verify that text changes make sense.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16454: Remove another occurrence in comment

Note that few other occurrences exist in DB.

This patch also replaces a tab with 4 spaces.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16454: Use 'inventory' instead of 'inventory/stocktaking'

To test:
1) Go to the tools home page. Confirm it says 'Inventory' instead of
   'Inventory/stocktaking' in heading and in description
2) Click Inventory. Confirm heading now says 'Inventory' instead of
   'Inventory/stocktaking'
3) Confirm that it now says 'Inventory' instead of
   'Inventory/stocktaking' in the tools menu along the left of the page
4) Click Help in top right corner. Confirm help heading now says
   'Inventory' instead of 'Inventory/stocktaking'
5) Go to user account page, go to Set Permissions
6) Expand Tools option and confirm it says 'inventory' instead of
   'inventory (stocktaking)'

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>

Bug 16385: Fix breadcrumbs when ordering from subscription

- Rephrase removing 'Shopping'
- Fix link to vendor

To test:
- Create a subscription for vendor X
- Create a baskest for vendor X
- Choose: order from subscription
- Check breadcrumbs:
  - Link to vendor works
  - 'Shopping' is gone, Basket remains

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>

Bug 15086 - DBRev 3.23.00.064

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 15086: Creators layout and template sql has warnings

When doing a hacked install off the master branch:
use ... {koha database name}
truncate creator_layouts;
truncate creator_templates;
truncate printers_profiles;
source installer/data/mysql/... {name of a sample_labels type file}
Warnings are generated, which may not be visible in the UI.

Most of the warnings were triggered by:
-/*!40000 ALTER TABLE `creator_layouts` DISABLE KEYS */;
-/*!40000 ALTER TABLE `creator_layouts` ENABLE KEYS */;
http://dev.mysql.com/doc/refman/5.7/en/error-messages-server.html#error_er_illegal_ha

In the Russian, the layout_name was truncated, because the layout_name was only 20. An atomic update sql and kohastructure.sql update were provided to widen it to 25.
http://dev.mysql.com/doc/refman/5.7/en/error-messages-server.html#error_warn_data_truncated

Also fr-FR, ru-RU, and uk-UA were slightly different in structure, so the structure was made the same as the other files.

See comment #1 for the test plan.

NOTE: pl-PL is likely very out of date, but is not affected in this regard.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Works, no more warnings
mysql> show warnings;
Empty set (0.01 sec)

No errors

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16481 - Report menu has unexpected issues

This patch corrects a problem with the reports toolbar include file
which can cause a problem with the "duplicate" button. The duplicate
button passes the entire report SQL as a URL parameter, but the template
variable was not escaped as a URL variable.

This patch also adds the "uri" filter to the report name and notes
fields just to be safe.

To reproduce the bug, create a new SQL report using the SQL in the bug
report. When you run the report you should see a broken "Duplicate"
button.

Apply the patch and run the report again. The duplicate button should
look correct and work correctly.

Signed-off-by: Aleisha <aleishaamohia@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16259: Replace CGI->param with CGI->multi_param in list context - part 2

This patch is a follow-up of bug 16154.

It removes the warning "CGI::param called in list context" in the
following scripts:
admin/branches.pl
admin/categories.pl
admin/patron-attr-types.pl
admin/preferences.pl
catalogue/image.pl
circ/circulation.pl
patroncards/add_user_search.pl
serials/add_user_search.pl
tools/marc_modification_templates.pl
virtualshelves/shelves.pl

Note that the warning from catalogue/itemsearch.pl still exists (the
call to CGI->param is done from the template).

Test plan:
- Add/modify a library, patron category, patron attr type
- Update a syspref
- Set localcoverimage and call catalogue/image.pl?biblionumber=XXX
- Search for patrons in the patron cards or serials module
- Add a marc modification templates
- Add a list (shelves)

You should not get the warning in the log after all these actions.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 1859: Do not display <<>> if an entry without value is selected

For instance there are ---TABLES--- to delimite fields' tables, if
selected we do not want to add "<<>>" to the letter.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 1859 - Notice fields: can't select multiple fields at once

When selecting database fields to appear in a notice, selecting multiple
fields at once doesn't work correctly. The selected fields are grouped
together between << and >> instead of each being bracketed separately.

This patch corrects this problem. Also changed: Removal of 'onclick';
Conversion of 'insert' button to a Bootstrap buttons styled with a Font
Awesome icon; HTML validation error fixes.

To test, apply the patch and go to Tools -> Notices & Slips.

- Edit any notice.
- Select a notice version to edit (Email, Print, etc.).
- Confirm that the "Insert" button looks correct.
- Using ctrl+click, select multiple database fields to add to the
  notice. Confirm that clicking the "Insert" button adds the fields
  correctly.
- Confirm that there are no HTML validation errors.

Signed-off-by: Nicole C Engard <nengard@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16288: (QA followup) Remove the syspref INSERT line

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>

Bug 16288: Remove unused EDI Shipping Budget syspref

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>

Bug 16288: Remove Reference to obsolete EAN syspref

Remove reference to obsolete syspref EDIfactEAN
which was present in earlier test version of
EDI code but should not now be used

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>

Bug 16476: Do not call CGI->param in list context, some more

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16402 - DBRev 3.23.00.063

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16402: (QA followup)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16402: Fix fox existing installations

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16402: Make kohastructure.sql valid for MySQL v5.7

A primary key should not contain null value, and MySQL v5.7 is now more
strict and won't accept our errors in the DB structure.

Test plan:
  mysql < installer/data/mysql/kohastructure.sql
should work using MySQL v5.7

QA:
- letter.branchcode should not be NULL, the default (all) is an empty
string
- permissions.code should not be NULL, it's always a string

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Clean install of Ubuntu 16.05, MySQL 5.7.11
kohastructure.sql loads ok, just one warning
No errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 15816: Redirect back to correct page after login

This uses a hacky but simple method to get the correct script name under
proxied packaged Plack.

Test plan:
  1) Log out of both the OPAC and staff side.
  2) Try to access a page that requires login (opac-reserve.pl is a
good one for the OPAC), then log in.
  3) You will be redirected back to mainpage.pl or opac-user.pl.
  4) Repeat above for both staff side and OPAC.
  5) Apply patch.
  6) Repeat steps 1-4; you should be redirected back to the original
     page you were on.
  7) Repeat the above for both a traditional CGI and kohadevbox/package
     Plack installation.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16473: Fix typo "an problem" vs "a problem"

Test plan:
Confirm the wording is correct

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>

Bug 16480: Unclosed tag <span> in shelves line 332 on intranet

Unclosed tag <span> in shelves line 332 on intranet when translating
<span>An error occurred when creating this list./span>
This error was introduced by bug 15403

To test: Verify the template in line 332 to see the unclosed tag fixed
or follow test plan for bug 15403

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>

Bug 16474: Standardize spelling of EDIFACT

Some templates use Edifact and edifact. As EDIFACT
is an abbreviation, we should spell it like one.

To test:
- Check templates for occurrences of EDIFACT
  - acquisitions navigation menu
  - basket page
  - basketgroup page
  - EDIFACT messages page
- Verify changes are correct

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Changes are correct

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>

Bug 16412: Cache undef in L1 only

No need to cache undef in L2.
Prevent numerous warnings like:
Use of uninitialized value in subroutine entry at /usr/share/koha/masterclone/Koha/Cache.pm line 284

Test plan:
Run t/Cache.t
Run t/db_dependent/sysprefs.t

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Amended patch: remove trailing space char and replace 'you' with 'we' to
match existing wordings.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>

Bug 16412: Make sure Koha::Cache->set_in_cache will not warn anymore

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>

Bug 16489: ES code incorrectly refers to Moose

Probably a leftover from the original SolR code, there are unused references to Moose in the new Elasticsearch code.

To test:
- Run:
  $ git grep Moose
=> FAIL: There are unneeded references to Moose:
Koha/SearchEngine/Zebra/QueryBuilder.pm:use Moose::Role;
Koha/SearchEngine/Zebra/Search.pm:#use Moose::Role;
- Apply the patch
- Run:
  $ git grep Moose
=> SUCCESS: No references to Moose
- Run:
  $ prove t/Search* t/db_dependent/Search*
=> SUCCESS: Tests pass
- Sign off

Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>

Bug 13871: [QA Follow-up] Fix small regression on AF with branchcode in Patron Info request

If you enable send_patron_home_library_in_af in your sip account, you
want a separate AF field for the home branch.

Test plan:
Send a 63 (Patron Info) and verify that you have an extra AF.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested Patron Status and Patron Info.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16496: Add test

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16496 - accountlines.issue_id not set when new overdue is processed

It appears that somehow the adding of issue_id to accountlines for new
fines was missed! This is incredibly important, otherwise UpdateFine
will always create a new fine!

Test Plan:
1) Create a new overdue checkout
2) Run fines.pl to generate an accountline for it
3) Note it has no issue_id
4) Apply this patch
5) Repeat steps 1 and 2
6) Note it now has an issue_id!

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16414 - Update Schema

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Merges translations for Koha 16.05

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 13041 - Can't add user as manager of basket if name includes a single quote

If you are trying to add a user as a manager of a basket in
acquisitions, a JavaScript error will be triggered if that user has a
single quote in their name (e.g. "O'Neil"). This patch corrects the
issue by removing the "onclick" attribute and using the JS change made
in Bug 16386.

Also changed by this patch: Increased the size of the patron search
popup and made a correction to some invalid HTML.

To test you should have a patron whose name contains a single quote who
is also a user with permission to manage acquisitions.

- Apply the patch and go to Acquisitions.
- Locate an open basket and view the details for that basket.
- In the "Managed by" section, click the "Add user" button to trigger
  the patron search popup.
- Search for the patron described above and click the "Add" button.
- In the parent window, the patron you chose should have been added to
  the "Managed by" section.

Tested on top of Bug 16383, behaves as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Bug 16386 - Replace the use of "onclick" from patron card creator templates

This patch removes the use of "onclick" from several patron card creator
templates. Events are defined instead in JavaScript.

Also changed:

- Patron search pop-up window is now slightly larger because I found it
  never quite large enough to prevent horizontal scrolling.
- Replaced "Borrower" with "Patron" in a couple of places.
- "Add" link in patron search pop-up is now styled as a Bootstrap
  button.
- Removed Bootstrap styles from some submit buttons.
- Some Font Awesome icons were made using the invalid element
  <icon></icon> instead of <i></i>. These are corrected.
- Fixed some other HTML validation errors.

To test, apply the patch and go to Tools -> Patron card creator.

- Choose New -> Card batch.
  - Click "Add patrons" and perform a search for patrons in the pop-up
    window.
  - Click the "Add" button. The corresponding borrowernumber should be
    added to the textarea in the parent window, and a message should
    appear at the top of the pop-up window confirming that the patron
    has been added. (Note: This patch fixes a version of Bug 13041 which
    prevented the "add" button from working on patrons whose name
    include an apostrophe).
  - In the parent page, in the list of patrons you added to the batch,
    confirm that clicking the "Delete" link triggers a confirmation
    dialog. Test both confirming and canceling.
  - Confirm that clicking the "Export" button next to an individual
    patron triggers a modal window.
    - In the export window, confirm that the "Cancel" link works to
      close the modal.
    - Click the "Export" button again and then the "Export" button in
      the modal.
      - Test that the "Done" button works to close the modal.
- Choose Manage -> Card batches.
  - Test the "Delete" and "Export" buttons as described above.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
All works, no errors.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Bug 15823: Redirect opac-discharge.pl to 404 page

See comment #17: Redirect to 404 in opac-discharge.pl and remove
message in template because with the redirect it will never be
reached.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Bug 15823: Can still access patron discharge slip without having the syspref on

EDIT: Fix for OPAC side
EDIT: Comment 10
EDIT: Merge conflicts

To test:
1) Ensure syspref useDischarge is disabled
2) Go to /cgi-bin/koha/members/discharge.pl?borrowernumber=X&discharge=1
3) Validate that you are still able to generate a discharge slip for this patron
4) Apply patch and refresh page
5) Confirm that you are redirected to the circulation.pl page for the user and that an error message is there.
OPAC SIDE
6) Go to the OPAC
7) Go to /cgi-bin/koha/opac-discharge.pl
8) Confirm you get a message saying discharges are disabled
9) Go to /cgi-bin/koha/opac-discharge.pl?op=request
10) Confirm you see same message

Sponsored-by: Catalyst IT
Followed test plan, works as expected (both staff client and OPAC).
Re-tested, works OK.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Merge branch 'master' of git://git.koha-community.org/koha

Bug 15919: Batch checkout - Display the due date

When an item has been checked out in a batch, the due date will now be
displayed in the information column.

Test plan:
Use the batch checkout to check some items out
Confirm that the due date is displayed if the items have been checked
out.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

DBRev Bug 16414 - aqorders.budgetgroup_id has never been used and can be
removed

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Bug 16414: Follow-up - change to kohastructure.sql

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Bug 16414: Drop aqorders.budgetgroup_id

This column has never been used and can be removed.

Test plan:
1/ update the schema
2/ prove t/db_dependent/Acquisition.t
should return green

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Test green pre & post patch
No errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Translation updates for Koha 16.05 (beta) release

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16451: Fix missisng body id in orders_by_budget.tt

Bug 11371 introduced a new template, which fails the xt/tt_valid.t tests.

To test:
- On master, run:
  $ prove xt/tt_valid.t
=> FAIL: Tests fail on orders_by_budget.tt
- Apply the patch
- Run
  $ prove xt/tt_valid.t
=> SUCCESS: Tests don't fail on orders_by_budget.tt
- Sign off

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Bug 16453: Make Elasticsearch tests be skipped if configuration entry missing

The current tests fail to run if the configuration entry is missing. This is
problematic on jenkins and should be fixed.

To test:
- On master, having koha-conf.xml without (or commented) an <elasticsearch> entry
- Run:
  $ prove t/db_dependent/Koha_ElasticSearch_Indexer.t \
          t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t -v
=> FAIL: Tests fail due to missing configuration entry
- Apply the patch
- Run:
  $ prove t/db_dependent/Koha_ElasticSearch_Indexer.t \
          t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t -v
=> SUCCESS: Tests pass, and the ES-configuration-dependent tests are skipped
- Have elasticsearch running and the koha-conf.xml entry
- Run:
  $ prove t/db_dependent/Koha_ElasticSearch_Indexer.t \
          t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t -v
=> SUCCESS: Same results as without the patch
- Sign off

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Bug 16426: follow-up of bug 15840 - correctly manage userid while inserting patrons

Bug 15840 tried to fix a bug but makes things more complicated than it
was before.
If an userid is not provided for 1 or more rows of the csv file, it
should not be updated. However, if a userid is provided and it already
used by an other patron, the import should fail for this row (but not
crash!).

Test plan:
0/ Create a patron with a userid=your_userid
1/ Use the import patron tool to update this userid
=> userid should have been updated
2/ Update another data and do not provide the userid
=> data should have been updated and not the userid
3/ Update another data and provide the userid, but set it to an empty
string, or '0'
=> data should have been updated and not the userid
4/ Update another patron, and set userid=your_userid
=> Update should fail and an error whouls be displayed ("already used by
another patron")

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Bug 16426: Add tests for ModMember - do not update userid

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Bug 16447: Remove occurrence of the borrow permission which does no longer exist

Bug 7976 has removed this permission, but other patches re-added it...
Note that the occurrences in sendbasket.pl, edithelp.pl, opac/svc/login should
have been removed by bug 7976.

Test plan:
  git grep 'borrow.*=> 1'
should not return any results.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>

Bug 16448: Fix perlcritic errors introduced by 12478

Bug 12478 introduced two perlcritic errors. This patch fixes them.

To test:
- On master, run:
  $ prove t/00-testcritic.t
=> FAIL: catalogue/search.pl and Koha/Authority.pm fail the critic tests.
- Apply the patch
- Run:
  $ prove t/00-testcritic.t
=> SUCCESS: All happy
- Sign off :-D

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
All test successful. Needs $ export TEST_QA=1

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>

Bug 16325: Do not return all suggestions if search for STATUS=''

This is a quick and dirty way to fix a bad bug in a messy area.
The "unknown status" tab in the suggestions table display all the
suggestions. It should only display suggestions with a STATUS=''

Test plan:
- Create some suggestions
- Go to Home > Acquisitions > Suggestions management
- Edit some suggestions and give them different status,
e.g. accepted, rejected, pending.
- Verify that they appear in the tabs as appropriate
- Edit one suggestion, set "Mark selected as" to --Choose a status--
=> Without this patch: New tab "Status unknown" containing all
suggestions
=> With this patch: tab contains only suggestions with "Unknown status"

Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>

Bug 16325: Add a test for SearchSuggestions when searching for STATUS=''

Test fails as expected without second patch and passes OK with second patch.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>

Bug 16452: Remove the warnings raised by PatronLists.t

We need to define a userenv to get rid of the warnings

Test plan:
  prove t/db_dependent/PatronLists.t
should not return any warnings

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>

Bug 16439: Allow styling to button for upload local cover images (Font Awesome Icons)

Improve button for upload local cover image with Font Awesome Icon

To test:
-Set syspref LocalCoverImages and OPACLocalCoverImages to Allow
-A new tab 'Images' appear in bib record detail on Intranet.
-Notice about the button upload.
-Apply the patch and reload the page.
-The button is changed for class=btn btn-mini like the button "edit"
 used in items in the tab Holdings
-Also added the Font Awesome icon upload "fa fa-upload" class.

Button is styled as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>

Bug 16445: Revert changes made by bug 12478 to Koha/Database.pm

These changes were a mistake, let's revert them.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>

Bug 16438: (followup) remove item where no single icon exist for options

Remove the double icon in options where double icons don't work well

To test: follow the previous commit

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>

Bug 16438: Use Font Awesome icons in batch templates

Add Font Awesome icons in tables in authorities and bib records
in batch item/record deletion, modification templates

To test:
1- Go to Tools > Batch item/record deletion/modification
2- Try to delete/modify itmes/bib and authorities records
3- Look all tables presented and notice about the new icons in Select all
   Clear all, etc.
4- Apply patch and repeat step 1 to 3

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Work, nice view.
No Errors

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>

Bug 16159 - guarantor section missing ID on patron add form

In the patron entry form template most <fieldset> and <legend> tags have
unique ids. This patch adds ids to fieldsets and legends which lack
them.

To test apply the patch and view the patron entry form. There should be
no visual changes. There should be no HTML validation errors triggered
by this change.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>

Bug 16415 - Layout problem on staff client detail page if local cover images are enabled

This patch fixes a layout problem which appears on the staff client
bibliographic detail page if local covers are enabled but Amazon covers
are not.

To test, apply the patch and turn off AmazonCoverImages.

- Enable the LocalCoverImages system preference and locate or create a
  record which has a local cover image associated with it.
- View the bibliographic detail page. The bibliographic details should
  display in two columns, with text on the left and local cover image on
  the right.
- Test with AmazonCoverImages and LocalCoverImages enabled.
- Test with AmazonCoverImages enabled and LocalCoverImages disabled.

All combinations are OK.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>

Bug 15194 - Drop-down menu 'Actions' has problem in 'Saved reports' page with language bottom bar

This patch changes the direction of the "actions" menu on the saved
reports page so that it popup up instead of down.

To test, apply the patch and go to Reports -> Saved reports.

- Click the "Actions" menu for any report and confirm that the menu
  displays above the button instead of below it.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>

Bug 16315 - OPAC Shelfbrowser doesn't display the full title

This patch adds subtitle information to the display of titles in the
OPAC's shelf browser.

To test, apply the patch and make sure OPACShelfBrowser is enabled.

- View the detail page for any title in the OPAC which has items.
- Click the "Browse shelf" link next to any item in the holdings table.
- The titles in the shelf browser should display with all subtitle
  information as defined in Keywords to MARC mapping.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Adding 245a and c as 'subtitle' in Keywords to Marc make them
show on shelf browser.
No errors.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>

Bug 16411: Make Hold.t not depend on two existing branches

If you do not have two branches, this test will fail.
Can't call method "branchcode" on an undefined value.

This patch adds a borrower and two branches with TestBuilder.

Test plan:
Run the test.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>

Bug 16429 - Fix root problem

The root of this issue is old code in circulation.pl that is no longer
needed and leaves the affected scripts open to future regressions.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Heather Braum <hbraum@nekls.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16429 - Going to circulation from notice triggers may change logged in branch

If you edit notice triggers and then use the "Checko out" tab from the
top bar, you may change your logged in library by accident!

Test Plan:
1) Disable the system preference CircAutocompl
2) Go to the notice triggers
3) Select the triggers for a library other than the one you logged in as
4) Use the "Check out" tab at the top to check out to a patron
5) Note your logged in branch has changed ( you'll need to refresh the
    page if the check out search took you directly to one patron ).
6) Apply this patch
7) Repeat steps 2-4
8) Note the branch no longer changes

Tested with both patches applied. Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Heather Braum <hbraum@nekls.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16390: Accounts.t does not need MPL

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>

Bug 16405: Fix Circulation/NoIssuesChargeGuarantees.t test 2

Resolves:
Failed test 'Patron cannot check out item due to debt for guarantee'
at t/db_dependent/Circulation/NoIssuesChargeGuarantees.t line 63.
          got: '10'
     expected: '10.00'

Test plan:
Run the test again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 10171: (QA followup) Make search tests use the right form

This bug removed the search tab from the main search box, so the tests
that use /cgi-bin/koha/catalogue/search.pl for searching, should use
the advanced search box instead.

To test:
- Run
  $ prove t/db_dependent/www/search_utf8.t
=> FAIL: Several tests related to searching on the intranet interface fail.
- Apply the patch
- Run
  $ prove t/db_dependent/www/search_utf8.t
=> SUCCESS: Tests pass
- Be happy

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 14622 - New data in French Canadian for the web installer. Add "fr-CA" folder.

 TEST PLAN :
 - install Koha
 - choose fr-CA as the installation language
 - check every options in the web installer
 - every file should load smoothly

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Works well, no problem/warning loading all sample files.
No koha-qa errors

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 7143: Moving about to 16.05. Hurray.

Any suggestions to highlight the version number more prominently or
elegantly are welcome. Just add a follow-up.

Test plan:
Go to About. Check Koha team. Hover over the version number.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
16.05, Yeah!
No errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 13903: (QA followup) change routes to /holds

GET    /holds?borrowernumber=X (list)
POST   /holds                  (create)
PUT    /holds/{reserve_id}     (update)
DELETE /holds/{reserve_id}     (delete)

Unit tests in t/db_dependent/api/v1/holds.t

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 13903: Add API routes to list, create, update, delete reserves

GET    /reserves?borrowernumber=X (list)
POST   /reserves                  (create)
PUT    /reserves/{reserve_id}     (update)
DELETE /reserves/{reserve_id}     (delete)

Unit tests in t/db_dependent/api/v1/reserves.t

Test plan:
1. Apply patch
2. Run unit tests
3. Play with the API with your favorite REST client, using documentation
   in the swagger.json file
4. Try to make reserves until the maximum number of reserves for a user
   is reached (you should have a 403 error)

Depends on bug 15126

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 15126: Update patron definition

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 15126: Update dependencies

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 15126: x-mojo-controller deprecation

Remove the use of soon to be deprecated x-mojo-controller from our
specification and replace with the recommended operationId format.

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 15126: Refactor spec file

Spreading the specification over multiple files should lead to a more
manageable specification long term

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16155: (QA followup) fix small bug in t/db_dependent/ILSDI_Services.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
With all patches applied all test pass,
all == git grep -l "use t::lib::TestBuilder" | grep -v -e 'pm$' -e Old | xargs prove

No errors

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16155: [QA Follow-up] Add transaction to BiblioFrameworks.t

This unit test does not have a transaction.
It does not need TestBuilder.

Test plan:
[1] Optionally remove records with mfw1, mfw2 from biblio_framework table.
    If you ran this test before and it failed, you may have them.
[2] Run the test.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16155: Adjust a few other tests

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>

Bug 16155: Adjust TestBuilder.t

The changes in TestBuilder.pm require some changes in this test.

[1] Tests have been organized under subtests. A few superfluous tests have
    been removed. (There is still some overlap between the sections
    of overduerules_transport_type and userpermission.)
[2] The results in the build all sources-test are checked one step further.
[3] Tests are added for field length, null values and delete method.
[4] The former defaults from TestBuilder are incorporated in the tests
    for userpermission.

Test plan:
Run t/db_dependent/TestBuilder.t

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16155: Remove a second use from Members_Attributes.t

Test plan:
Run t/db_dependent/Members_Attributes.t

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16155: Composite keys in TestBuilder and more

The number of tests using TestBuilder is gradually growing. Once you
are familiar with its use, you will appreciate it and find yourself
using it when writing new tests. Although it works quite well, some details
still needs some polishing.

While looking at the handling of composite keys in TestBuilder, a number
of related points came up too. This patch finally ended up in setting
the following design goals:

[1] TB should not only look at the first column of a composite FK.
[2] TB should optionally create records for fixed FK values.
[3] TB should create a new record, never change an existing record.
[4] TB should respect auto_increment columns.
[5] Passing a hash for a foreign key should always imply a new record.
[6] Explicitly passing undef for a column should mean NULL.
[7] Add a delete method; it will replace the clear method.
[8] Simplify by removing $default_values, hash key _fk and param only_fk.

The comments below further clarify these points. Note that they refer to
the old behavior (before this patch) unless stated otherwise.

Comments for point 1
====================
Look at:
    $builder->build({
        source => 'UserPermission',
        value => {
            borrowernumber => $borrowerno,
            module_bit => { flag => 'my flag' },
            code => 'will_be_ignored',
        },
    });
Module_bit and code here are a composite FK to permissions.
TB ignores the value for the code column here and creates a record with a
randomized code.

But if we would pass the hash in the second column of this compound key like:
            borrowernumber => $borrowerno,
            module_bit => 10,
            code => { code => 'new_code' },
TB would now crash when passing the hash for code thru to DBIx.

Comments for point 2
====================
Look at:
    $builder->build({
        source => 'UserPermission',
        value => {
            borrowernumber => $borrowerno,
            module_bit => 99,
            code => 'new_super_tool',
        },
    });
TB detects a fixed value for the module_bit, continues and will crash on
DBIx if the foreign keys are not found. In this case it would be friendly
to create a missing linked record.

Comments for point 3
====================
Look at:
    $builder->build({
        source => 'Branch',
        value  => { branchcode => 'CPL' },
    });
If this branch already exists, this call would modify the branch record and
overwrite all columns with randomized values (expected or not). In any case,
it would be safer here to return undef than modifying an existing record.

Comments for point 4
====================
Look at:
    $builder->build({
        source => 'Borrower',
        value  => { borrowernumber => '123456789' },
    });
If this number would exist, we would update (as earlier). But if this
number does not exist, we would create the record. Although that is
technically possible, I would prefer to have TB respect the auto
increment property of this column.

Comments for point 5
====================
Look at:
    $builder->build({
        source => 'Item',
        value  => { homebranch => { branchcode => 'MPL' } },
    });
As discussed under point 3, we should actually not pass primary key values
if we expect to build new records. The same also holds for the deeper
(recursive) calls to build when using hashes like here for homebranch.
In this case again an existing record might be overwritten.

Comments for point 6
====================
Look at:
    $builder->build({
        source => 'Reserve',
        value => { itemnumber => undef },
    });
As you know, a reserve without an itemnumber is a biblio level hold.
Unfortunately, TB did not care about passing undefs until now. So you would
get an item level hold.
In the new situation TB will respect these explicit undefs, as long as the
corresponding foreign key column is nullable of course.

Comments for point 7
====================
This patch will allow you to delete records created by TB:
    my $patron = $builder->build({ source => 'Borrower' });
    $builder->delete({ source => 'Borrower', records => $patron });
Or:
    $builder->delete({ source => 'Borrower', records => [ $patron, ... ] });
For safety, delete requires you to provide all primary key values in the
passed hashref(s).
Deleting all records in a table via clear is no longer supported and can
still be arranged in one statement.

Comments for point 8
====================
Current use of TestBuilder reveals that $default_values and only_fk
are not really needed. The current $default_values should imo not be in the
module anyway; if you want to use it, you could still pass it to TB:
    $builder->build({ ..., value => { %defa, %your_values } });

Only_fk stops at the very last step of saving the top level record while
storing all linked records at the lower levels. Practical use is not
very obvious; it can be easily simulated by one delete statement.

The hash key _fk is now used to store all linked records one or more levels
down. It is used in a few tests to retrieve a value one level down.
Why not retrieve that one value via the database and get rid of the
whole structure?

Implementation
==============
This highlights the main changes:

The $default_value hash (with some hardcoded values for UserPermission)
is removed from the module. It was used by a test in TestBuilder.t and has
been relocated.
The value of $gen_type is returned now by sub _gen_type. (See new.)

The main change in the build method is moving the foreign keys logic to a
new subroutine _create_links. This routine now looks at all columns of a
composite FK. It checks if a linked record exists for passed values, and
it looks at NULL values.

Routine _buildColumnValues is slightly adjusted to allow for passed undef
values. The theoretically endless loop is replaced by three tries. For
composite unique constraints we only check complete sets of values.

Routine _getForeignKeys contains a check to not return the same relation
twice in case of doubled belongs_to relations in the DBIx scheme.

The eval in _storeColumnValues is removed. The autoincrement check in sub
_buildColumnValue got more priority; the handling of foreign keys has been
adjusted and a check for not-nullable columns has been added.

TEST PLAN
=========
This patch only deals with the TestBuilder module itself. In the follow-up
patches TestBuilder.t and a few other unit tests are adjusted.

[1] Do not yet apply this patch. But apply the 'OldBehavior' patch.
    Verify that all tests pass. (They cover the first six design goals.)
[2] Apply this patch. Does TestBuilder still compile (perl -c)?
[3] Run the OldBehavior test again. Do all tests fail now? This means
    that we got rid of all unwanted side-effects in the list of goals.
[4] Run some other tests that use TestBuilder. (See below.)
    Skip the following tests; a follow-up patch deals with them.
    t/db_dependent/Accounts.t
    t/db_dependent/Barcodes.t
    t/db_dependent/Circulation/AnonymiseIssueHistory.t
    t/db_dependent/Circulation/CalcFine.t
    t/db_dependent/Holds.t
    t/db_dependent/Items/MoveItemFromBiblio.t
    t/db_dependent/Koha/BiblioFrameworks.t
    t/db_dependent/Members.t
    t/db_dependent/TestBuilder.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
The following tests pass:
    t/db_dependent/Acquisition/OrderUsers.t
    t/db_dependent/Auth/haspermission.t
    t/db_dependent/Barcodes_ValueBuilder.t
    t/db_dependent/Budgets.t
    t/db_dependent/Category.t
    t/db_dependent/Circulation.t
    t/db_dependent/Circulation/GetTopIssues.t
    t/db_dependent/Circulation/IsItemIssued.t
    t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t
    t/db_dependent/Circulation/Returns.t
    t/db_dependent/Circulation/TooMany.t
    t/db_dependent/Circulation_dateexpiry.t
    t/db_dependent/Circulation_transfers.t
    t/db_dependent/CourseReserves.t
    t/db_dependent/Creators/Lib.t
    t/db_dependent/DecreaseLoanHighHolds.t
    t/db_dependent/Exporter/Record.t
    t/db_dependent/Holds/LocalHoldsPriority.t
    t/db_dependent/Holds/RevertWaitingStatus.t:
    t/db_dependent/HoldsQueue.t
    t/db_dependent/Holidays.t
    t/db_dependent/Items.t
    t/db_dependent/Items_DelItem.t
    t/db_dependent/Koha/Acquisition/Currencies.t
    t/db_dependent/Koha/Authorities.t
    t/db_dependent/Koha/BiblioFrameworks.t
    t/db_dependent/Koha/Cities.t
    t/db_dependent/Koha/Libraries.t
    t/db_dependent/Koha/Objects.t
    t/db_dependent/Koha/Patron/Categories.t
    t/db_dependent/Koha/Patron/Images.t
    t/db_dependent/Koha/Patron/Messages.t
    t/db_dependent/Koha/Patrons.t
    t/db_dependent/Koha/SMS_Providers.t
    t/db_dependent/Koha_template_plugin_Branches.t
    t/db_dependent/Letters.t
    t/db_dependent/Members/AddEnrolmentFeeIfNeeded.t
    t/db_dependent/Members/GetUpcomingMembershipExpires.t
    t/db_dependent/Members_Attributes.t
    t/db_dependent/Patron/Borrower_Debarments.t
    t/db_dependent/Patron/Borrower_Discharge.t
    t/db_dependent/Patron/Borrower_Files.t
    t/db_dependent/Ratings.t
    t/db_dependent/Reports_Guided.t
    t/db_dependent/Reserves/GetReserveFee.t
    t/db_dependent/Review.t
    t/db_dependent/Serials_2.t
    t/db_dependent/ShelfBrowser.t
    t/db_dependent/Virtualshelves.t
    t/db_dependent/api/v1/patrons.t

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 15263: (QA followup) Use the new XSLTParse4Display everywhere

Edit: fixed catalogue/detail.pl and opac/opac-detail.pl so they use the right
XSLT syspref.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 15263: (QA followup) Make *shelves.pl use the new API

This patch makes the lists work as the search results for rendering on
XSLT-driven context. No behaviour change is expected.

To test:
- Apply the patch
- Navigate lists (OPAC and intranet)
=> SUCCESS: the only difference is speed (faster)
- Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 15263: Bug 15263: (follow-up 2) XSLT display fetches sysprefs for every result processed

Don't retrieve prefs if we won't need them

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 15263: (follow-up) XSLT display fetches sysprefs for every result processed

Set variables ($sysxml, $xslfilename, $lang) if they are not passed to
the subroutine. This happens from catalogue/detail.pl,
opac/opac-shelves.pl, opac/opac-tags.pl and virtualshelves/shelves.pl.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 15263: XSLT display fetches sysprefs for every result processed

On search, every single result goes through some XSLT processing.
This includes fetching the relevant sysprefs every single time.
We should do it only once per search.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16016: Sitemap handling scripts for packages

This patch introduces the koha-sitemap script. This script wraps calls to
the misc/cronjobs/sitemap.pl script so it can be done easily instance-wise.

It sets /var/lib/koha/${instance}/sitemap as the destination directory for
the sitemap files. A followup will make them available through an Apache
configuration entry.

koha-functions.sh is provided with a handy is_sitemap_enabled function so
we can later add filters to other commands (koha-list, koha-foreach, etc).

Exposes sitemap files to apache. This is suitable for including the sitemap in
the robots.txt file as proposed in

  http://www.sitemaps.org/protocol.html#submit_robots

Note: it depends on Apache 2.4+ so we can patch the file as needed. Older
Apache users will be warned that they need to tweak the apache files on their
own.

To test:
- You can build a package out of this patchset, or do the following on a kohadevbox
  cp debian/scripts/koha-functions.sh /usr/share/koha/bin
- Run
  $ debian/scripts/koha-sitemap --help
- Go through all the options (--enable, --disable, generate).
(a) --enable:
  - debian/scripts/koha-sitemap --enable kohadev
=> SUCCESS: /var/lib/koha/kohadev/sitemap.enabled is created
  - call it again, a suitable warning is raised and the file is still there
(b) debian/scripts/koha-sitemap --disable kohadev
=> SUCCESS: /var/lib/koha/kohadev/sitemap.enabled is deleted
  - call it again, a suitable warning is raised and the file does not exist
(c) --generate:
  - debian/scripts/koha-sitemap --generate kohadev
=> SUCCESS: sitemapindex.xml and sitemap000X.xml files are generated in
            /var/lib/koha/kohadev/sitemap/
- Sign off :-D

Sponsored-by: Orex Digital
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 15564 - Fix tranfert branch: Set collection branch with return branch if it is empty.

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 15564 - Display "print slip" option when returning an item which is in a rotating collection

To Test -
1. Make a new rotating collection (under tools) with at least one book in it.
2. Check out the book/s from rotating collection.
3. Set library to a different library.
4. Check in book you just checked out.
5. Should show a dialog box with no print slip link or city.
6. Apply patch and repeat steps 1-4. Dialog box should now show the item name, city name and print slip link.

Patch works as expected. (Superfluous white space removed on line 136)
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 16383: Making Local Use sysprefs actions buttons

To test:
1) Go to Admin -> System Preferences -> Local Use tab
2) Confirm buttons show as font awesome buttons and work as expected
3) Confirm buttons do not wrap on a narrower browser

Sponsored-by: Catalyst IT
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>