Bug 24759: Rename renewalbranch to renewal_branchcode

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24759: Unit tests for AddRenewal

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24759: CleanUp OpacRenewalBranch values

We had a unique behvaiour where the syspref was set to string 'NULL'
as opposed to undef, we need to clean that up

To test:
1 - Set OpacRenewalBranch to 'NULL' in staff interface
2 - Renew via opac
3 - Check statistics to ensure branch is blank

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24759: Move OpacRenewalBranch code to Koha::Item

This patchset moves all code to calculate the correct renewal branch into Koha::Item.pm

When interface is opac we follow the syspref, otherwise we use the current userenv, or pass through
a defined branch

To test:
1 - Check out an item to a patron
2 - Set allowed renewals in the circ rules to 100 (just so you can keep testing)
3 - Renew the item in staff interface, confirm it is recorded correctly in statistics table (as signed in branch)
4 - Renew via the opac, testing with each setting of OpacRenewalbranch
5 - prove -v t/db_dependent/Koha/Item.t

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24696: (RM follow-up) Fix spaceing on command line

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24696: Use strftime

Seems easier to me.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24696: Add completion times to updatedatabase

This patch adds completion times to the print statements in
updateadatabase.pl.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24131: (RM follow-up) Final Rebase

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24131: Always display bug numbers on 5 characters

To correctly align bug numbers < 10000

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24131: Remove bullet points for secondary descriptions

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24131: (QA follow-up) Formatting in web client

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24131: (follow-up) Another rebase

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24131: (follow-up) Fix and rebase

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24131: Handle several descriptions

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24131: Move the print statement to a new subroutine

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24052: Add koha_xslt_security to koha-conf.xml

The line for enabling the unsafe expand_entities option is commented
and includes a warning to keep it like that ;)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24052: Catch warning between test 5 and 6

Resolve:
ok 5 - Engine returns error on undefined text
XSLT::Base: Empty String at /usr/share/koha/Koha/XSLT/Base.pm line 214.
ok 6 - Engine returns error on empty string

Test plan:
Run perl Base.t and verify that this warning is no longer printed.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24052: Fix Koha/XSLT/Base.t

Moving the separate small attachments into the test script.
Creating them now as temporary files or code fragments.
Deleting original files.

Since Breeding.t also used one of these files, a similar change was made in
that script.

Test plan:
Run t/db_dependent/Koha/XSLT/Base.t. Should pass now.
Run t/db_dependent/Breeding.t again.
Git grep on XSLT_Handler. You should only find release notes.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24052: Rename XSLT_Handler

Should be XSLT::Base now.
Removes old XSLT_Handler stub too (from bug 23290).
Result of a git grep | sed statement.

Test plan:
Run qa tools (so modules compile).
Run t/db_dependent/Breeding.t
Run t/db_dependent/Koha/XSLT/Base.t (This test fails when only this patch
has been applied; see subsequent patch.)
Enable XSLT use on results and details display. Check search results and
detail view on OPAC and staff.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 18414: Allow to use other select criteria with delete_patrons.pl

Adds the ability to specify a file with the --file flag that
should be a list of borrowernumbers for deletion.
If used without other flags it will delete the list of borrowers,
if used with other flags it will treat the other criteria
as filters for the list.

To Test:
1) Specify a list, e.g. borrowers.txt, of borrowernumbers in the database and put them in a text file with one per line
2) Invoke the script like ./delete_patrons.pl --file borrowers.txt to delete those patrons
3) Populate the list with a mix of borrowers in a category and those not, say categoryfoo
4) Invoke the scirpt as ./delete_patrons.pl --category_code categoryfoo --file borrowers.txt
5) Only the borrowers in the file that have the specified category will be deleted

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24872: Set languages prefererences after install

This patch set 'languages' and 'opaclanguages' system
preferences with the language used during install.

To test:
1. Apply the patch
2. Do a clean install in a language other than english
3. Verify that 'languages' and 'opaclanguages' system
preferences show the language code used during install,
besides english.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24672: (bug 24430 follow-up) Do not get active orders on deleted biblio record

On bug 24430 we replaced the call to CountBiblioInOrders with
Koha::Biblio->active_orders
In case the bibliographic record is deleted, the count must be set to 0.

This prevent the app to crash with:
Can't call method "active_orders" on an undefined value at
/home/vagrant/kohaclone/acqui/parcel.pl line 244

Test plan:
Create 2+ orders
Receive one, delete it, delete the bibliographic record
Receive the order one
=> Without this patch, on the parcel page you should get the error
=> With this patch applied you must be allowed to finish the receive

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

Bug 24874: Printing is broken on opac-results.pl page

to test:

- search for something in opac
- print page using file -> print
- observe printing is broken (pic1)

- apply patch

- search for something in opac
- print page using file -> print
- observe printing is fixed (pic2)

it seems this issue was introduced by bug 5287

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24876: Fix capitalization on patron search for holds

When placing a hold in the staff interface, the heading on the
search box reads: Search Patrons or clubs
The patch changes it to be Search patrons...  instead.
Also changes club id to club ID.

To test:
- Place a hold in the staff interface
- Verify the capitalization error
- Switch tabs to Clubs, see: club id
- Apply patch
- Verify it's all good now

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24854: DBRev 19.12.00.050

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24854: Remove IDreamBooks integration

The IDreamBooks service has not seen updates in a long time, so
we should remove the service from Koha as it's no longer operational.

To test:
- Apply patch and run the database update
- Verify that the IDreamBooks related system preferences are gone
- Verify that opac detail pages and result lists still work
  as expected
- Run t/db_dependent/UsageStats.t

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24918: Fix attribute mappings in Koha::Acquisition::Basket

When bug 18731 was written, the official vote on the RFC hadn't been
held yet. Once it got voted, the mappings were adapted and all the
dependent work fixed as well. But three attribute names were overlooked
and they become evident on developing 20212 which required them.

This patch fixes this really tiny mistake.

To test:
1. Just verify the voted attribute names.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24464: (QA follow-up) Make the method reflect the lack of FK

This patch makes it super obvious to understand by reading the code that
there's originally no foreign key.

To test:
1. Run the tests and notice they pass
2. Spot the # FIXME
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24464: Add K::A::Basket->creator

This patch adds a ->creator method to Koha::Acquisition::Basket to
retrieve the related Koha::Patron object.

Note: this should be re-done once 22658 is pushed, but at the time
being, there's not patch submitted, and it is not a trivial task because
of the column type missmatch. That said, the implementation of ->creator
in this class is simple enough to just rewrite it once 22658 is
submitted, and the tests will reflect the expected/unchanged behavior.

To test:
1. Apply this patches
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Acquisition/Basket.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24464: Unit tests

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24463: Add a basket_group relationship for baskets

This patch aims to bring consistency between the accessor name and the
underlying relationship to ease things when using it from the API.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Acquisition/Basket.t
=> SUCCESS: Tests pass! No behavior change!
3. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 21708: Sort MARC fields in TransformHtmlToMarc

Instead of pushing 999 in the front :)

Test plan:
Do not yet apply this patch.
Edit a record. Save. And open MARC view/view plain on opac-detail.
Verify that the biblionumber field (MARC21 999) follows leader.
Apply this patch. Restart Plack.
Edit a record. Save. And open MARC view/view plain on opac-detail.
Verify that the biblionumber field is in right position.
Run t/db_dependent/Biblio/TransformHtmlToMarc.t. Should pass now.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 21708: Add a few tests in TransformHtmlToMarc.t

Test plan:
Run t/db_dependent/Biblio/TransformHtmlToMarc.t without the module change
patch. Verify that the test fails on the order of fields.
Go to test plan of the second patch.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24324: Resolve error with delete_records_via_leader.pl

To test:

Have a record with the 000 position 5 set to "deleted" (record status)
Use koha-shell to run the job with the -i flag: perl misc/cronjobs/delete_records_via_leader.pl -c -i -v
* items and the record should be deleted
Mark another record with the 000 position 5 set to deleted (record status)
Use koha-shell to run the job without the -i flag: perl misc/cronjobs/delete_records_via_leader.pl -c -v
* if the record had items, it will not be deleted
* if the record did not have items, it will be deleted.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24902: Join different mc- limits with AND (elasticsearch)

In the advanced search form, you can enable several limits using syspref
AdvancedSearchTypes (namely itemtypes, shelving locations, collections)
When used, the resulting query parts end up being joined with OR, even
if the field is different. That means that if you pick "Book" under
itemtypes tab, and "Fiction" under collection tab, it will search
"itype:BOOK OR ccode:FIC". It should be AND.

For instance, if you select:
    Itemtypes:
        ✓ Book
        ✓ DVD
    Location:
        ✓ Child
        ✓ Adult

it should search:
    itype:(Book OR DVD) AND location:(Child OR Adult)

Test plan:
0. Do not apply the patch yet
1. Enable elasticsearch
2. Set syspref AdvancedSearchTypes = 'itemtypes|loc|ccode'
3. Create a new itemtype and a new authorised value for categories LOC
   and CCODE
4. Create a biblio with the new itemtype, another biblio with the new
   location, another biblio with the new collection, and again another
   biblio with the new itemtype, location and collection
5. Verify that you can find these new biblio records using only the
   "advanced search types" in the advanced search form
6. In the advanced search form, pick all 3 limits (itemtype, location,
   collection) and verify that it returns the 4 records.
7. Apply the patch
8. Repeat step 6, it should now return only the biblio that satisfies
   all criteria
9. Verify that if you select more than one
   {itemtype|location|collection} it still returns results that
   satisfies any selected criteria
10. prove t/Koha/SearchEngine/ElasticSearch/QueryBuilder.t

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Add missing use statement

This is a follow-up for commit 836e950d25148ca04ed0561c487772e3b0f7f8ee
    Bug 20443: Remove SearchIdMatchingAttribute

We removed the use of C4::Members::Attributes, that was using Text::CSV

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Fix merge_with behavior

merge_with were returning Koha::Patron::Attribute for existing attribute
and hashref for new attribute.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Handle non existent attribute when importing patrons

There is much more to do here, but this patch has the same behavior than
before: a warn is displayed in the log, the UI is not aware of it

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Fix patron modification approval

There was a mismatch between "value" and "attribute". Before this
patchset, "value" was sometimes used, but then it comes "attribute" to
match the DB column's name.
We must keep both here, when an upgrade is done we could have "value"
that is still in the borrower_modifications.extended_attributes JSON

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Do not explode if attribute does not have a valid av

Template process failed: undef error -
Can't call method "lib" on an undefined value at /kohadevbox/koha/Koha/Patron/Attribute.pm line 109.
                                                    at /kohadevbox/koha/C4/Templates.pm line 122

We should not crash as it if the attribute is linked to an invalid
authorised value (for instance the AV has been deleted)

We must provide a check in the search_for_data_inconsistencies.pl script

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Fix creation of new patron with attributes

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Fix POD

Fixes the following failures:

 FAIL Koha/Patron/Attribute.pm
   FAIL   pod
*** ERROR:
empty =head3

 in file Koha/Patron/Attribute.pm

*** ERROR:
empty =head3

 in file Koha/Patron/Attribute.pm

 FAIL Koha/Patron/Attributes.pm
   FAIL   pod
*** ERROR:
empty =head3

 in file Koha/Patron/Attributes.pm

   FAIL   pod coverage
POD is missing for merge_with
POD is missing for object_class

 FAIL Koha/Patrons.pm
   FAIL   spelling
 paramter  ==> parameter

except the missing pod for object_class, not needed.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Fix add/edit for checkboxes

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Fix Patrons/Import.t

->get_patrons has been removed but not replaced!

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Use search_with_library_limits for attribute types

Dealt with that previously in the module during the rebase.
It conflicted with bug 23281.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Fix Auth_with_ldap.t

Must be a hashref

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: (follow-up) Fix t/db_dependent/Koha/Patrons.t

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: (follow-up) Move GetBorrowerAttributes to Koha::Patron - ILSDI

Either we display everything, or only the info from the attribute itself
(i.e. not its type)

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Remove extended_attributes_code_value_arrayref AND C4::Members::Attributes

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Remove extended_attributes_merge

TODO We need tests here!

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Remove SearchIdMatchingAttribute

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Remove SearchIdMatchingAttribute - prove we are not cheating

Tests are still passing that way, we can continue

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Remove C4::Members::AttributeTypes

We do no longer need this package, we can use
Koha::Patron::Attribute::Types directly instead.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Remove num_patron

Replace C4::Members::AttributeTypes->num_patron with
Koha::Patrons->filter_by_attribute_type

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Move C4::Members::AttributeTypes::GetAttributeTypes to Koha::Patron::Attribute::Types

We can then now start to move methods from C4::Members::AttributeTypes
as well.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Remove CheckUniqueness

There is already a method in Koha::Patron::Attribute to check the
uniqueness constraint, let us it to replace CheckUniqueness

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Remove UpdateBorrowerAttribute and SetBorrowerAttributes

This patch replace Koha::Patron->get_extended_attributes with
->extended_attributes
It's now a getter a setter method.

It permits to replace UpdateBorrowerAttribute and use
create_related from DBIx::Class

Notes:
* We face the same variable names difference than in a previous patch
(value vs attribute)

Bug 20443: Remove SetBorrowerAttributes

squash  + RM get_extended_attributes

 RM get_extended_attributes

SQUASH Bug 20443: Remove UpdateBorrowerAttribute and SetBorrowerAttribute

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Remove DeleteBorrowerAttribute

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Remove DeleteBorrowerAttributes

This subroutine was only used once, easy to replace.
SetBorrowerAttributes must replace the attributes for the ones logged-in
user is allowed to edit, that's why we filter by the library limits

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Improve filtering method

Koha::Patron::Attributes->search mimicks what is done in
Koha::AuthorisedValues->search.
But actually it should be more explicit when the caller use it.
For instance filter_by_branch_limitation (see discussion on bug 11983).

This will be useful for the following patches as we will need a way to
replace the $no_branch_limit flag.
When the $no_branch_limit flag is called, a simple ->search call should
be done.
When we want to limit on a specific library we can pass the branchcode
in paramter of filter_by_branch_limitation (this is not used yet).
If not passed the logged-in user library will be used by default.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Move GetBorrowerAttributes to Koha::Patron->extended_attributes

The GetBorrowerAttributes subroutine return the attributes for a given
patron.

Using get_extended_attributes we can acchieve it easily. The problematic
here is to restore the method's name (value vs attribute,
value_description vs description of the authorised value, as well as
display_checkout that should not be a method of Attribute, but
Attribute::Type instead)

value_description was used when the attribute types were attached to an
authorised value category. To avoid the necessary test in template and
controller there is now a $attribute->description method that will
display either the attribute's value OR the value of the authorised
value when needed. We should certainly use this one from few other
places.

Notes:
* This patch rename Koha::Patron->attributes with Koha::Patron->get_extended_attributes.
It will be renamed with Koha::Patron->extended_attributes in ones of the next
patches when it will become a setter as well.
* GetBorrowerAttributes did not care about the library limits, we still
do not
* The opac_only flag was not used outside of test, we drop it off.
* To maintain the existing behavior we add a default order-by clause to
the search method [code, attribute]
* From C4::Letters::_parseletter we always display the staff description
of the AV, There is now a FIXME to warn about it
* FIXMEs are not regressions, existing behaviors must be kept
* TODO add a new check to bug 21010 to search for inconsistencies in
patron's attributes attached to non-existent authorised values
* One test has been updated in Modifications.t, order_by is now
by default set to ['code', 'attribute']

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Move GetBorrowerAttributes to Koha::Patron - ILSDI

FIXME See comment

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Move GetBorrowerAttributeValue to Koha::Patron->get_extended_attribute_value

We want to retrieve a specific patron's attribute for a given patron.
We then add a new method to Koha::Patron.

This patch add a getter method ->get_extended_attribute_value
to use the DBIx::Class relation

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Remove opac_display and opac_editable from Patron::Attribute

Same as previously for methods that have been added by bug 17792.
It's better to be explicite and tell we are fetch the related attribute's type

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 20443: Revert some stuffs from 12159

These methods have been added to Koha::Patron::Attribute but are wrong, see bug 18339

We should use ->type->$method instead

Moreover the tests exist in another subtest, we do not need them.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Fix selenium tests (highlight a bug in date management)

There is something wrong, and a regression has been caught by those
tests:
If an invalid date is passed from the add item form, the app now
crashes.
Before:
 * if the date was completely invalid, the field was blanked
silently
 * DateTime::Format::MySQL was used to convert dates, and it's not
 strict at all. For instance, what happened in the selenium tests for
 dateaccessionned: %Y-%m-%d was prefilled by the framework plugin, then
 the biblionumber was added, we ended with something like (eg for today)
 2020-03-234242 (with biblionumber=4242). DateTime::Format::MySQL
 converts that to 2020-03-23

We must deal with invalid dates, but I do not think it is good to add it
back to Koha::Item->store, we will prefer to raise the error to the end
user, saying that something went wrong (and more specifically the
dates).

The (ugly) trick was in C4::Items::_mod_item_dates

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Fix wrong variable name

Fix for t/db_dependent/Labels/t_Batch.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Fix CourseReserves/CourseItems.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Replace a new occurrence of ModItem

This new script have been pushed just before 23463.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: (RM follow-up) Add try block around Illrequest item creation

The recently introduced Ill item creation for circulation feature was
not properly handling the move from AddItem to Koha::Item->new()->store.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Fix items.cn_sort vs cn_sort

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: (follow-up 2) Fix timestamp default value

Removing previous change, again.
But this time in Object.pm
It fixes t/db_dependent/Koha/Object.t and t/db_dependent/Koha/Item.t is
still passing.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Use new method Koha::Object->set_or_blank

This patch fixes an issue when editing items.

* The issue
Cannot blank a subfield when editing an item.
If you have an item with itemcallnumber=42, then edit it, blank it and
save. The itemcallnumber is still 42

* Why? (line numbers from https://gitlab.com/joubu/Koha/-/tree/bug_23463)
additem (and other item's edition forms) receives a list of tags,
subfields and values, generates a MARC::Record::XML then calls
ModItemFromMarc:

717     my $itemtosave=MARC::Record::new_from_xml($xml, 'UTF-8');
727         my $newitem = ModItemFromMarc($itemtosave, $biblionumber,
$itemnumber);

And ModItemFromMarc:
 282     my $item = TransformMarcToKoha( $localitemmarc,
$frameworkcode, 'items' );
 283     $item->{cn_source} = delete $item->{'items.cn_source'}; #
Because of C4::Biblio::_disambiguate
 284     $item_object->set($item);

ModItemFromMarc never knows that the field has been blank.
Prior to bug 23463 we had a map of default values, and ModItemFromMarc
was doing:
 426     my $item = TransformMarcToKoha( $localitemmarc,
$frameworkcode, 'items' );
 427     my $default_values = _build_default_values_for_mod_marc();
 428     foreach my $item_field ( keys %$default_values ) {
 429         $item->{$item_field} = $default_values->{$item_field}
 430           unless exists $item->{$item_field};
 431     }

I do not want to reinsert that list of default values.

Here I wrote a generic method in Koha::Object to set the value passed in parameter,
or "blank" if not passed.
It's nulled if can be set to null in DB, or the default value is retrieved from
the schema info.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Replace new occurrence of AddItem

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Fix XSLT.t

From commit bf49eecdd87e2b29760226281ab1afc0a185c7f0
     Bug 23463: Replace AddItem calls with Koha::Item->store

in build_sample_item:
-    my $itype = delete $args->{itype}
-      || $self->build_object( { class => 'Koha::ItemTypes' } )->itemtype;
+    # If itype is not passed it will be picked from the biblio (see Koha::Item->store)

So before we generated a new itemtype if not passed, now we pick the one from biblioitem->itemtype.

For this specific test we need to make sure they are different.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Fix cn_source for new item

get_dirty_columns only work for existing items.

This fixes t/db_dependent/ShelfBrowser.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Fix IsItemIssued.t

It seems that some of those tests could be removed as safe_delete is
widely tested in Koha/Item.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Fix tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Fix safe_return return value in deletion tools

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: (follow-up) Fix timestamp default value

For discussion, this patch revert the changes made previously.

This line exists in Koha::Item->store as it is the translation of:
    if (exists $item->{'timestamp'}) {
        delete $item->{'timestamp'};
    }
that was coming from _do_column_fixes_for_mod (called from ModItem)

To preserve existing behavior I would be in favor of keeping it like that to
avoid regression, and deal with it separately if we want to improve/remove it.

So basically here we are setting it to undef in Koha::Item->store to make it
handle correctly by the parent Koha::Object->store. I agree that's kind of weird
and must be improved.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Move DelItem.t and Items_DelItemCheck.t tests to a Koha/Item.t subtest

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Fix timestamp default value

This sounds wrong as we should let the DBMS do that, but it was failing.
Here we are doing the same as Koha::Patron->store for dateenrolled

To recreate the failure, prove t/db_dependent/Koha/Item.t without this
patch:
DBD::mysql::st execute failed: Column 'timestamp' cannot be null [for
Statement "UPDATE `items` SET `more_subfields_xml` = ?, `timestamp` = ?
WHERE ( `itemnumber` = ? )" with ParamValues: 0='<?xml version="1.0"
encoding="UTF-8"?>

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: (follow-up) Remove DelItemCheck and ItemSafeToDelete

Wrong replacement in additem.pl

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: cn_sort needs to be updated if cn_source changed

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Replace recently added occurrences

Few occurrences have been added since this patchset has been originaly
written

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: build cn_sort only if needed

No need to calculate cn_sort if not dirty when we store

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Fix items.cn_source vs cn_source

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: update permanent_location if location has been updated

This change to Koha::Item->store seems correct here, but tests from /db_dependent/Items.t is failing now.

Adjusting them to make them pass.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Fix AutomaticItemModificationByAge.t

We do not want to log if nothing changed

How could we do that better?

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Getting less item-level_itypes warnings (?)

So, we get less warnings. I do not really understand why...

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Fix failing tests

And adding a FIXME about discard_changes

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Fix CancelReceipt.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Remove no longer needed subs related to default values

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Remove DelItemCheck and ItemSafeToDelete

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Remove DelItem

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Remove ModItem

There should not be any calls left

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Remove _mod_item_dates

We did not do anything useful here.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: _do_column_fixes_for_mod dealt with integer location and timestamp moved

That's done in Koha::Item->store, and Koha::Object->store for integers.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: _set_derived_columns_for_mod was only used for cn_sort

Same as _set_defaults_for_add

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23463: Move plugin hook

From C4::Items to Koha::Item

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>