Bug 15498: Fix show/hide csv profile list

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 15498: Do not display sql csv profiles

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 15498: Add ExportCircHistory and remove ExportWithCsvProfile

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 15498: Let the user choose the CSV profile to export circ history

The way the export options are displayed at the bottom of the checkouts table
was not consistent.
Prior to this patch set, they are display if ExportRemoveFields or
ExportWithCsvProfile is set.
It does not make any sense, the user could want to export the checkouts in
iso2709 format without having to define a csv profile and fill the pref.

Moreover the behavior of this pref did not match its description: it's used as
a default CSV profile when exporting records from the export tools or the
command line.

This patch set adds a new pref ExportCircHistory and remove
ExportWithCsvProfile. The new pref is set if ExportWithCsvProfile or
ExportRemoveFields were set.
A new dropdown list with the CSV profile list will be displayed in the
export area, at the bottom of the checkouts table.

Note that now --csv_profile_id is mandatory for the export command line
(misc/export_records.pl) if the export format is csv.

Test plan:
0/ Do not execute the DB entry
1/ Clear both ExportWithCsvProfile and ExportRemoveFields prefs
2/ Execute the DB entry
3/ ExportCircHistory should not be set and the export options should not
be displayed at the bottom of the checkouts table.
4/ Remove the pref
  DELETE FROM systempreferences WHERE variable='ExportCircHistory';
and reinsert the previous one, with a value:
  INSERT INTO systempreferences (variable, value) VALUES
  ('ExportWithCsvProfile', 'something');
Execute the DB entry again
=> The now pref should be now set
5/ Export some checkouts using the CSV entry
6/ Note that the export tool and commandline script still work using the
csv format. You have to provide a --csv_profile_id option to make it
work.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 17971 (QA Followup) Clarify comment

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

Bug 17971: Add support for objects represented by fk

For instance an issue is not fetch from its fk but using the fk
itemnumber.
We need to support them.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 17971: TT syntax for notices - Add support for plurals

On of the awesome things we will be able to do with the TT syntax is the support of plurals.

For instance we will be able to send a list of items, checkouts, etc. to the notice template.
That way we will get rid of our custom syntax like <<items.content>> or <item></item> for instance.

The existing code already has the playground for that but it is not used.

Basically the idea is to add a "loops" key which can contain a list of
object to retrieve from the DB and send to the template.
For instance:
    loops => { overdues => [ $itemnumber_1, .., $itemnumber_N ] }

will send a variable "overdues" to the template. It will contain the
Koha::Checkout objects relative to the id passed.

There is one quite big inconvenient to this approach so far: since we
are still supporting the historical syntax, the objects can be fetch by
a script, then the script will send the id to GetPreparedLetter which
will refetch them.
This must be improved, but I suggest to do that later.

Test plan:
  prove t/db_dependent/Letters/TemplateToolkit.t
should return green

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 17970: Fix GetPreparedLetter behavior if nothing to substitute

From C4::Letters::GetPreparedLetter:

    my $tables = $params{tables};
    my $substitute = $params{substitute};

    $tables || $substitute || $repeat
       or carp( "ERROR: nothing to substitute - both 'tables' and 'substitute' are empty" ),
          return;

So if the parameter tables or substitute is passed but does not contain anything, it will not warn as expected.

Test plan:
1/ Apply the patch with tests
2/ Confirm that they do not pass
3/ Apply this patch
4/ Confirm that the tests now pass

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>

Bug 17970: Add tests to highlight the problem

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>

Bug 17963: TT syntax for notices - Prove that AR_* are compatible

Nothing new here since bug 17962, the AR_* notice messages are quite
simple. They send the article_request, patron, biblio, biblioitem, item and
library linked to the article request.

All the fields from these 6 tables should still be accessible using the
TT syntax.

Test plan:
Define TT notice templates for AR_PENDING, AR_PROCESSING, AR_COMPLETED
or AR_CANCELED.

You should manage to create a template to generate the same result as
the historical syntax.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 18269: Move field mappings related code to Koha::FieldMapping[s]

The 3 subroutines GetFieldMapping, SetFieldMapping and
DeleteFieldMapping from the C4::Biblio module were only used from the
field mappings admin page.
They can easily replaced with new packages Koha::FieldMappings based on
Koha::Object[s]

Test plan:
Add and delete field mappings (admin/fieldmapping.pl, Home ›
Administration › Keyword to MARC mapping).
Add an existing mapping > Nothing should be added

Note that this page has not been rewritten and you will not get any
feedbacks, but it's not the goal of this page to improve 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>

Bug 18269: Add Koha::FieldMapping[s] packages

Tested both patches together, 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>

Bug 14146 - DBRev 16.12.00.017

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

Bug 14146: Add the new pref CumulativeRestrictionPeriods

Sponsored-by: Orex Digital
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 14146: Clean a bit and make the code understandable

The code was a bit weird and this patch cleans it a bit by renaming
variables and adding a variable.

Sponsored-by: Orex Digital
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 14146: Add the ability to cumulate restriction periods

At the moment the default behaviour is not to cumulate the restriction
periods but to apply the longest one.
This patch set creates a new syspref CumulativeRestrictionPeriods. If
on, the behaviour changes and the restriction periods are cumulated: the
days of the second restriction are added to the actual restriction
period.

We could add a new circulation rule instead of a syspref, but I am not
sure it's very useful to have such granularity for this behaviour (can
be changed if needed).

How it works:
Let's take 2 items, A and B.
A is returned with Na days late, and B Nb days late
The grace period is Ng and there is 1 day of suspension charge per day
of overdue
The suspension period is until day D = Na - Ng + Nb - Ng

I would have expected D = Na + Nb - Ng but it's how it worked before
this patch.

Test plan:
Create several overdue for a given patron
Do the checkins and confirm that the period are added if the pref is on.
If the pref is off, you should not get any changes in the existing behaviour.

Sponsored-by: Orex Digital
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 14146: Add tests for AddReturn + CumulativeRestrictionPeriods

Sponsored-by: Orex Digital
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 17847: Remove C4::Koha::GetAuthvalueDropbox

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 17847: Replace C4::Koha::GetAuthvalueDropbox with Koha::AuthorisedValues

The C4::Koha::GetAuthvalueDropbox subroutine does the same job as
Koha::AuthorisedValues->search
We should then replace the different calls to this subroutine to finally
remove it.
There were 2 calls to this subroutine:
- from the AuthorisedValues TT plugin (called from av-build-dropbox.inc
and members/housebound.tt)
- from the acqui/ajax-getauthvaluedropbox.pl ajax script

To make sure that this patchset does not introduce regressions, we will have
to test that the TT plugin and the ajax script still behave as before.

Test plan:
1/ Test acqui/ajax-getauthvaluedropbox.pl
- Link a fund to an authorised value category
- Create a new order
=> When you select a fund linked to AV category, the sort1 (and/or
sort2, depending on what you set) should be replaced with a dropdown
list populated with the authorised values
2/ Test av-build-dropbox.inc
- Create some authorised values for Bsort1
- Edit a patron
=> The sort1 should be a dropdown list populated with the Bsort1 AV
3/ Test members/housebound.tt
- Enable the housebound module (pref HouseboundModule)
- On the patron detail page, click on the "Housebound" tab
=> The frequency dropdown list should be populated with the different
HSBND_FREQ AV

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 17844: Replace C4::Koha::get_notforloan_label_of with Koha::AuthorisedValues

This patch is more a bugfix than a refactoring.
Indeed the C4::Koha::get_notforloan_label_of behaviors were buggy:
1/ It does not display the opac description at the OPAC, but always the
staff description
2/ It does not care of the framework of the biblio, but retrieve the
first row of the marc_subfield_structure mapped with items.notforloan

These 2 bugs can easily be fixed using the
Koha::AuthorisedValues->search_by_koha_field

Steps to recreate the issues:
- Create 2 authorised value categories for not for loan (NFL1 and NFL2)
with the same values. Define a different description for the OPAC.
- Define link 952$7 to NFL1 for the default framework and to NFL2 for
the BK framework
- Create 2 bibliographic records (B1 using NFL1 and B2 using NFL2) with
2 items each (1 item should have a not for loan value)
- Go to the "Place a hold" view for this record.
- In the item list, you should see the not for loan value
=> The staff description of NFL1 will always be used, even for the OPAC

Test plan:
- Recreate the issues without this patchset
- Apply this patchset
- Recreate the steps to recreate the issues
=> The staff description of NFL2 should be displayed for the B2 item
=> The opac description of NFL2 should be displayed for the B2 item at
the OPAC

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 17844: Remove C4::Koha::get_notforloan_label_of

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 18258: (QA followup) Use Koha::Subscriptions

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

Bug 18258: Add the Koha::Biblio->subscriptions method

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

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 18270: Do not fetch the MARC record when deleting an item

$record is never used later, the call is superfluous.

Test plan:
Quick glance at the code should be enough

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

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>

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.

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>

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>

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>

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>

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>

Bug 18144: Restore pieces of C4/Auth to make Google OpenID Connect work

By restoring some pieces of logic, with the name changed from $persona
to $emailaddress, the openid will work again for OPAC logins.

See https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10988#c68
for an excellent test plan.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Did not test it, but trust in author and signoffer

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

Bug 13757 - DBRev 16.12.00.016

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

Bug 13757: (QA followup) Filter out non-editable params before storing

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

Bug 13757: (QA followup) Exclude empty attributes from rendering if non-editable

In self registration opac displayable (and not editable) attributes are
displayed as empty. This an empty value is passed to the template for
creating an empty input and it shouldn't when the attribute is not
editable.

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

Bug 13757: Attribute with value 0 should be stored

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

Bug 13757: (regression test) Attribute with value 0 should be stored

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

Bug 13757: Better display for attr changes in members-update.pl

This patch changes the way changed attributes are displayed for the
staff user to make the decision to approve (or not) the changes.

Regards

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

Bug 13757: Make K::P::Modifications->pending return K::P::Attribute

This patch makes Koha::Patron::Modifications->pending return
Koha::Patron:Attribute objects. They are not stored on the DB but only
live in memory on runtime.

members-update.pl is the only place this is used, and this way we have
all we need for better rendering the UI.

Tests are added for the changed API.

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

Bug 13757: (QA followup) Fix non-editable attrs on failed save

When a field is not editable but displayable in the OPAC, and you submit
an incomplete/wrong update, those attributes are displayed as empty.

This patch fixes that.

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

Bug 13757: (QA followup) Make opac-memberentry.pl handle attrs deletion

The original code on this bug skipped empty-valued attributes. But
emptying attributes is the only way to tell the controller script that
the user wants to delete them.

This patch makes opac-memberentry.pl check the existence of attributes
sharing the code of the empty for the given patron, and it stores the
deletion on the Koha::Patron::Modification as needed. Otherwise
deletions got skipped.

To test:
- Verify setting/deleting attributes that are opac-editable and verify
the results are sound.

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

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

Bug 13757: Make Koha::Patron::Modification->store del empty attrs

This patch makes Koha::Patron::Modification->store delete the passed
attributes that contain empty values.

This is the way it currently works, as all opac-editable attributes are
presented to the end-user and they are allowed to delete them, and the
best way I found to represent the deletion on the modification request
is by setting it to the empty string. I chose this way because it is how
the staff interface handles it, so it is consistent.

To test:
- Apply this patch
- Run:
  $ prove t/db_dependent/Koha/Patron/Modifications.t
=> SUCCESS: This time tests pass!
- Verify comment #70 on the bug is fixed now
- Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
https://bugs.koha-community.org/show_bug.cgi?id=13737

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

Bug 13757: (regression tests) Empty attributes should delete existing

This patch introduces tests for the required functionality.

To test:
- Run:
  $ prove t/db_dependent/Koha/Patron/Modifications.t
=> FAIL: The current code doesn't work like that

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
https://bugs.koha-community.org/show_bug.cgi?id=13737

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

Bug 13757: (QA followup) Check DB structure before altering table

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

Bug 13757: (followup) Fix authorized value display when opac_display & not opac_editable

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

Bug 13757: (followup) Regression tests for ->approve changes

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

Bug 13757: (followup) Only touch opac_editable attributes

As reported by Owen, the members-update.pl was showing every attributes
the patron has (display issue) instead of showing only those affected by
the changes.

This patch fixes this by filtering the patron's attributes by opac
editability.

It also fixes Koha::Patron::Modification->approve so it only clears the
attributes with the updating 'code' and leaves the others untouched.
As its been coded so far (until someone refactors it all) the
Koha::Patron::Modification object needs to contain all the attributes
for a specific code. And it comes from parsing the UI's input.

Tests for Koha::Patron::Modification->approve to come.

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

Bug 13757: (followup) Staff interface changes

This patch adds proper extended attributes display and handling on the
patron modifications moderation page (members-update.pl).

It also adds changes checking to the opac-memberentry.pl page so it
only saves a modification request if there are changes (it only checked
regular fields and not the extended ones).

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

Bug 13757: (followup) Remove warnings

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

Bug 13757: Add extended attributes to the patron modification

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

Bug 13757: DBIC update

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

Bug 13757: OPAC changes

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

Bug 13757: DB update

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

Bug 13757: Add the option to set patron attributes editable in the OPAC

Note: this is a squashed version of the original patchset, because it was needed

This patch adds an opac_editable property of borrower attribute types
that can be set in the interface. I'm removing work on OPAC and will
refactor it, keeping the author attribution.

Test plan:
  1. Repeat the following with a new and existing borrower attribute
     type:
  2. Verify that "Editable in OPAC" can only be checked if "Display in
     OPAC" is checked.
  3. Verify that this new property is correctly saved.

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

Bug 17792: Add opac_editable and opac_display methods

This patch adds two methods to the Koha::Patron::Attribute:

- opac_display
- opac_editable

Both method just check the corresponding Koha::Patron::Attribute::Type
and return the values for those attributes. This is useful to avoid
checking that manually on the controller scripts.

To test:
- Run:
  $ prove t/db_dependent/Koha/Patron/Attributes.t
=> SUCCESS: Tests pass!
- Sign off :-D

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

Bug 17792: Introduce Koha::Patron::Attribute(s)

This patch introduces stub Koha::Object(s) for handling patron attributes.

Edit: amended the POD to fix C&p mistake

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

Bug 17755: (followup) Override ->search to allow filtering by branchcode

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

Bug 17755: (QA followup) Return $self when appropriate

As failure situations raise exceptions that should be handled outside
the object class, methods should return $self so successive calls can be
chained nicely.

This patch makes methods return $self and adjusts the tests to reflect
this change.

Make sure tests pass:
- Run:
  $ prove t/db_dependent/Koha/Patron/Attribute/Types.t
=> SUCCESS: Tests return green
- Sign off :-D

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

Bug 17755: Introduce Koha::Patron::Attribute::Type(s)

This patch introduces the Koha::Object-based classes for handling
patron attribute types.

It also adds branch limitation handling to the
Koha::Patron::Attribute::Type class.

It is built on top of the new Koha::Object::Library::Limit class
that extends Koha::Object so it handles library limits.

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

Bug 17755: Unit tests

This patch introduces unit tests for Koha::Object::Library::Limit. It is
done this way because it needs to be instantiated to be usable.

To test:
- Run:
  $ prove t/db_dependent/Koha/Patron/Attribute/Types.t
=> SUCCESS: Tests pass
- Sign off :-D

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

Bug 17755: Introduce Koha::Object::Limit::Library

This patch introduces a new class for extending Koha::Object using
multiple inheritance. It cannot be used standalone, it needs to be
used in Koha::Object implementations like this:

use base qw( Koha::Object Koha::Object::Limit::Library );

Its goal is to provide a single way and place to deal with this common
pattern in Koha's codebase.

As it happened with Koha::Object, that needed to be tested in a real object
class, this work was done on top of Koha::Patron::Attribute::Type implementation
and it is fully covered by the tests that are introduced for it.

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

Bug 18066: Update Schema

This patch basically reverts 85a21dd9715518746dd93defd4803a81d23a5b18

Bug 18066: Add the new geolocation column to kohastructure.sql

Fix updatedatabase.pl syntax error

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>

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>

Bug 17605: [MASTER] 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>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>

Updating the Schema

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

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>

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>

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>

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>

DBREV for Bug 18066 - Hea - Version 2

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

Bug 18066: Fix map positioning in admin/usage_statistics.pl

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>

Bug 18066: Fix tests

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>

Bug 18066: Use https for ws

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>

Bug 18066: Do not use token for OSM tiles

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>

Bug 18066: Add the leaflet library

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>

Bug 18066: Hea V2

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>

Bug 14608: Move country list to an include file

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>

Bug 14608: Follow up on labels and search box

This addresses concerns raised in comment #26 and comment #27.

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>

Bug 14608: Add a reference to Hea at the end of the installer process

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>

Bug 14608: Add a link from the admin home page

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>

Bug 14608: Add a page to configure shared statistics

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>

Update DB Schema

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

DBREV for Bug 8010 - Search history can be added to the wrong patron

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

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>

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>

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>

Bug 18215: Third case which was missed.

Git install old-school style a pre-tls koha
Drop the DB
Create the DB
change branch to master
Run a web install.

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

Bug 18215: Fix another tls warning

Resolve warning:
Use of uninitialized value in string eq at C4/Installer.pm line 79.

Test plan:
Add a trivial .sql file in atomicupdate (comment is sufficient).
Run updatedatabase.pl
Without the patch you have the warning.
Apply the patch and repeat (no warning).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>

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>

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>

Bug 17169 - Use CCODE descriptions instead of codes

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

Bug 17169: (QA followup) Add NORMARC and UNIMARC definitions for CCODE

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>

Bug 17169: Add a facet for ccode fields to ElasticSearch

To test:
1 - Apply patch
2 - Backup your db
3 - Drop and create a new db to ensure your mappings are refreshed from
the patch
4 - add some titles with items with collection codes
5 - search and see collection code facets
6 - sign off

Work to be done:
1 - Replace codes with descriptions

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>

Bug 18256: Koha::Items - Remove GetItemsCount

C4::Items::GetItemsCount can be replaced with Koha::Biblio->items->count

Test plan:
Create a bibliographic record with items attached
Try to delete the record from a basket (acquisition module), the detail
page and the batch item deletion tool.

=> You should not be able to delete it.

Remove the items and then try again to delete the record

=> Now you must be able to delete 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: Brendan A Gallagher <brendan@bywatersolutions.com>

Bug 18264 - (QA Followup) Remove use of GetBiblioFromItemNumber Replace <blank> with (blank)

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

Bug 18264 - Course reserves - use itemnumber for editing existing reserve items

This patch alters the crouse reserves module to use itemnumber as the
lookup method for existing reserves as opposed to barcode

To test:
1 - Apply patch
2 - Add some items to a course reserves - all should work as expected
3 - Delete the barcode for an item on reserve
4 - Note the edit link still resolves to the correct item

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>

BUG 8010: Removed Auth2.t

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>

BUG 8010: Correct a few syntax errors, and moved tests

- Tests are now in t/db_dependent/Search/History.t
- There were 2 differents sysprefs in sysprefs.sql and in atomicupdate => fixed

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>