Bug 18001 - LocalHoldsPriority can cause multiple holds queue lines for same hold request

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

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

Bug 18001 - Unit Test

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

Bug 18372 - transits are not created at check in despite user responsing Yes to the prompt

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

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

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

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

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

Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 07156cf8fd0f11eb43a11ffa7f75d4c95f3766df)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18321: Correctly calculate switch_onsite_checkout

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

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

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

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit f04c5d5a2c549bed54036b726965ccda083def7b)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18321: Add tests

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

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 1c7d283aae0952877b871f486c31cd3f04529222)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

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

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: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 508147d24443498a2c12a7c55240e70941c49dd6)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

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: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 476179a12311b32643087cc8ec10c31409bb1b99)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

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: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 17f4b05d23cb19cebeedb361005bf411a5f25c60)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

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: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 6940aa91a53371ba49528022954a9a349044ab63)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

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: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 392641ff1ec0cf2b6480af134fd676edf64f4df7)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

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

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

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: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 6b75e5b4783cff1d14816ce363e8095569d67718)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

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>

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 18242: 16.11.x adaptation - Old::Checkouts vs OldIssues

Bug 18242: [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: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit 7aa0808ff227850fc448b74dabed9b15ac0eb927)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18242: [SOLUTION 2]Add tests

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit 48b1328ebeb7ed2337ba75a582705389767b691a)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

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>

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>

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>

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>

Bug 17995 - HOLDPLACED notice should have access to the reserves table

To test:
1 - Add reserves.reservenotes to HOLDPLACED message
2 - Enable emailLibrarianWhenHoldIsPlaced OpacHoldNotes sysprefs
3 - Place a hold via OPAC with a note
4 - view the messagequeue and note the reservenotes is blank
5 - Apply patch
6 - Place a hold with a note
7 - view the messagequeue and note the reservenotes is populated

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: Brendan A Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit 3b794b1312d0cfac81b302d0e823e2c5683a1f0e)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18022: Fix possible crash if CircAutoPrintQuickSlip=clear screen

If CircAutoPrintQuickSlip is set to clear screen, the $borrowernumber variable
is reset but $borrower.
In 16.11.x and before that causes the app to crash, because
$borrower->{flags} is set to a hashref by GetMemberDetails, if
$borrowernumber is set.

This case is better handled in master (17.05) but side-effects could
happen: if $borrowernumber is reset, $borrower should be reset too.

The way to drive this behaviour is terrible and lot of things should be
done to clean this area. This patch is a quick and easy fix to make it
backportable easily.

Test plan:
On master, no change expected
On 16.11 and before:
0. Set CircAutoPrintQuickSlip=clear screen
1. Create a new patron
2. Set permission for this patron to 'reservesforothers' (or something
else).
3. Go on the circulation tab
4. Do not fill the barcode input and submit
=> Without this patch you will get an error
Can't use string ("64") as a HASH ref while "strict refs"....
=> With this patch apply you will get a blank screen (expected
behaviour).

I would recommend to test this patch with the other value of
CircAutoPrintQuickSlip as well

Bug 18058: Allow borrower_message_preferences to be truncated

borrower_message_preferences cannot be truncated because of the foreign.
DBMS fails with
  "Cannot truncate a table referenced in a foreign key constraint"

To avoid that we should remove the FK check and truncate the other table
as well.

I am wondering if we really need a truncate here
  DELETE FROM borrower_message_preferences;
should do the job, but leave it as it because of the param name.

Test plan
  perl misc/maintenance/borrowers-force-messaging-defaults --doit --truncate
Should no longer raise the error message

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

Bug 18124: Change the calls to generate and check CSRF tokens

The parameter change in Koha::Token should be applied to the calling
scripts.

Test plan:
Confirm that the different forms of the scripts modified by this patch
still work correctly.

Test the problematic behavior:
Open 2 tabs with in same user's session, go on the edit patron page
(memberentry.pl).
Log out and log in from the other tab.
Submit the form
=> Wrong CSRF token should be raised

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

Bug 18124: [Follow-up] Handle default parameters in a sub

Adds a internal routine to handle default values for the parameters
id and secret.
Also adds a parameter session_id for generate_csrf and check_csrf. This
session parameter is combined with the id parameter when generating or
checking a token.

Test plan:
Run t/Token.t

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

Bug 18124: Restrict CSRF token to user's session

Currently the CSRF token generated is based on the borrowernumber, and
is valid across user's session.
We need to restrict the CSRF token to the current session.

With this patch the CSRF token is generated concatenating the id
(borrowernumber) and the CGISESSID cookie.

Test plan:
Run t/Token.t

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

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: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 8dad1582c100017f8ad3e331c9a9b9cc9ed4e4d6)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Add release notes to 16.11.06 release

Merge remote-tracking branch 'transl/16.11.06-translate-20170327' into 16.11.x

Increment version for 16.11.06 release

Translation updates for Koha 16.11.06

Bug 15854: Use a READ and WRITE LOCK on message_queue

To make sure we will not never get a race conditions for these kinds of
notices, we need to add a LOCK on the message_queue table.

This does not smell the best way to do that, but I faced deadlock issues
when I tried to use "UPDATE FOR"

https://dev.mysql.com/doc/refman/5.7/en/innodb-locking-reads.html
https://dev.mysql.com/doc/refman/5.7/en/lock-tables.html
https://dev.mysql.com/doc/refman/5.7/en/commit.html

To test this patch, or another solution, you need to apply manually this
change:

         my $message = C4::Message->find_last_message($borrower, $type, $mtt);
         unless ( $message ) {
+            sleep(1);
             C4::Message->enqueue($letter, $borrower, $mtt);
         } else {

And repeat the test plan from first patch.
Do not forget to truncate the message_queue table.

Followed test plans, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit be156d9ad9e5bcfadab34d44f90e04fd61e256ad)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 15854: Simplify the code to limit race conditions

There is an obvious race condition when CHECKIN and RENEWAL are
generated from circulation.pl calling svc/renew or svc/checkin in AJAX.

The 2 first queries will try to get the id of the last message
(find_last_message) and if it does not exist, they will insert it.
Theorically that could be lead to have several "digest" messages for a
given patron.
I did not recreate more than 2 messages, from the third one at least one
of the two firsts existed in the DB already.

This patch just simplifies the code to make the SELECT and INSERT or
UPDATE closer and limit the race condition possibilities.

Test plan:
0. Set RenewalSendNotice and circ rules to have a lot of renewals available
1. Use batch checkouts (or one by one) to check out several items to a
patron
2. Empty message_queue (at least of this patron)
3. Renew them all at once ("select all" link, "renew or check in"
button)
4. Check the message_queue
Without this patch you have lot of chances to faced a race condition and
get at least 2 messages for the same patron. This is not expected, we
expect 1 digest with all the messages.
With this patch apply you have lot of chances not to face it, but it's
not 100% safe as we do not use a mechanism to lock the table at the DBMS
level.

Tested both patches together, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit 607b14516a955c9989e4764c69527edbc1f36ba0)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 17605: [STABLE] Add currency to orders generated by quotes

Sets the vendors currency in the edi generated order
The currency used is agreed between vendor and library
and the value in the quote is optional

Edifact potentially allows the currency to be overwitten
by specifying another currency in the CUX segment but
we know currently of no supplier doing this

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Bug 18010: Remove potential exposure from gettemplate

A similar bad template check from C4::Auth::get_template_and_user
should be applied in C4::Templates::gettemplate.

Before this patch it would be possible to expose files like:
my $template = C4::Templates::gettemplate(
    '/etc/passwd', 'intranet', CGI::new, 1
);
print $template->output;

Note that the is_plugin flag in the above call is the culprit. This patch
provides a quick security fix without touching get_template_and_user, and
can be backported to stable branches.
I will provide an enhanced and centralized check on report 17989, also
removing the is_plugin flag.

Note: We allow .pref here too for use in admin/preferences.pl.

Test plan:
[1] Run t/db_dependent/Auth.t (triggering get_template_and_user and
    gettemplate).
[2] Run t/db_dependent/Templates.t again (see first test plan).
    The tests should no longer fail.
[3] Open a page on opac or intranet.
[4] Open a systempreferences tab.
[5] Add a book to the cart and send it ([opac-]sendbasket uses gettemplate).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit fbbd4b40f0d71b79194fc186e66f985488be26c6)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18010: Unit test for gettemplate

A trivial test, similar to the ones in Auth.t.
Without the check in gettemplate (added in the second patch), the passwd
file will be exposed and the test fails.

Test plan:
Run t/db_dependent/Templates.t without second patch. The two tests in the
last subtest should fail.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit 5508897d5a2b73c30d50fed26a6bfed62bf49398)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Conflicts:
t/db_dependent/Templates.t

Bug 18028: Remove outdated install_misc directory

This directory is no longer maintained and contain outdated information
on how to install Koha.
On the Internet there are tutos using them and that can create confusion
to new user (yes it could be confusing as well to execute a file that
does no longer exist).

NOTE: last tweak was mid 2016, but before that 2015. So I agree with
      the deletion.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit a6f50cbcb92c2575eb31e906ea54bf0386654906)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18069: Remove residue of rebuild_zebra -x

Bug 17731 removed the -x option of rebuild_zebra but koha-rebuild-zebra
still cals the script with this option.

"Warning: You passed -x which is already the default and is now deprecated"

Test plan:
sudo koha-rebuild-zebra -f
should no longer raise the warning

Signed-off-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit fe898bc641b6d76d7b869654dce4ec21e219daf8)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

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: Brendan A Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit 5823d4ca6a71404d502eeb7f1bf5dd152fd98e22)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

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: Brendan A Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit f86500351af0a1a4d1593eb2aa689b8ec52bc401)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18246: Patron card creator: Unit not always displays properly in layouts

To reproduce:

1) Create a new layout
2) Select an unit in "Units selection"
3) Save
4) Edit layout again
5) Select other unit, e.g. 'SI Centimeter'
6) Save
7) Repeat
8) Edit again
Error: Latest unit seems to stick and can no longer be changed, because
in units drop down, multiple untis are marked as selected.

To test:
1) Apply patch
2) Repeat steps above, verify that value for units always display as expected.

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit 37da40b323d8bc012724510fe4839dcb908c2b3b)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 8603: Patron card creator - 'Barcode Type' doesn't stick in layouts

To reproduce:

1. Make a new layout
2. Tick "Print card number as barcode"
3. Change 'Barcode Type' to anything other than Code39
4. Save
5. Edit it again
6. Its back to Code39

To test:
1. Apply patch
2. Repeat steps above
3. Verify that 'Barcode type' is selected as expected

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
https://bugs.koha-community.org/show_bug.cgi?id=18063

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

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

Bug 18150 - CanItemBeReserved doesn't work with (IndependentBranches AND ! canreservefromotherbranches)

The variable $item used to be a hash, but at the end of the function,
it's a Koha object. As $item->{homebranch} doesn't yield anything and
should be $item->homebranch. It prevents people using different
branches without holds between branches from placing a hold on an item
they should be able to place hold on.

Test plan:

1. Before patch

  a. with IndependantBranches off
  b. try to place hold on an item you should be able to place hold on
  c. it should work
  d. put IndependantBranches on and canreservefromotherbranches off
  e. shouldn't work

2. after patches redo steps from (1) and everything should be working
fine.

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Note: Item is fetched twice, it's not necessary. But out of the scope of
this patch.

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit f79cf7930d887ea157fa64bb89fb0eb130b74aae)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18305 - jquery.fixFloat.js breaks advanced MARC editor

Remove use of ECMASCRIPT 2015 default parameter syntax in
jquery.fixFloat.js to be compatible
with a wider range of browser versions.

Test plan:
  1. Usa a not so modern browser, for example Chrome/Chromium < 49, any version of IE
  besides Edge, Opera, or Firefox < 15.0.
  2. Edit a bibliographic record in staff client and try switching to
  advanced MARC editor (check console, no errors should appear).

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit a649958315c98500ebe0ea3f91c05a8067d54e58)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Add release notes to 16.11.04 security release

Merge remote-tracking branch 'transl/16.11.05-translate-20170316' into 16.11.x

Translation updates for Koha 16.11.05

Increment version for 16.11.05 release

Bug 18275: Do not rely on CGI param userid to log a user in if auth is not required

From opac/opac-memberentry.pl, authnotrequired is set.
That means a patron can access the page without being logged in. It is
used on this page for the self registration feature.

From C4::Auth::get_template_and_user, we have
  $userid = $q_userid;
$q_userid is previously set to the 'userid' CGI param.

We end up here if authonotrequired is set AND CGISESSID does not exist.

Test plan:
- Run:
  $ prove t/db_dependent/Auth.t
=> FAIL: Regression test for checkauth fails
- Apply this patch
- Run:
  $ prove t/db_dependent/Auth.t
=> SUCCESS: Tests pass.
- Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 18275: Regression test

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 16202: GetIssuingCharges should return a collectable amount

Charges should not include elements less than a penny/cent
they are not displayed but can be saved to the database
causing "odd" behaviour down the line

Make the routine round the resultant charge to nearest cent,
so consistent values are returned.
Removed the one case where it was rounded post call.
Although the main danger is values generated by the discount
calculation apply the rounding to all returned charges in case
the item charge is defined using the 3rd or 4th decimal
places.

NOTE: prove -v t/db_dependent/Circulation.t triggers the change.
      Though, all the returned amounts are 0.00 only.

Signed-off-by: Mark Tompsett <mtompset@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 d84ed89557e2c921f261b1c620434d432dcbd3f7)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 7143: Replace Santallan by Santellan

Missed one typo in about.tt.
Can go along with the next patch here.

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

Bug 18135: Show alert if no authorities selected in batch record deletion

To test:
1) Go to Tools -> Batch record deletion
2) Select Authorities as record type, put in an auth id, click Continue
3) Unselect the authorities and click Delete selected records
4) Confirm an alert shows up
5) Clear the alert and confirm deleting the authority still works as
normal

Sponsored-by: Catalyst IT
Signed-off-by: Mark Tompsett <mtompset@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 7a5f17ff6ecbec3a7f73fa54b900dae909996b29)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18122 - Audio alerts: Add hint on where to enable sounds

This patch adds a hint to page Home > Administration > Audio alerts
and related help page about where to enable the sound system
(syspref 'AudioAlerts').

To test:
- Apply patch
- Disable syspref 'AudioAlerts'
- Go to Home > Administration > Audio alerts
- Note hint below title 'Audio alerts'
- Enable syspref 'AudioAlerts'
- Go to  Home > Administration > Audio alerts
- Hint is no longer visible (since audios are enabled)
- Open Help page (top right)
- Verify that hint on help page makes sense.

Amended to add alert class as requested by comment #2 2017-02-21 mv

Signed-off-by: Mark Tompsett <mtompset@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 b6cb18c4a8292cabe5fd611b349b51d5c79b5fe4)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 17840: adding classes to notes in checkouts

Test Plan
1) Add an internal and a public note to an item.
2) Check out the item.
3) Verify that both notes are shown in between title and barcode in the
second column of the details table.
4) Verify there is an empty class="" in the HTML source code for each
5) Apply patch and refresh page
6) View page source, confirm that class has been changed

I tested this by first applying the patch for bug 17919 to catch the
check-in software error. Patch working as expected :)

Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
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>
(cherry picked from commit 257c1793420108f1cb5efdec7bf03649d41940a0)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 13968: Follow up - Clean HTML tags for hints

Signed-off-by: Mark Tompsett <mtompset@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 e9d5615e70056cd887b9471827611a78275016ab)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 13968 - Branch email hints are misleading

This patch replaces missleading notes with Reply-To and Return-Path fields
to reflect indicate falback values if they are empty (ReplyToDefault and
ReturnpathDefault system prefs).

To test:
- Apply patch
- In Home > Administration > Libraties and Groups, edit a library
- Verify that hints display for fields Reply-To and Return-Path, indicating
  their default values.

Amended to remove extra </li> - 2017-02-21 mv

Signed-off-by: Mark Tompsett <mtompset@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 b2eb6109ff308de7d2bc9338d708ac7fe9b6e3d7)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

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

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: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 18af48bffe112a5df61a8a546da86dc31f32a7e7)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 17260: DBRev 16.11.04.001

Bug 17260: updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN columns

The loading of file admin/searchengine/elasticsearch/mappings.yaml
specifies 'type' as empty, which fails with Mysql 5.7+ which is more
stringent.

Also, forcing an empty value into a boolean also fails.

Both issues arise when updatedatabase.pl hit 3.23.00.050.

NOTE: both issues could also be resolved by actually setting values in
      the load file. This doesn't make this solution incorrect, though.

To Test/reproduce:
-1) Happens with Mysql 5.7.4.  Maybe earlier, but certainly at
    that point. Use a Xenial kohadevbox to more easily test.
0) Find a database on 3.22 or earlier, save it.
    a) place
           sql_mode=IGNORE_SPACE,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION
       into the [mysqld] section of /etc/mysql/mysql.conf.d/mysqld.cnf
    b) restart you mysql server
    c) drop the db and recreate it
    d) checkout the 3.22.x branch
    e) do a web install
    f) remove the added sql_mode
    g) restart the mysql server
1) Set your code base to master
2) run updatedatabase.pl
3) See the errors on 3.23.00.050
4) Apply the patch
5) Reload the 3.22 db.
   a) repeat steps 0(a)-0(g)
   b) don't forget the caching issues
6) succeed with updatedatabase.pl
7) drop the db and recreate it
8) run the web installer
9) notice no issues either.
10) run koha qa test tools

NOTE: This bug only solved the upgrade portion.
      I added the kohastructure.sql change as well.
      I confirmed that all the code changes were
      triggered with this test plan.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Mehdi Hamidi <mehdi.hamidi@inlibro.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 77337a2ab537843eab70cc06b8bc2531ad2014dc)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18248: Fix SQL statement to setting marc_subfield_structure.authorised_value to NULL

We want to update it if it's empty, not equal to ';'
nopaste vim mode is to blame

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

Bug 17947: Searching my library first shows the branchcode by the search bar rather than branchname

Test Plan
1: Turn on SearchMyLibraryFirst and go into OPAC
2: It should say Search (in <Branch Code> Only)
3: Apply patch and refresh page. It should now say Search (in <Library Name> only)

Sponsored-by: Catalyst IT
Signed-off-by: Mark Tompsett <mtompset@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 e8420b5d80558a8a74f7f6a1421352dcad2c9c88)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 17895: 'Re-set' typo fix

To test:
1) Log into OPAC and go to change your password
2) Enter current password incorrectly and click Submit changes
3) Observe error message and typo 're-set'
4) Apply patch and refresh page
5) May be asked to resend data - if so, click Resend, if not, repeat
step 2
6) Observe error message and corrected spelling of 'reset'

Sponsored-by: Catalyst IT
Signed-off-by: Caitlin Goodger <caitlingoodger.student@wegc.school.nz>
Signed-off-by: Mark Tompsett <mtompset@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 86ec263b2d0994ecb53ff283ef246b529d4570e2)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Add release notes for the 16.11.04 release

Fix broken ar-Arab staff po file

<failure message="msgfmt failure">misc/translator/po/ar-Arab-staff-prog.po:8952: 'msgstr' is not a valid C format string, unlike 'msgid'. Reason: The character that terminates the directive number 18 is not a valid conversion specifier.
 msgfmt: found 1 fatal error
 8098 translated messages.
</failure>

Translation updates for 16.11.04 release

Merge remote-tracking branch 'transl/16.11.04-translate-20170221' into 16.11.x

Translation updates for Koha 16.11.04

Increment version for 16.11.04 release

Bug 17852 Follow up.Multiple URLs (856) in list email are broken

This is a follow-up to my previous patch. This fixes it on the OPAC and
the previous one was from the intranet
Test Plan
1: Add multiple urls to a biblographic record
2: Add the biblographic record to a list and then email it from the OPAC
3: See that the links are broken and have been merged into one.
4: Apply patch
5: Send the list again and see that the links work

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

Bug 17852; Multiple URLs (856) in list email are broken.

Test Plan
1: Add multiple urls to a biblographic record
2: Add the biblographic record to a list and then email it.
3: See that the links are broken and have been merged into one.
4: Apply patch
5: Send the list again and see that the links work

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

Bug 17865 'If a subscription has no history end date, it shows as expired today in OPAC'

--TEST PLAN--
1) View a subscription with no history end date
2) Search for an item with a subcription in OPAC
3) Under subscription tab click more details
4) The end date will be the current date
5) Apply patch and refresh
6) The end date will not be shown

Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.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 f9a4f5edb84b826d9caf16b2221fed79fd7d421e)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18111: Swap framework_name and frameworkcode

With the frameworkcode changed to an empty string, which really
is closer to the physical reality, some javascript somewhere
breaks. This means the spinner does not appear when you confirm
that you wish to import it.

TEST PLAN
----------
1) apply first patch
2) export default framework, doesn't matter format
3) import default framework using that file.
4) answer yes or whatever it is.
   -- spinner does not appear.
5) apply this patch
6) import default framework use the same file.
7) answer yes or whatever it is.
   -- spinner does appear.
8) decide if you wish to find and fix the javascript conditions.
   if yes -- do so, and then obsolete this once you have.
   if not -- sign this off after running koha qa test tools

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 1ce61ac6a372783b39a11386018e08d92ccd8f39)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18111: Fix import of default framework

Caused by bug 16035.

Recreate the issue:
Go to Home › Administration › MARC frameworks
For the default framework: Actions > import
It will not work

Test plan:
Confirm test plans from bug 16035 and bug 17389 still pass
Confirm that the import for the default framework now works

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit c12aa9664ca71b38a0b570c71cbc0495ecfb9c0f)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 17913: [16.11.x] Add syspref to sysprefs.sql

See comment 69.

Bug 17461: Warn about plugins that can't be loaded

The logs should have some info if a module has been installed that can
not be loaded.

To test:
- Make sure you have an up to date dev install
- Install v1.0 of the KitchenSink plugin from
  https://github.com/bywatersolutions/koha-plugin-kitchen-sink/releases
- Make sure the plugin does not show up in the list of installed
  plugins, and that the intranet-error.log has no info about it
- Apply the patch
- Reload plugins-home.pl
- Verify that there is now some interesting info in intranet-error.log

Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Works as advertised, generates useful diagnostic information in the form of a log entry to either intranet-error or plack-error.log. Relief, thanks Magnus!
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 830786aba7084698c081a3f7745fe0fe54643c18)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 17950: Merge.t: Improve variable handling at top level

This patch does:
[1] Remove unused $dbh.
[2] Move all mocking statements to one sub.
[3] Promote a few lexicals to globals when used in the subtests.

Test plan:
Run t/db_dependent/Authorities/Merge.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 849877d8d3bcd3791c624a84ec5e4cb0550fa0fe)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 17950: Improve quick UNIMARC fix in Merge.t

Bug 17909 and 17913 added a quick fix for Merge.t on UNIMARC records.
This patch improves that fix with the sub compare_fields, a merge from
compare_field_count and compare_field_order.
Also it adds the option to test MARC21/UNIMARC by adding a command line
switch that triggers mocking the marcflavour preference.
The test on a cleared field 609 in strict mode has been broken up in two
tests: first a count without 609 and then counting 609s only.

Note: Could have mocked GetMarcBiblio too, but decided to go this way.

Test plan:
[1] Run perl t/db_dependent/Authorities/Merge.t
[2] (For UNIMARC users:) Run perl Merge.t -flavour MARC21
[3] (For others:) Run perl Merge.t -flavour UNIMARC

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 5d279befed1b6faf829ab0ebd19357dbfc7c576e)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>