Bug 18066: Hea V2 [squashed]

This patch is the Koha part of the Hea v2 project.
You can find the (testing) code for the server at
  hea-ws  - https://github.com/joubu/hea-ws/commits/v2
  hea-app - https://github.com/joubu/hea-app/commits/v2
They contain the different pull requests made over the last 6 months.

More information on Hea at https://wiki.koha-community.org/wiki/KohaUsageStat_RFC
The goal of this commit message is to provide an overview of what could
be a new version of Hea.

Prior to these changes, the Hea database was filled with 1 line per Koha
installation. System preferences were filled by the libraries and a
cronjob (share_usage_with_koha_community.pl) collected these values to send
them to a webservice (hea-ws/upload.pl).

With the need to collect more data we would want to collect data at the library
level (branch) and not at the installation level.
For instance the geolocation, the url or the country can be different from one
library to another, even if managed from the same Koha installation.
The Hea DB has been upgraded to reflect that change (see hea-app/sql/schema.sql).

The hidden goal of this patch is to make Hea sexier and explain
better to libraries how it can be useful to share their information
with the Koha community. I guess the main problem is the lack of
communication and explanations about what we are doing we these data.
To fill this gap I'd like to (TODO)
  1. Communicate on the ML about this new version of Hea (once it got
pushed and backported)
  2. Link the Privacy_Policy.md from the Hea interface
  3. Get help from a native English speaker to add
popup/help/info/whatever on "Home › Administration › Usage statistics",
to clearly explain what happens (and what will not happen!) when an option or
another is set.

You can find screenshot of this whole enhancement on bug 18066, comment 2.

What this patch does:
- Create a new branches.geolocation DB field
- Add 3 new sysprefs:
  * UsageStatsGeolocation
  * UsageStatsLibrariesInfo
  * UsageStatsPublicID
- Integrate the Leaflet JS library to get a fancy map to pick
geolocations

How does it works:
On the new administration page where statistics to share are configured,
there are several new things. It is now possible to share information either
per Koha installation or libraries. If UsageStatsLibrariesInfo is set,
the info at library level (url, name, country, geolocation) will be
sent to the Hea webservice. If it is not set, you can decide to fill
UsageStatsLibraryUrl, UsageStatsLibraryName, UsageStatsCountry,
UsageStatsGeolocation to share these information. Note that even if the
data are retrieved at installation level, it's better to fill the prefs
as well: On the Hea website the different libraries defined for a given
Koha installation could be displayed on the same page.
This page is a public page which will be attributed to every
installation (with the pref UsageStatsPublicID). On this page all the
info available publicly will be displayed.

TODO later:
- Add a button on the administration page to delete the info shared
publicly. It will be easy to show that the info are no longer displayed
on the public page.
- Add an icon per Koha installation to get a better "public page"
- Any suggestions?

Test plan:
We will need to test hea-ws, hea-app and the Koha-side code to test the
whole enhancement.
1/ To start, clone the hea-ws and hea-app project and checkout the
'master' branch (*not* 'v2')
2/ Create the hea database and user
  CREATE DATABASE hea
  CREATE USER 'hea'@'localhost' IDENTIFIED BY 'hea';
  GRANT ALL PRIVILEGES ON hea.* TO 'hea'@'localhost';
  FLUSH PRIVILEGES;
3/ Fill the DB with some data
  mysql hea < hea-app/sql/schema.sql
  mysql hea < hea-app/sql/sql/mock-data.sql
4/ Checkout the 'v2' branch for both hea-ws and hea-app
5/ Execute the upgrade DB script
  % cd hea-app
  % perl -p -i -e 's/REPLACE_ME/hea/' sql/upgrade.pl # Fill the DB info
  % perl sql/upgrade.pl
Now the DB is using the v2 structure. That means we have 1 installation
row per library previously defined. 1 library row has also been created.
5/ Configure hea-ws
% echo '192.168.50.1 hea.koha-community.org' >> /etc/hosts
<VirtualHost *:80>
  DocumentRoot "/path/to/hea-ws"
  ServerName "hea.koha-community.org"
  <Directory "/">
    Options +ExecCGI
    Require all granted
    AddHandler cgi-script .pl
  </Directory>
</VirtualHost>

And enable it with a2ensite, then restart apache.
The copy the database.yml.sample to database.yml and edit it to fill the
DB info.

6/ Launch the hea-app
  % cd hea-app
  % edit README.md # to install the missing modules
  % cp environments/config.yml environments/development.yml
  % edit environments/development.yml # to fill the DB info
  % perl bin/app.pl
Then hit localhost:3000
You should see a local version of Hea with sample data

7/ Back to Koha side
A. We will test that the webservice still works with previous version of Koha (without v2)
a. Do not configure Hea
  % perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
Then hit localhost:3000
=> Nothing added
b. Configure Hea on admin/usage_statistics.pl
perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
=> New library added
c. Modify the Hea configuration
perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
=> Info are modified

B. Not we will test that it works with the new version (much more fun ;))
% git checkout hea-v2 # koha
a. Configure Hea using /admin/usage_statistics.pl
perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
=> Check the result on localhost:3000
b. Share libraries's info
perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
c. Continue to play a bit and share the info.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 14608 - HEA : add possibility of sharing usage statistics [squashed]

This patch set adds:
- a reference to Hea at the end of the installation process
- a link to the new page from the admin home page
- a new page to easily configure shared statistics

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Update release notes to 16.05.12 release.

Bug 18329 - Batch record deletion broken

Hie, Tools > Batch record deletion seems broken.
Any deletion returns error :
Bibliographic record YYY was not deleted. An error occurred. (The error was: {UNKNOWN}: DBD::mysql::db begin_work failed: Already in a transaction at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1560. at /home/koha/src/C4/Biblio.pm line 3468 , see the Koha log file for more information).

Looks like it is because of Bug 18242 which added a transaction  in C4::Biblio::_koha_delete_biblio_metadata : $schema->txn_do.
The script batch_delete_records created a transaction with $dbh->{AutoCommit} = 0;

This patch fixes by using also Koha::Schema in batch_delete_records to manage transaction.
It also removes "$dbh->{RaiseError} = 1", this behavior is managed in Koha::Database.

Test plan :
 - Go to Staff interface : Tools > Batch record deletion
 - Enter a few existing biblionumbers
 - Click on "Continue"
 - Click on "Delete selected records"
=> Without patch you get a DB error
=> With patch you get confirmation message
 - Try to get the biblios to confirm they are deleted : /cgi-bin/koha/catalogue/detail.pl?biblionumber=xxx
 - Test with and without Plack

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18242: 16.05 version - 16.11.x adaptation - Old::Checkouts vs OldIssues

Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18242: 16.05 version - [SOLUTION 2]Handle correctly move to old_issues

The table old_issues has a primary key defined on the issue_id column.
This issue_id comes from the issues table when an item is checked in.

In some case the value of issue_id already exists in the table

Basically this happens when an item is returned and mysqld is restarted:
The auto increment value for issues.issue_id will be reset to
MAX(issue_id)+1 (which is the value of the last entry of old_issues).
See also the description of bug 18003 for more informations.

In this solution the change is done at code level instead of DB
structure: If old_issues.issue_id already exists before moving from
the issues table, the issue_id is updated (not on cascade for
accountlines.issue_id, should it?) before the move.

Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18242: 16.05 version - [SOLUTION 2]Add tests

Signed-off-by: Mason James <mtj@kohaaloha.com>

Revert "Bug 18329 - Batch record deletion broken"

This reverts commit f6cac0592397ae829e34a576173fd0c86980d1bc.

Fix t/db_dependent/HoldsQueue.t tests

FIX CSRF for opac-memberentry.pl

Bug 18094: Only search in searchable patron attributes if searching in standard fields

Test plan:
- Add a new patron attrbute and mark it searchable
- Populate a new patron with 'potato' in that field
- Add/edit another patron to have email potato@invalidemail.com'
- Perform a patron search with query 'potato' (in standard fields)
=> Both patrons are returned
- Perform a patron search with filters 'Email' and query 'potato'
=> Only 1 patron is returned and you are redirected to the patron detail page.

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: Mason James <mtj@kohaaloha.com>

Bug 18094: Add tests to highlight the problem

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>

revert buggy CSRF in opac/opac-memberentry.pl

Revert "Bug 18307 - Branchname is no longer displayed in subscription tab view"

This reverts commit 719dc345f4d38b1e34ef2318f472e4757709a647.

Bug 17146: Fix CSRF in picture-upload.pl

If an attacker can get an authenticated Koha user to visit their page
with the
url below, they can change or delete patrons' images
/tools/picture-upload.pl?op=Delete&borrowernumber=42

Test plan:
1/ Hit /tools/picture-upload.pl?op=Delete&borrowernumber=42
And confirm that you get a "Wrong CSRF token" error
2/ Go on the patron detail page with a patron's image
3/ Click on the Delete link (note the csrf_token param)
4/ The image will be deleted and you are redirected to the patron detail
page.

Regression tests:
Upload an image from the patron detail page and from the "upload patron
images" tool.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 17116: Fix CSRF in import_borrowers.pl

If an attacker can get an authenticated Koha user to visit their page
with the url below, they can change patrons' information

The exploit can be simulated triggering
  /tools/import_borrowers.pl?uploadborrowers=42

In that case it won't do anything wrong, but it you POST a valid file,
it could.

Test plan:
Trigger the url above
=> Without this patch, you will the result page
=> With this patch, you will get the "Wrong CSRF token" error.

Regression test:
Import a valid file from the import patron form, everything should go
fine.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 17109: [QA Follow-up] Die when wrong token

Removes template var csrf_error and associated handling.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Restested with opac and intranet: Still sends or dies elegantly..

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Bug 17109: Use Koha.Preference in sendbasket template

No need to send OPACBaseURL to the template, if you load the Koha TT
plugin inside the template.

Test plan:
Send a few items in your cart from OPAC and intranet.

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

Bug 17109: Add CSRF token to [opac-]sendbasket

If you have no (valid) token, you will not be able to send the message.

Test plan:
[1] Verify if you can still send the cart from opac and intranet.
[2] While still being logged in, try to send the cart from opac by
    using the following URL:
    /cgi-bin/koha/opac-sendbasket.pl?email_add=you@somedomain.com&comment=csrf_test&bib_list=doesnotmatter&csrf_token=justsomeguess12345
    This should now result in a csrf error.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 17109: Remove second authentication from (opac-)sendbasket

Patch deals with opac and intranet variant.
If we authenticated the first time, it is not necessary to do it
a second time rightaway.

Replaces a call to get_template_and_user (including checkauth) by
gettemplate.

Also removes duplicate use C4::Biblio statements.

Test plan:
[1] Put a few books in the cart.
[2] Send the cart from OPAC.
[3] Send the cart from intranet.

Tested 3 patches 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: Mason James <mtj@kohaaloha.com>

Bug 18124: add to members/deletemem.pl

Bug 17097: [QA Follow-up] Exit after redirect

Adds one exit statement, and some whitespace.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Verified deleting a patron again.
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 17097: here the var is 'member', not 'borrowernumber'

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 17097: Fix CSRF in deletemem.pl

If an attacker can get an authenticated Koha user to visit their page
with the url below, they can delete patrons details.

  /members/deletemem.pl?member=42

Test plan:

0/ Do not apply any patches
1/ Adapt and hit the url above
=> The patron will be deleted without confirmation
2/ Apply first patch
3/ Hit the url
=> you will get a confirmation page
4/ Hit /members/deletemem.pl?member=42&delete_confirmed=1
=> The patron will be deleted without confirmation
5/ Apply the second patch (this one)
6/ Hit /members/deletemem.pl?member=42&delete_confirmed=1
=> you will get a crash "Wrong CSRF token" (no need to stylish)
7/ Delete a patron from the detail page and confirm the deletion
=> you will be redirected to the patron module home page and the patron
has been deleted

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 17097: Add a confirmation page when deleting a patron

It won't hurt to have a confirmation page when deleting a patron.
Moreover it's the more easy way to protect against CSRF attacks :)

Test plan:
Make sure you get a confirmation page when deleting a patron
Confirm that approving or denying the confirmation work as expected

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

t/Token.t merge typo fix
modified:   t/Token.t

Bug 18124: [16.05.x] remove HouseboundModule code

modified:   members/memberentry.pl

Bug 16993: Fix CSRF in memberentry.pl

If an attacker can get an authenticated Koha user to visit their page
with the url below, they can change patrons' passwords or other
patrons'details

members/memberentry.pl?op=save&destination=circ&borrowernumber=3435&password=ZZZ&password2=ZZZ&nodouble=1

Test plan:

Trigger
members/memberentry.pl?op=save&destination=circ&borrowernumber=42&password=ZZZ&password2=ZZZ&nodouble=1

=> Without this patch, the password will be updated
=> With this patch applied you will get a crash "Wrong CSRF token" (no
need to stylish)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended: removed the commented use Digest::MD5-line.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 17110: Add unit test for MaxAge parameter in Token.t

Test plan:
Run t/Token.t

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 17110: Lower CSRF expiry in Koha::Token

Default expiry in WWW:CSRF is one week.
This patch sets it to 8 hours by default in Koha, and allows to
change the expiry period individually by passing MaxAge.

Test plan:
[1] Put items in your cart.
[2] Apply the example patch too.
[3] Send the cart from opac within the allotted 10 seconds.
[4] Send again, but wait some 10 seconds before submitting. Too late!

Tested 3 patches 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: Mason James <mtj@kohaaloha.com>

Add release notes to 16.05.12 release

Translation updates for Koha 16.05.12

Revert "Bug 18094: Only search in searchable patron attributes if searching in standard fields"

This reverts commit b696300c3a2ddbd670775fe22a0cb95b61751abf.

Revert "Bug 18094: Add tests to highlight the problem"

This reverts commit dde4036df4fba9b8d3652a78c320c42f9e71c660.

fix t/db_dependent/HoldsQueue.t tests

Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18312: Fix export unless a file is supplied

Bug 18087 breaks export unless a file is supplied.

Can't use an undefined value as a HASH reference at
/home/vagrant/kohaclone/tools/export.pl line 75.

Test plan:
Export records using a file of id that is not a valid file (not txt or
csv)
Export records using a valid file
Export records without supplying a file

=> The export should work or fail as expected.

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18087: Handle invalid filetypes

If an invalid file is used as biblionumber list, we should display a
message.

Test plan:
1/ Use csv, plain text files
=> Should work
2/ Use invalid files (binary files like pdf, doc*, xsl*, etc.)
=> Should not work and see a warning message.

Amended patch after signoff: Remove one warn debug line

Signed-off-by: Joy Nelson <joy@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18087 - Clarification on File type when using file of biblionumbers to export data

Added a line to the screen  detailing the types of files that can be used to upload a list of biblionumbers.

Test plan:
1. Go to tools->export data
2. under File option,  (File types accepted: .csv and .txt)  should appear
3. Check both Bibliographic and Authority Export screens for this new string.

Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Signed-off-by: Joy Nelson <joy@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 15451: Better error handling

1/ If a librarian edit (add_validate) a non-existing csv profile, we
explicitely die
2/ If you try to delete a non-existing csv profile, you will now get a
nice alert box

Bug 15451: (followup) fix filename extension for csv file

This patch does the same as basket/downloadcart.pl
to set '.csv' as filename extension for downloadshelf.pl

To test:
1) Define a CSV MARC profile
2) On staff download a list, extension is '.NN'
with 'NN' the CSV profile id.
3) Apply the patch
4) Download again, check extension is now '.csv'

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Bug 15451: Do not considered a Resultset as a Result

The previous calls were wrong, but there is something bad with the DB
structure: export_format.profile should be a unique key.
This patch fixes the previous calls and add a FIXME not to forget to fix
the DB structure.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Previous test where done with all patches applied,
including this one, and all worked.
No errors

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Bug 15451: Koha::CsvProfiles - Remove the residue

This patch erase all traces of C4::Csv since it's not used anymore.
All occurrences have been replaced by previous patches to use
Koha::CsvProfiles.

Note that GetMarcFieldsForCsv was not used prior this patch set.

Test plan:
  git grep 'C4::Csv'
should not return any result.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
No more traces of the file.
This produces a koha-qa fail, due to the missing file.
No other errors

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Bug 15451: Koha::CsvProfiles - Remove GetCsvProfile

This subroutine just returned a csv profile for a given id.
It is replaced in this patch by a call to Koha::CsvProfiles->find.

There is nothing to test here, these changes have been tested in
previous patches.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Bug 15451: Koha::CsvProfiles - Remove GetCsvProfileId

This subroutine returned the export_format_id for a given profile name.
This can be done easily with the Koha::CsvProfiles->search method.

Test plan:
Export records using the misc/export_records.pl script and the
export tool.
If you are exporting using the MARC format, the profile filled in the pref
ExportWithCsvProfile will be used (or the one passed in parameter of
misc/export_records.pl).
If you are exporting using the CSV format, you can choose a profile in
the dropdown list.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Exported using tool & cmd, marc & csv. Pref is used.
No errors

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 15451: Koha::CsvProfiles - Remove GetCsvProfiles

This subroutine did the same job as GetCsvProfilesLoop, so this patch
applies the same changes as the previous patch.

Test plan:
1/ Claim some serials, sql profiles should be listed
2/ Export records using the export tool. MARC profiles should be listed.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Listed sql & marc profiles
No errors

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 15451: Koha::CsvProfiles - Remove GetCsvProfilesLoop

This subroutine returned the csv profiles for a given type.
This could be done easily with the new Koha::CsvProfiles->search method.

Test plan:
To do at the OPAC and staff interface!
1/ Export a list using a CSV profile
2/ Export your CART using a CSV profile
Note that only MARC profiles should be available.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Tested on staff/opac & cart/list
Small problem on filename extension fixed in followup.
No errors

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 15451: Rewrite the csv profile tool script

This page was inconsistent with the other admin and tool pages.
The 2 tabs 'New profile' and 'Edit existing profile' were useless, the
ergonomic needs to be revisited.
This patch applies the same script/page structure as others: by default
a table containing all csv profiles is displayed with 2 action links:
edit and delete.

Test plan:
1/ Create 1+ CSV profiles, with different types (marc and sql)
2/ Update some values using the Edit link
3/ Delete a CSV profile

Note: When deleting a CSV profile, it would be great to warn the user if it is
used.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
No problems on create, update and delete.
No errors

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 15451: Add the 2 new modules Koha::CsvProfile[s]

There are based on Koha::Objets. Tests provided.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

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: Mason James <mtj@kohaaloha.com>

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: Mason James <mtj@kohaaloha.com>

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: Mason James <mtj@kohaaloha.com>

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: Mason James <mtj@kohaaloha.com>

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: Mason James <mtj@kohaaloha.com>

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>

Bug 18423 - Add 'Add child' button to files, housebound, and deletemem pages

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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>

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>

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: Mason James <mtj@kohaaloha.com>

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: Mason James <mtj@kohaaloha.com>

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: Mason James <mtj@kohaaloha.com>

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: Mason James <mtj@kohaaloha.com>

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: Mason James <mtj@kohaaloha.com>

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: Mason James <mtj@kohaaloha.com>

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: Mason James <mtj@kohaaloha.com>

Bug 18001 - Unit Test

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

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: Mason James <mtj@kohaaloha.com>

Bug 17821 - due date in intranet search results should use TT date plugin

Intranet search results displays due date from item onloan.
This should use the TT date plugin.

Test plan :
- set syspref dateformat not on yyyy-mm-dd, for example dd/mm/yyyy
- checkout an item
- at intranet, perform a search where you see the item
=> You must see : "date due : dd/mm/yyyy"

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 12930: Web installer does not show login errors

Implemented invalid_username_or_password template
variable being handed to Auth.tt. Removed indentation
changes and modification to return value of checkpw

Test plan:
See comment #2

Alternative test plan:
- Clear session cookies or close / open your browser
- Go to  [MY SERVER]/cgi-bin/koha/installer/install.pl
- Verify that message appears if wrong username/pw provided
- Verify that you can log in with databas administrator account

Tested with alternative test plan, works as expected. Title and
description in commit message adapted.
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>
(cherry picked from commit 87eb80136d49451d0a9cc25e68db452a33f1771a)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18373: Re-add UpgradeBackup.pm

Bug 18028 removed the install_misc directory but
install_misc/UpgradeBackup.pm was still used by the 'upgrade' rule of
make.

Other files from install_misc were useless to it may be better not to
reintroduce this directory with only 1 file.

Test plan:
`make`
`sudo make install`
`make upgrade`

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 17872: Fix small error in GetBudgetHierarchy and one of its calls

In aqbudgetperiods.pl a commented line is removed that contains a
wrong userenv hash key. Should be branch instead of branchcode.

In aqbudgets GetBudgetHierarchy is called with the same wrong userenv
hash key. Should be userenv->{branch}.
This made another bug visible: if you call GetBudgetHierarchy with a
branch and without owner, the where clause should take into account
that the branchcode can be empty (not null).

Test plan:
[1] Run Budgets.t
[2] Run aqbudgets.pl from Administration
[3] Add the show_mine=1 parameter in the URL
[4] Change owner of one of the funds and repeat step 2 and 3.

t/Budgets.t returns green. Followed test plan, 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: Mason James <mtj@kohaaloha.com>

Bug 14932: Do not call can_edit_subscription with an empty value

ref($sub) eq 'ARRAY' should always be true, what we want is to call
can_edit_subscription if there is at least 1 serial for this
subscription.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 14932 - serials/serials-collection.pl-page is very slow. GetFullSubscription* checks permission for each serial!

Currently we have subscriptions with 300+ serials received.
It takes a lot of time to show serials-collection.pl (~20s).
This is especially troublesome when receiving serials, since after receival we get redirected to that page.

We no longer can receive daily serials in the allotted timeframe.

This quick and dirty fix prevents checking the subscription editing for each serial, but instead checks it for the first serial only.

This reduced page load time by ~18s

TEST PLAN:
1. Receive ~300 serials (or just a bunch :)  )
2. Observe the gradual slowing of the receival action.

AFTER THIS PATCH:
1. Receive ~300 serials more (or just a bunch)
2. Observe a significant performance improvement.

Signed-off-by: Paul POULAIN <paul.poulain@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18349: [QA Follow-up] Add four missing error messages

The error codes come from CanBookBeIssued.
The warnings speak for themselves. Note that the GNA message is similar
to the one used in opac-user.tt (gonenoaddress is translated to the patron
as contact information not up-to-date).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18349: Remove useless info in alert message

Empty tags should be removed, and message id
Note that not all error codes are covered here.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18349: If a confirmation is required, consider as blocker

Only RENEW_ISSUE should not be considered as blocker, others should.
Note that this code is not robust and the whole script as well as the
return of CanBookBeIssued should be rewrittent completely.
We need a small patch to easy backport to stable releases

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18349: $borrower is a hashref

I guess this code was never called before...

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18349: SCO - Do not trust the confirmed flag

The "confirmed" flag is used to know if that user confirmed a situation
that needs a confirmation.
But if the issue/renew is impossible the CanBookBeIssued and the
'impossible flags' should be checked.

Otherwise a patron can checkout and renew bypassing the circulation
rules (Understand 'no limit' here...)

Test plan:
Want to renew?
Checkin $barcode, then
/cgi-bin/koha/sco/sco-main.pl?patronid=$cardnumber&barcode=$barcode&confirmed=1&op=checkout

Want to bypass the checkin?
Same url...

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 85bd15a83ffdab0c2e28eae54e50ce4dee9e608b)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Conflicts:
opac/sco/sco-main.pl

Bug 17309 - Renewing and HomeOrHoldingBranch syspref

The AddRenewal subroutine currently uses the circulation rules for the branch
stored in the Issues table (which is the holding branch) when calculating the new due date.

This patch replaces using the branch from the Issues table with the branch
specified by the HomeOrHoldingBranch syspref.

To test:
1. Set up 2 branches, Branch1 and Branch2
2. Set up a loan rule in Branch1 for DVDs with a 21 day loan period and a
   21 day renewal period.
3. Set up a loan rule in Branch2 for DVDs with a 14 day loan period and a
   14 day renewal period.
4. Checkout a DVD belonging to Branch1 while logged into Branch2. It will
   receive the correct 21 day loan period.
5. Renewing the same DVD while logged into either Branch1 or Branch2
   will give a 14 day due date, rather than 21 days.
6. Checkout a DVD belonging to Branch2 while logged into Branch1. It will
   receive the correct 14 day loan period.
7. Renewing the same DVD while logged into either Branch1 or Branch2
   will give a 21 day due date, rather than 14 days.
8. Apply the patch and repeat steps 4-7. The correct due date should be given
   when the item is renewed, regardless of where it is checked out or renewed.

This update removes reassignment of $branch variable.

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 7d35bdf58a0bcebab20df47b50b249cafe12ddb6)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18329 - Batch record deletion broken

Hie, Tools > Batch record deletion seems broken.
Any deletion returns error :
Bibliographic record YYY was not deleted. An error occurred. (The error was: {UNKNOWN}: DBD::mysql::db begin_work failed: Already in a transaction at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1560. at /home/koha/src/C4/Biblio.pm line 3468 , see the Koha log file for more information).

Looks like it is because of Bug 18242 which added a transaction  in C4::Biblio::_koha_delete_biblio_metadata : $schema->txn_do.
The script batch_delete_records created a transaction with $dbh->{AutoCommit} = 0;

This patch fixes by using also Koha::Schema in batch_delete_records to manage transaction.
It also removes "$dbh->{RaiseError} = 1", this behavior is managed in Koha::Database.

Test plan :
 - Go to Staff interface : Tools > Batch record deletion
 - Enter a few existing biblionumbers
 - Click on "Continue"
 - Click on "Delete selected records"
=> Without patch you get a DB error
=> With patch you get confirmation message
 - Try to get the biblios to confirm they are deleted : /cgi-bin/koha/catalogue/detail.pl?biblionumber=xxx
 - Test with and without Plack

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 0f32bd99997c82cf7b70eef32818dc817cbff64b)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18266: Fix internal error when paying fine for lost item without.. item

If a fine is created for a lost item but the itemnumber is not supplied,
the system will return it.
The item should not be mark as returned if there is no item linked to
the fine.

Test plan:
1. Turn StoreLastBorrower on
2. Create a manual invoice for a lost item, do not supply a barcode
3. Pay the fines 'Pay fines > Pay'

=> Without this patch applied you get
Can't call method "last_returned_by" on an undefined value at
/home/marc/koha/C4/Circulation.pm line 2188.

=> With this patch applied, you must not get the error.

Bug 17346: Make checkin column hidable

To test:
1) Go to columns_settings.pl --> Circulation --> issues-table and note that you cannot change the settings for checkin.
2) Go to columns_settings.pl --> Patrons --> issues-table and note that you cannot change the settings for checkin.
3) Apply the patch.
4) Go to both tables again and note that you can now change the settings.

Followed test plan, 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>
(cherry picked from commit 9098a4d3fa7268bf11ab108693192070c44c9eaf)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 17945 - Breadcrumbs broken on opac-serial-issues.pl

==TEST PLAN==
1) Make a subscription with a record
2) Search and view the record in OPAC
3) Click on the subscription tab and click more details
4) The breadcrumb will say 'Detals for' without the record name
5) Apply patch
6) Refresh the page
7) The breadcrumb will display the name of the record

(fixed small error)

Signed-off-by: Sonia BOUIS <sonia.bouis@univ-lyon3.fr>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 39bcba540a6c00fc374284dc025297a568befed6)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18263: Make use of syspref 'CurrencyFormat' for Account and Pay fines tables

On Home > Patrons > Accounts for... and Home > Patrons > Pay fines for...,
make the amounts display as defined in syspref 'CurrencyFormat'

To test:
- Apply patch
- Give a patron a high fine or credit (e.g. 12345.67)
- Got to pages Home > Patrons > Accounts for... and
  Home > Patrons > Pay fines
- Verify that amounts display following syspref 'CurrencyFormat'
  (e.g. 12'345.67 for syspref set to 360'000.00 (CH)

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>
(cherry picked from commit db467287d174827e52c322cb86d75b8628df52fe)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Conflicts:
koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt

Bug 17290 - Standardize on "Patron Categories"

We need to standardize on terminology, this changes all instances of
patron types to be patron categories.  To text, just view the patch

Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 9a4fd7b27074d7ce923f7b47061d1e6636ecbc56)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18307 - Branchname is no longer displayed in subscription tab view

This patch restores the branchname in OPAC subscription detail view.
Additionally, it adds the subscriptons count to the tab title to make
it consistent with the other tab titles.

To test:
- Search a subscription in OPAC catalog
- Go to Subscriptions tab (see screenshot in initial comment)
- Verify that library name does not display
- Apply patch
- Verify that library name displays properly in Subscriptons tab
- Verify that the tab title displays the subscriptions count
  e.g.: Subscriptions (2)

Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit c6463c258f644ea5a1983680814eb765f1801283)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Conflicts:
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt

Bug 18209: Add default, 'Actions' and fix case

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit f14de0007c47ae66d0276186df073c0375d37610)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18209 - Patron's card manage.pl page is not fully translatable

The headers of the tables displayed in patroncards/manage.tt are hardcoded in the .pl and thus cannot be translated.

This simple fix reuse the translate_card_element tool already in the code to make the strings translatable.

Applied patch and verified that table headers on manage.pl display the same as before.
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>
(cherry picked from commit bb1492e9f67439753bf428bd06fa710791e70c74)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18019: [QA Follow-up] Also catch the delete from authorities detail

Good catch from Jonathan. See comment11.
Authorities detail should pass a CSRF token to authorities-home when
deleting a record without linked biblios.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Passing the token with GET is not a good way to do, but nothing quick to
replace that.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit d2ee53fb5f90277d47fc56667e71018f5d9b88a9)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18019: Add CSRF protection to authorities-home.pl (op==delete)

Without this patch, it is possible to delete authority records with URL
manipulation.
Like: /cgi-bin/koha/authorities/authorities-home.pl?op=delete&authid=[XXX]

Test plan:
[1] Go to Authorities. Search for some authorities (without links).
[2] Delete an authority. Should work.
[3] Apply patch.
[4] Construct an URL like above to delete another authority. Should fail.
    Under Plack this results in an internal server error, the log tells
    you: Wrong CSRF token.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Amended the test plan.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 5a7dc0749f581e4c4bc6ec68d3f3ab6bac12afd5)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 12972 - Transfer slip and transfer message (blue box) can conflict

Instead of passing the homebranch to the slip, let's pass
TransferWaitingAt (the value we display)

To test:
1 - Check in an item that will initiate a transfer (hold or automatic
        item return)
2 - Note the transfer message and receipt match
3 - Remove the reason for transfer (bot don't cancel transfer)
    i.e. cancel hold or change homebranch of item to current library
4 - Check the item in again
5 - Note the message displays the transfer destination and the slip
shows the homebranch
6 - Apply patch
7 - Repeat 5 but note slip and message match
8 - Test any edge cases for correctness
9 - sign off

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>
(cherry picked from commit f454013ec9beb8f6e4c382253b76bfac5ca65244)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 7728: [QA Follow-up] Fix POD whitespace

Resolves complaints from qa tools.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 53f2b56fa15fac2f0d2ef0777e95f436d6914ec7)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Conflicts:
C4/Serials.pm

Bug 7728: QA fixes

- Remove useless comments
- Use Koha::Objects::find instead of Koha::Objects::search

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit c26afc2568311a2b7a99f33705cbc35b715563a8)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 7728: Adding unit test ReNewSubscription.t

Test plan:
 * run without the patch, the test will fail
 * run with the patch, the test will pass

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit d79a264b6991d48bda026fc0ccde0607b519423f)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 7728: Fix subs history end date consistency

Without this patch, there is an inconsistency in subscriptions:
 * a new subscription has no history end date set (as expected)
 * if you renew it, a date is set.

This patch removes the setting of the history end date during renewal

Test plan:
 * create a subscription, go to subscription-detail.pl page, tab "Summary". No History end date is set
 * renew the subscription => a date has been set
 * apply the patch
 * create another subscription
 * renew it => no history end date has been set
 * sign-off

Signed-off-by: beroud <isabelle.beroud@univ-lyon3.fr>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 1843ee148b25a904d8baffbbe8f0ea504b539a64)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18244: Patron card creator does not take in account fields with underscore (B_address etc.)

Fields with underscore like B_address do not print on patron cards.

To reproduce:
- Create patron card layout using fields with underscore in their name
  (e.g. <surname><B_address> )
- Print (export) patron card
- Verify that fields without underscore are replaced by their value,
  but fields with underscore do not replace but show the field name

To test:
- Apply patch
- Try to reproduce and verify that fields with underscore are replace
  as expected

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit a723de2ae0c393743fc111955ee81b5e6c5b7f4a)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 14535: Update the supplier filter too

The combobox on the left side of late orders is filled by sub
GetBooksellersWithLateOrders. The same change as in the first patch
must be made here to include suppliers with late orders without a
price.

Bonus: Sort the list.

Test plan:
[1] Run t/db_dependent/Bookseller.t.
[2] Go to late orders. Use the filter on suppliers.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit b76eac9ca2c32a5db51b9b8ca32a2cf4fa6397b5)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 14535: Late orders does not show orders with price = 0

This patch removes the filter of unitpricesupplier <> 0 and
unitpricelib <> 0 from GetLateOrders as used in lateorders.pl.
This allows you to claim late orders without a price entered.

Test plan:
[1] Run t/db_dependent/Acquisition.t
[2] Check if you see an order with no price in lateorders.pl. (If needed,
    remove prices from a few orders.)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 71f267fd3f3750510e3df5f2819d57ed45dcf919)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 12913: [QA Follow-up] Reword 'change item status' and add zero status

The zero status should be added in @notforloans. It is a legitimate status
for available books. The zero status description can be translated in
the template.
The 'Change item status' is confusing. We mean that we saw a notforloan
status that we do not expect (read: does not exist in the table).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 664d6534171677455f3cf1467e8726146714133a)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 12913 - wrong inventory results when notforloan filter is not used

In inventory tool, there is a test on scanned items notforloan value : if the value is not one of authorized values, the inventory results displays the problem "Change item status".
The bug is that if notforloan filter is not used, the notforloan values array is not filled and so all scanned items are marked with this problem.

Test plan :
- Check you have in default framework an item subfield mapped with items.notforloan and with an authorized value catergory, for example 'NOTFORLOAN'
- Check the authorized value category has at least value '0'
- Create a new value in this category, for example '9'
- Create a biblio with default framework
- Create an item with barcode='000AAA1', callnumber='ZZZAAA1' and notforloan=0
- Create an item with barcode='000AAA2', callnumber='ZZZAAA2' and notforloan=9
- Delete new value created above, for example '9'
- Create a file 'barecodes.txt' containing 2 lines '000AAA1' and '000AAA2'
- Go to inventory tool : /cgi-bin/koha/tools/inventory.pl
- Choose file in 'Barcode file'
- Enter item callnumber between 'ZZZ' and 'ZZZZ'
- Check 'Compare barcodes list to results'
- Submit
=> without patch, you get 2 results with problem 'Change item status' even for item with notforloan=0
=> with patch, you get 1 result with 'Change item status' on item with notforloan=9

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 358fb744c74cc63b7d2ae7d7e406ada205502ea8)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 17758 - SIP checkin does not handle holds correctly

This patch updates SIP checkin to use reserve_id as param, to fix regression
introduced by Bug 14695.

To test:

Given 1 patron and 1 biblio with two items attached:

1) create two record level holds on one biblio on same user
Using SIP client or telnet:
2) checkin item x on pickup branch, observe item is waiting and hold 1 is filled
3) checkin item y on pickup branch, observe item y now is waiting and x is available
4) apply this patch and repeat 1-3, now expected behaviour should be
   that both holds are filled and both items waiting

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 72de37e24bd02ffccad3cd1a76e0d74202cf7794)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>