Bug 13568 OAI Server doesn't handle properly resumption token

When responding to ListRecords and ListIdentifiers verbs, OAI server doesn't
return proper resumption token. At the end of a result set, OAI server
generates a resumption token even if there isn't anymore records. Consequently,
OAI harverster will send a new request, based on this invalid resumption,
token. OAI Server responds with an empty resultset, which is considered as an
invalid response by most of the harvesters.

TO TEST:

- Find in your DB, a day where a few biblio records have been created. The
  number of created biblios must inferior to OAI-PMH:MaxCount.

- Let say this day is 2014-01-09. Send an OAI-PMH request to Koha OAI Server:

  /cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=marcxml&from=2014-01-09&until=2014-01-09

- At the end of the result, you will see a resumption token which looks like that:

  <resumptionToken cursor="47">marcxml/47/2014-01-09/2014-01-09/</resumptionToken>

  This is wrong. No resumptiion token should be sent since there isn't anymore
  records to harvest.

- Apply the patch.

- Resend the OAI-PMH request. There is no resumption token at the end of the
  result.

- You could test also with ListIdenfiers verb in place of ListRecord.

Signed-off-by: Christophe Brocquet <christophe.brocquet@obspm.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13712 - Missing newline in DB update 3.19.00.010

To test:
- Make sure your database is pre 3.19.00.010 (for example by loading in
  an older backup)
- Run through the web-based database update (web installer, step 3)
- Verify that updates 3.19.00.010 and 3.19.00.011 are squashed
  together on one line
- Apply the patch
- Reset the database and run the web-based database upgrade again
- Verify that the two uupdates are noe displayed on separate lines
  (as separate bullet points)

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 12976: Fix some comment in tests

Previous comments were wrong.
Actually the main part of price values is correct.
Only some rounding and tax values are badly calculated.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 12976: Fix the footer if several tax rate exist

If more that 1 tax rate exist, 1 total ligne should be display in the
footer.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 12976: Use the centralize VAT and prices calculation - invoice.pl

Bug 12969 introduces a subroutine to centralize VAT and prices
calculation.
It should be use in the acqui/invoice.pl script.

Test plan:
0/ Don't apply the patch
1/ Create 4 suppliers with the different configurations
2/ Create a basket and create several orders
3/ Receive the items and create an invoice
4/ Go on the invoice page acqui/invoice.pl?invoiceid=XXX
5/ Verify you don't see any difference before and after applying the
patch on the invoice details table.
Note: The only different you should see is the price formating for
"Total tax exc.". Before this patch "432.10" was displayed "432.1".

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 7741 [Follow-up] Clear search terms in Z3950 search page

This patch makes a follow-up change and takes as its basis a dependency
on Bug 13218 - usability enhancement for z39.50 search. If we have a
one-column search form it's possible to put the "Clear search form" link
at the bottom of the form rather than in the header, logically
associating it with the grouping of form fields which will be affected.

Doing so also allows us to eliminate all the custom styling which was an
issue for QA.

Other changes made by this patch: Fixing the indentation
error, correcting the case of the link text ("Clear search form" instead
of "Clear Search Form"), and adding "preventDefault()" to the click
handler so that clicking the link doesn't scroll the user back to the
top of the page.

To test, apply the previous patch and this one on top of Bug 13218.
Repeat the test plan.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 7741: Added Clearing Capability to the Z39.50 Search fields

Tested in:
/koha/cataloguing/z3950_auth_search.pl
(Home:Authorities:New from Z39.50)

/koha/cataloguing/z3950_search.pl
(Home:Cataloging:New from Z39.50/SRU)

/koha/acqui/z3950_search.pl
(Home:Acquisitions:[Bookshop]:[Basket]:Order from external source)

All three forms can now be cleared.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13672 - OPAC locks up browser when browsing items from series link with NoveList and OpacHighlightedWords

It appears that an empty string is sneaking into the list of words
to highlight and that is causing the browser to freak out.

Test Plan:
1) Enable NoveList
2) Enable OpacHighlightedItems
3) Search a title with a series (i.e. The Hunger Games)
4) Click into the record
5) Click on the series to search it
6) Click on a title in the search results
7) If Firefox doesn't crash, it gives the unresponsive script message
8) Apply this patch
9) Repeat steps 3-6
10) Browser doesn't freak our

Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
could not confirm the problem, but small change and
highlighting terms on result list and couldn't find any
regressions in highlighting.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13700: Fix item search templates for translation

Since translator rebuild HTML tags with double quotes for attributes
values, use double quotes in english template too and escape them with
backslash.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

It works well!!

Test:
1) Intall translation  (de-DE, es-ES)
2) to Go to item search, in english, do a search that gives results
3) Switch language, repeat search, result's table does not render
4) Apply patch
5) Install translation again
6) Repeat 3, now results are displayed

No koha-qa errors

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

Bug 13684: C4::Budgets should not be used in C4::Output

Bug 12844 removed the call to C4::Budgets::GetCurrency.
The use of this module can be completely removed from C4::Output.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
It makes sense and no runtime errors found.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 13603 - autoBarcode setting hbyymmincr not taking month into account when looking up next increment

The hbyymmincr autoBarcode builder looks up all barcodes that begin with
any number of letters ( the branchcode ) and a two-digit year directly
afterward ( e.g. MPL15 ), then grabs the last four digits as the
increment counter. The problem is, this does not take months into
account, so unlike the description that says the system is limited to
10,000 items a month, it is limited to 10,000 items per *year*.

This situation can easily be corrected my adding the month into the
lookup ( e.g. MPL1501 ).

Test Plan:
1) Enable autoBarcode, set to hbyymmincr
2) Catalog an item, let the barcode.pl plugin generate
   the next barcode.
3) Catalog an item with the barcode MPL15009999
4) Catalog another item, use the barcode.pl plugin to
   generate the next barcode.
5) Note the barcode is generated ends with '0001' instead of the
   correct increment
6) Apply this patch
7) Repeat step 4
8) Note the barcode now ends with the correct increment

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

Signed-off-by: Jeanne Heicher <nppublib@pa.net>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Problem confirmed - the continuing number bit was not reset
to start counting from 1 again on the beginning of a new month.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13417: DBRev 3.19.00.011

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13417: Remove warnings

Without this patch
  perl -wc C4/VirtualShelves.pm
failed:
Constant subroutine C4::VirtualShelves::SHELVES_MASTHEAD_MAX redefined
at /usr/share/perl/5.20/constant.pm line 156.
Constant subroutine C4::VirtualShelves::SHELVES_COMBO_MAX redefined at
/usr/share/perl/5.20/constant.pm line 156.
Constant subroutine C4::VirtualShelves::SHELVES_MGRPAGE_MAX redefined at
/usr/share/perl/5.20/constant.pm line 156.
Constant subroutine C4::VirtualShelves::SHELVES_POPUP_MAX redefined at
/usr/share/perl/5.20/constant.pm line 156.
Constant subroutine C4::VirtualShelves::SHARE_INVITATION_EXPIRY_DAYS
redefined at /usr/share/perl/5.20/constant.pm line 156.
Subroutine GetShelves redefined at C4/VirtualShelves.pm line 103.
Subroutine GetAllShelves redefined at C4/VirtualShelves.pm line 159.
Subroutine GetSomeShelfNames redefined at C4/VirtualShelves.pm line 189.
Subroutine GetShelf redefined at C4/VirtualShelves.pm line 230.
Subroutine GetShelfContents redefined at C4/VirtualShelves.pm line 262.
Subroutine AddShelf redefined at C4/VirtualShelves.pm line 315.
Subroutine AddToShelf redefined at C4/VirtualShelves.pm line 353.
Subroutine ModShelf redefined at C4/VirtualShelves.pm line 394.
Subroutine ShelfPossibleAction redefined at C4/VirtualShelves.pm line
454.
Subroutine DelFromShelf redefined at C4/VirtualShelves.pm line 545.
Subroutine DelShelf redefined at C4/VirtualShelves.pm line 589.
Subroutine GetBibliosShelves redefined at C4/VirtualShelves.pm line 603.
Subroutine ShelvesMax redefined at C4/VirtualShelves.pm line 628.
Subroutine HandleDelBorrower redefined at C4/VirtualShelves.pm line 648.
Subroutine AddShare redefined at C4/VirtualShelves.pm line 683.
Subroutine AcceptShare redefined at C4/VirtualShelves.pm line 703.
Subroutine IsSharedList redefined at C4/VirtualShelves.pm line 731.
Subroutine RemoveShare redefined at C4/VirtualShelves.pm line 750.
Subroutine _shelf_count redefined at C4/VirtualShelves.pm line 764.
Subroutine _CheckShelfName redefined at C4/VirtualShelves.pm line 788.
C4/VirtualShelves.pm syntax OK

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13417: Add the permission for all languages

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

Bug 13417: (qa follow-up) Rename the permission to delete_public_lists

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

Bug 13417: Limit the permission to delete

It's preferable to limit the permission to delete shelves.

Apply both patches before testing, then follow this test plan

Currently a public list can only be deleted by its owner.
This means lists can exist infinitely.
This will introduce a new permission for list. With this permission, a
staff member will be allow to delete any public lists.

Test plan:
1/ Add the manage_shelves permission to a patron.
2/ Login with this patron
3/ Go on the public list view
4/ You should be able to delete all public lists

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

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

Bug 13417: Allow staff members to manage public lists

Currently a public list can only be deleted by its owner.
This means lists can exist infinitely.
This will introduce a new permission for list. With this permission, a
staff member will be allow to delete any public lists.

Test plan:
1/ Add the manage_shelves permission to a patron.
2/ Login with this patron
3/ Go on the public list view
4/ You should be able to edit all public lists

Followed test plan. Works as expected.
Signed-off-by: Marc Veron <veron@veron.ch>

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

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

Bug 13142 - Change "mobile phone" label back to "other phone"

Labeling a phone number field "mobile phone" eliminates the usefulness
of having the labels "primary" and "secondary." Generic labels let the
user populate the fields according to their importance rather than their
type.

To test I recommend editing a patron record so that the values in the
patron record contain a label matching the table column:

borrowers.phone : 555-555-1234 (primary - phone)
borrowers.phonepro : 555-555-5678 (secondary - phonepro)
borrowers.mobile : 555-555-9012 (other - mobile)

View this patron's information in the various affected templates and
verify that the labels correctly match the data:

 - OPAC "your personal details" (opac-memberentry.pl)
 - Submit changes to primary, secondary, and other phone via the OPAC.
   In the staff client, view the confirmation for those changes.
 - Patron details in the staff client (moremember.pl)
 - Patron entry/edit in the staff client (memberentrygen.pl)
 - Patron duplicate confirmation in the staff client
   (you can navigate directly to
   /members/moremember.pl?borrowernumber=XXXX&print=brief)

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

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

Bug 13545 [QA Followup] - Complete POD documentation

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13545: (followup) POD and error handling

This patch adds POD to the new /svc/barcode service, and also implements some
error handling.

Sponsored-by: Universidad Nacional de Cordoba
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13545 - Add barcode image generator service

If we add a script to Koha that can be passed a barcode, we will be able
to easily embed item and patron barcodes into html printable slips and
notices. This can be very helpful for librarians, as it means scanning
an image instead of typing in the barcode manually.

This patch adds a barcode image generator that can be passed a barcode
and an optional type ( defaults to Code39, all GD::Barcode types should
work ). This image can be embedded in html slips and notices.
( e.g. <img src="/cgi-bin/koha/svc/barcode?barcode=<<items.barcode>>"></img> )

Test Plan:
1) Apply this patch
2) Browse to /cgi-bin/koha/svc/barcode?barcode=123456789 on your server
3) Note the barcode image

Signed-off-by: Brandon <brandon_h27@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13189 - Patron card creator patron search browse by last name broken by extended attributes

The browse by last name letters on the patron search for the patron card
creator doesn't work quite right. If extended patron attributes are
disabled, it works fine, but if they are enabled, they are searched even
when using the browse last name. Thus, if a searchable attribute has a
"D" in it, and one clicks the "D" link for the last name browser, that
patron will show even if he or she has no "D" in his or her hame!

Test Plan:
1) Enable extended patron attributes
2) Add a new searchable patron attribute
3) Create a new patron with the last name "Ace"
4) Add the value "D" to the attribute for this patron
5) Browse to the patron card maker, start a new patron batch
6) Click "Add item(s)" to bring up the patron search
7) Click the letter "D" in the patron search box
8) Note that "Ace" shows in the results list
9) Apply this patch
10) Repeat step 7
11) Note that "Ace" no longer shows in the results list
12) Perform a regular search by putting the letter "D" in the "Name:"
    field, and hit the "Search" button
13) Note this time the results *do* have Ace in them

Signed-off-by: Frederic Demians <f.demians@tamil.fr>
  Well described for a tricky bug. Reproducible. Fixed with this patch.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
This works as described, no problems or regressions found.

Bug 13343: Embed items when send a basket/shelf

When a basket/shelf is downloaded, items are embedded, but not when
sending it by email.

We would expect to get the same file.

Test plan:
On Opac and intranet:
1/ Add records with items to your basket and send it by email.
Verify the file contains items.
2/ Add records with items to a list and send it by email.
Verify the file contains items.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13019 [QA Followup] - Fix stale unit test

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13019 [QA Followup] - Allow chaining

By returning the object itself instead of a boolean, we can chain
methods together while retaining the exact same functionality.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13019 [QA Followup] - Remove use of encode

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13019: [QA Follow-up] Rename new_from_dbic and few typos

Since new_from_dbic is not meant as a public method, this patch adds
a prefix to the name of this internal routine. For the same reason I
removed it from t/Borrower.t.
Removed one use overload-line in Objects (not used).
Resolved a few typos.

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

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

Bug 13019 [QA Followup] - Allow find() and search() to be called as static methods

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

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

Bug 13019: (follow-up) Remove smartmatch operator

This patch also adds 1 test.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

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

Bug 13019 - Add base classes on which to build Koha objects

The idea behind this is to have a pair of base classes on which to build
our new generation of Koha objects. Koha::Object is a base class, which
in it's most basic form, is to represent a row in a table. For example,
Koha::Borrower inherits from Koha::Object. So too could Koha::Biblio
and Koha::Item for example.

Koha::Objects is to represent a way to fetch and manipulate sets of
objects. For example, Koha::Borrowers has a method to get a
Koha::Borrower object by id and a method to search for an get a list
of Koha::Borrower objects. Right now Koha::Objects has only the
essentials but can easily be extended and those enhancements will be
passed down to all the child classes based on it.

By using these classes as a base, we will add consistency to our
code, allow us to keep our code DRY, reduce bugs, and encapsulate our
database access among other benefits.

Test Plan:
1) Apply this patch
2) prove t/Object.t t/db_dependent/Object.t t/db_dependent/Objects.t

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

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

Bug 13335 - Holds and priority display via OPACShowHoldQueueDetails confusing

No matter what the selection of OPACShowHoldQueueDetails is, if it is
enabled it displays a line "Holds and priority:" even if you've opted to
hide one of those!

Test Plan:
1) Apply this patch
2) Test each setting of OPACShowHoldQueueDetails
3) Ensure each setting displays the correct fields ( or lack thereof )

Signed-off-by: Christopher Brannon <cbrannon@debian.localdomain>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13636 - Staff search results item status incorrect for holds

Imagine this scenario: we have one record with four items. Two of those
items are checked out, one of those items is a waiting hold, and one of
those items is available. We would expect to see this on the search
results page. Instead, we will see both non-checked out items as
unavailable due to waiting holds.

This is due to a semantic issue GetReserveStatus.
C4::Search::searchResults uses GetReserveStatus to get the reserve
status of each item, but unlike all other calls to the sub, this one
passes in not only itemnumber, but biblionumber.

When no reserve is found for the available item, the subroutine uses the
biblionumber to grab what is essentially an arbitrary reserve to use for
the status. This makes no sense and this functionality should be
entirely removed from the subroutine so regressions like this will be
prevented in the future.

Test Plan:
1) Create one record with 4 items
   a) check two of the items out to patrons
   b) set one of the items as a waiting hold
   c) leave the fourth item as available
2) Run a search where this record will be in the results list
3) Note that the results list 2 items on loan, two unavailable
4) Apply this patch, reload the search results
5) Note that the results list 1 available, 2 on loan, 1 unavailable

Signed-off-by: John Andrews <jandrews@washoecounty.us>
Signed-off-by: Sheila Kearns <sheila.kearns@state.vt.us>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Note: This is for the staff search result list!

Works as expected.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13431 [QA Follow-up]: Shared FastMmap file causes issues

1) Removed 'use C4::Context;' because it can lead to introduction
of circular reference in the near future
2) Put fastmmap initialization code into an eval {} block, to catch
various kinds of errors which can still occur during it's init in
some [less usual] Koha setups and/or more unusual circumstances
3) Do not include UID in the sharefile name (it will be constructed
using namespace + database name + database host instead).

Test plan addendum:

   s/and UID//

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13431 - Shared FastMmap file causes issues

Koha::Cache package does not take into account that, when using
fastmmap caching variant, mmaped cache file created in /tmp
(typically: /tmp/sharefile-koha-koha), would only be further
accessible to the one given OS user - the one which created it.
In many Koha setups, in the circumstances when various system scripts
are executed by 2+ users with diffrent UIDs (like multi-tenant servers,
for example) this may cause many kinds of issues. Observable symptom
is usually the appearance of the below error when searching, or looking
at MARC Framework pages and a few other places:

Open of share file /tmp/sharefile-koha-koha failed: Permission denied
at /usr/lib/perl5/Cache/FastMmap.pm line 640.

This patch:
- disables initialisation of fastmmap caching subsystem unless it is
explicitly requested by the user (CACHING_SYSTEM=fastmmap)
- disables fastmmap cache usage for command line scripts
(i.e. when GATEWAY_INTERFACE environment variable is not defined)
- adds the database name, host name and an ID of the OS user to the
mmaped file name created in /tmp, to prevent various kinds of
unintentional conflicts and/or permission problems from happening

To test:

1) remove the /tmp/sharefile-koha-* file[s] (if any)
2) do something which would lead to its re-creation (e.g., performing
any search in OPAC should be sufficient to cause that)
3) observe that /tmp/sharefile-koha-koha got created
4) remove it
5) apply patch
6) redo step 2)
7) observe that aforementioned file is no longer created in /tmp
8) set CACHING_SYSTEM environment variable to 'fastmmap'
9) redo step 2), observe that /tmp/sharefile-koha-* file got created
and that it's name now contains hostname, database name and UID
10) ensure that everything still works like it should and that there
are no regressions of any kinds anywhere in the system ;)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13240 [QA Followup]

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

Bug 13240: Remove commented warns

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

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

Bug 13240: Remove some code obfuscation

my $bar;
my $foo = $bar->{borrowernumber} ||= {};
$foo->{one} ||= 'something';
$foo->{two}++;

What does $bar contain?

$VAR1 = {
          'borrowernumber' => {
                                'two' => 1,
                                'bar' => 'something'
                              }
        };

Not really obvious.

Maybe something I did not see is hidden.

Test plan:
Verify the digest for DUE and PREDUE work as before.

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

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

Bug 13670 - Patron holds table not loading when holds tab anchor is used

If a link to a patron record ends with #reserves, the holds tab is selected
on page load, but the holds table does not load.

Test Plan:
1) Build your holds queue
2) From the holds queue report, open any patron link
3) Note the patron's reserves do not load
4) Apply this patch
5) Clear your browser cache
6) Repeat step 2
7) Note the holds table now loads

Signed-off-by: Sean McGarvey <seanm@pascolibraries.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13319: Fix Koha::Acq::Order should set parent_ordernumber

The tests highligh the problem: if the parent_ordernumber attribute is
not set on inserting an order, the object returned by the method does
not contain the value (undef).

Test plan:
Verify the tests are consistent and
    prove t/db_dependent/Acquisition/CancelReceipt.t
should return green.

You can also verify that receive partially an order and cancel the
receipt work as expected.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 12323 [SIGNED OFF] - Acquisitions search results fixed in order of biblionumber

The acquisitions search is exhibiting the same behavior as bug 11410.
Results are always fixed in order of biblionumber, among other possible
issues ( including the ampersand issue ).

Test Plan:
1) From an open basket, choose "Add to basket"
2) Run a search for "From an existing record"
3) Note the "View MARC" URLs are fixed in order of biblionumber
4) Apply this patch
5) Refresh the page
6) Note the results are no longer fixed in order of biblionumber

Signed-off-by: David Cook <dcook@prosentient.com.au>

Works as described.

I think the code could be a bit tidier, but I think it makes sense to
use buildQuery here. It'll detect CCL, CQL, and PQF queries, as well
as parsing a regular keyword search as one would expect when searching
the catalogue.

It also has the added bonus of performing relevance searching,
so long as QueryAutoTruncation is off, and so long as library staff
avoid using the "*" truncation wildcard (see bug 12430).

While there are simpler ways to fix this acq bug, I think this was
probably the best move, as it adds a bit to the consistency of what
librarians can expect from their search results.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Same result with and without the patch but I agree with the changes.
BuildQuery should be called before SimpleSearch if QP is not used.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13675: Do not set aqorders.budget_id to NULL

C4::Acquisition::ModReceiveOrder updates the aqorders with
budget_id=NULL if no budget_id given in parameter.
Actually the same budget_id should be used.
In tests (especially t/db_dependent/Acquisition/TransferOrder.t),
ModReceiveOrder is not called with a budget_id param and set to NULL the
budget_id value.

test plan:
  prove t/db_dependent/Acquisition/TransferOrder.t
should return green

Note that this bug should not appear using the interface.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
The fix makes sense, and running
 $ prove t/db_dependent/Acq*
returns all green. koha-qa.pl also likes it.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 5511: DBRev 3.19.00.010

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 10900: 2 occurrences more

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 10900 - Follow up, since more has been added to master

Booksellers.t and Koha_template_plugin_Branches.t both had
function calls to ::set_userenv added to them. This patch handles
those additions.

TEST PLAN
---------
1) Branch a new git branch
2) prove -v t/db_dependent/Bookseller.t
   -- It should work.
3) prove -v t/db_dependent/Koha_template_plugin_Branches.t
   -- It should work.
4) Apply only the first patch.
5) Repeat steps 2 and 3.
   -- They should both FAIL!
6) Apply the second patch as well.
7) Repeat steps 2 and 3.
   -- The should both work.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 10900 - Incorrect calling conventions accessing C4::Context

There were multiple calling conventions for C4::Context's
set_userenv routine. So the following commands were used to
find discrepancies:
    grep "::set_userenv" `find .`
    grep "\->set_userenv" `find .`

The first grep demonstrated that the smaller change is from
:: to -> as only C4/Auth.pm, installer/InstallAuth.pm, and
t/db_dependent/Circulation.t would need to be modified. This
patch corrects C4::Context's set_userenv routine to be object
call based (use ->) by using a shift to ignore the first
parameter, and modify the three files found with :: calls.

As the result of trying to roll a distribution,
t/Circulation_barcodedecode.t was discovered to be faulty. The
cause being incorrect parameters! This was hidden when there
was no shift in the set_userenv routine. However, with its
correction, the test broke.

This led me to read the POD documentation for the function
set_userenv in C4::Context and realize it was outdated as
well. It has been revised to match the current version of
the function.

Then intentionally bad parameters passed to the set_userenv
routine in C4::Context were hunted down. The biggest problems
were missing surnames or branch names.

Rebase required because of shibboleth change in C4/Context.pm

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 10177 - Whitespace is accepted within the password but leading and trailing spaces are not.

Test plan:
1: Go to your patron
2: Go to change username and/or password.
3: Change password to something with spaces in the middle. Notice it is invalid.
4: Apply this patch.
5: Change password again using spaces in the middle. Notice it is accepted.
6: Change password with leading and/or trailing spaces, notice they are not accepted.

Applied bothe patches. Followed test plan. Patch behaves as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 10177 - whitespace is a valid password causing pernicious perturbations.

Test plan:
Go to your patron, go to change username and/or password. change
password to whitespace, notice it is accepted, apply this patch,
change password again, to whitespace, notice password is not valid.

Followed test plan. Patch behaves as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug - 5511 [Followup]: QA Fixes

- Move database update entry to correct spot
- Fix version number in database update
- Fix capitalization in sys pref description
- Fix sequence in sysprefs.sql

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

Bug - 5511 [Followup]: Missed Instance

There was an instance of the pragma missed which meant the the original
patch set didn't actually solve the problem in a large number of cases

This patch adds in the relevant statement.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug - 5511 [Followup]: Switched defults

Agree with Chris that the defualt should maintain the higher security
and not reduce it.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug - 5511: Added new system preference: SessionRestrictionByIP

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug - 5511: Check for Change in Remote IP address for Session Security. Disable when remote ip address changes frequently.

To Test:
1) Enable the system preference SessionRestrictionByIP
2) Change your system IP. It will not checkout your system IP or signout.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13007: DBIC updates

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13007: DBRev 3.19.00.009

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13007: Special case budgetid == ''

In this special case (the suggestion is linked to "all funds"), the
budgetid value should be NULL in DB.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, passes old and new tests.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13007: Add a foreign key for suggestions.budgetid

The DB field suggestions.budgetid should be linked to
aqbudgets.budget_id.

If the fund is removed, this column should be set to NULL.

Test plan:
1/ Using your SQL CLI (or equivalent), create or update 1+ suggestions and
set "0" in the budgetid field (or a nonexistent budget id).
2/ Execute the updatedabase script.
3/ Verify that your suggestion is unlinked to the nonexistent fund.
4/ Verify the constraint has correctly been added (show create table
suggestions).
5/ Check that this change does not affect the behavior on adding a
suggestion (linked to a fund or not).

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 12944: (QA followup) fix POD errors from koha-qa.pl

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 12944: Fix regression on translating

This patch fixes the translation for the "Remove" button.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Tested:
- acq history search with different searches
- patron lists patron search
Passes all tests and QA script.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 12944: The autocomplete should only be applied for the form

If it's the result view, the autocomplete should not be apply to the
input element (it does not exist).

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 12944 [Patron lists follow-up] There is no way to search orders by creator

This bug modifies not just the order search form but also the patron
lists feature, making use of the new script for building a patron
autocomplete form. However, the test plan does not specify that the
patron lists form be tested as well, and it doesn't work.

This patch corrects a minor flaw which broke the patron search
autocomplete when adding patrons to a patron list.

To test, create a new patron list if necessary. Click the "add patrons"
button to add patrons to the list. In the patron search form type a
partial patron name. You should get an autocomplete dropdown of existing
patrons. Selecting one should work correctly.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 12944: Search orders by basket creator

This patch adds the ability to search orders using the basket creator.

Test plan:
- go on the order advanced search form (acqui/histsearch.pl)
- use the autocomplete input to search patrons
- launch the search and verify the results are consistent with the
  values you have filled.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 12944: Refactor the patron autocomplete

The patron list feature uses an autocomplete field to search patron.
This will be reused in the next patch.
This patch should not introduce any behavior change.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

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

Bug 12944 [QA Followup] - Rename patrons.pl to patrons.js

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 11954 - Clean up currency default files

It makes no sense to have TRUNCATE or foreign key checks.

TEST PLAN
---------
1) Back up your database
2) in an mysql client:
     use kohadata;
     SET FOREIGN_KEY_CHECKS=0;
     TRUNCATE currency;
     source /home/mtompset/kohaclone/installer/data/mysql/ru-RU/optional/sample_currency.sql
     SET FOREIGN_KEY_CHECKS=0;
     TRUNCATE currency;
     source /home/mtompset/kohaclone/installer/data/mysql/uk-UA/optional/sample_currency.sql
     SET FOREIGN_KEY_CHECKS=0;
     TRUNCATE currency;
     source /home/mtompset/kohaclone/installer/data/mysql/it-IT/necessari/parameters.sql
     SET FOREIGN_KEY_CHECKS=1;
     QUIT
   -- These should all work without problem.

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

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

Bug 13299 - Add home library column to the holds ratios report

It seems odd that only the holding libraries are listed on the holds
ratios report, we should add the home libraries as well.

Test Plan:
1) Apply this patch
2) Run the hold ratios report
3) Note the new home libraries column

Signed-off-by: Heather Braum <hbraum@nekls.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Bug 13298 - Holds ratios report ignores ordered items

The holds ratio report ignores ordered items. This could cause a library
to inadvertently order more copies of a title than they actually need.
An option should be added to count ordered items ( i.e. any negative
notforloan value ).

Test Plan:
1) Apply this patch
2) Create a record with two items, one regular, one ordered.
3) Place 3 holds on the item
4) Run the reserve ratios report, by default you should see this record
5) Check the new 'include ordered' checkbox, rerun the report
6) Note that record is no longer displayed

Signed-off-by: Heather Braum <hbraum@nekls.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13201: GetSuggestion takes suggestionid

Having $ordernumber as the parameter in a function which is
handling suggestions and it is used as a suggestion id is
confusing to the coder and person trying to read the code.
This patch corrects the confusion.

REVISED TEST PLAN
-----------------
1) perldoc C4::Suggestions
   -- notice it says ordernumber for the GetSuggestions example.
2) Ensure you have CPL in your branches table.
3) prove -v t/db_dependent/Suggestions.t
   -- if you have more than 8 itemtypes in your database, two
      tests at the end will fail, but that is not relevant to
      the modified GetSuggestions code.
4) apply patch
5) perldoc C4::Suggestions
   -- notice it is corrected now.
6) prove -v t/db_dependent/Suggestions.t
   -- it should run with the same results as in step 3.
      That is, if you have 8 itemtypes, all pass, otherwise
      those two tests fail.
7) run koha qa test tool

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 12842: Add DDC classification numbers to MARC21 XSLT detail page

Patch will add DDC classifications, divided with | below the subjects
in the OPAC and staff detail pages.

To test:
- catalogue a record with one or mulitple 082$a subfields
- verify the classifications are displayed in OPAC and staff detail page
- Add following CSS to OpacUserCSS or IntranetUserCSS:

.results_summary.ddc {
  display:none;
}

- Verify the DDC classifications are now no longer displayed

Signed-off-by: Frederic Demians <f.demians@tamil.fr>

Test plan ok. Repetions of $a subfield are separated by a space, which seems
correct. I'm not sure whether other subfield than $a should be displayed on
intranet. Whatever, this could be added later.

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

Bug 12543 - Add userid as matchpoint for "Import patrons" tool

The a patron's userid should be a matchpoint in the same manner as
cardnumber. Though not enforced as a unique key by the database yet
( pending bug 1861 ), this field is effectively unique as uniqueness
is enforced by Koha itself.

Test Plan:
1) Apply this patch
2) Browse to tools/import_borrowers.pl
3) Download the starter CSV file
4) Edit the csv file to include 1 or more patrons
   * Make sure to leave the borrowernumber field empty
   * Make sure the userid field matches the patrons you wish to overwrite
5) From import_borrowers.pl, upload your file
   * Set "Field to use for record matching" to "Username"
   * Set "If matching record is already in the borrowers table" to "Overwrite the existing one with this"
6) Click "Import"
7) Verify the patrons in your file have been updated in Koha

Signed-off-by: Frederic Demians <f.demians@tamil.fr>

Do exactly what's announced. This may help.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13404: [QA Follow-up] Minor rewording of some warnings

Changes are confirmed with author (Mirko Tietgen). See Bugzilla.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13404 [QA Followup]

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13404 - More System information about GRS1 vs DOM settings

System information indicates missing <zebra_bib_index_mode> and <zebra_auth_index_mode> entries in koha-conf.xml.

This patch

- adds warnings about a possible mismatch between these settings and the actual setup
- adds info about the actual setup/what values to use for <zebra_bib_index_mode> and <zebra_auth_index_mode> if they are not set.

The assumption is that a path including 'zebra-*-dom.cfg' in the respective section indicates a DOM setup.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13401 - sort branches alphabetically in admin/authorised_values.pl

This patch adds a sort of branches alphabetically by name in authorised values administration, like in many other pages.

Also replaces var $branch (too ambigus) by $branchcode.

Test plan :
- Create 2 branches : code=AAA,name=Zbranch and code=ZZZ,name=Abranch
- Create a new authorized value : admin/authorised_values.pl
- Look at "Libraries limitation" :
=> Without patch branches are sorted by code : Zbranch then Abranch
=> Without patch branches are sorted by name : Abranch then Zbranch
- Select a branch and save
- Re-edit
=> Check the same branch is selected

Followed test plan. Patch behaves as expected.
Signed-off-by: Marc Veron <veron@veron.ch>

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

Bug 13352: QA Follow-up: Remove Price filter from "Warning at (%)"

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13352: On editing, prices should not be formatted

Bug 12979 refactored the way to display prices.
The price format configuration was duplicated everywhere it was used.

All calls looks good except the one in admin/aqbudgetperiods.pl
In this one, the prices are formatted for an edition field (input).
This means the input is incorrectly filled even if the user does not
update the field.

At the end, maybe should we manage formatted prices everywhere, even in
inputs, but it's not in the scope of this bug.

Technically, a new subroutine format_for_editing is added to the
Koha::Number::Price module.
It should be called everywhere a price is displayed in an input field.
At the moment, it only does a sprintf("%.2f"), but it is a first step to
let the number of decimals to display configurable.

To test:
1/ Verify the issue described is fixed (editing a budget with a total
amount > 1000 (With CurrencyFormat is US or FR).
2/ Verify you can edit a fund with a total amount > 1000

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13602: (QA followup) take advantage of the defined-or operator

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13562: Fix item search on item MARC subfields

Test plan:
1. Add 4 item search fields (in Administration › Items search fields):
   a. 1 biblio field linked to a DB field
   b. 1 biblio field not linked to a DB field
   c. 1 item field linked to a DB field
   d. 1 item field not linked to a DB field
2. Make sure you have some data in those fields to search on.
3. Go to item search page and do a search using these new fields, make
   sure the result is correct.

Signed-off-by: Nick <nick@quecheelibrary.org>

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

Bug 13602 - Username/password already exists when editing borrower attributes and messaging preferences

When editing a Patron's messaging preferences or borrower attributes, one cannot
save changes because 'userid' is not given as a parameter.

It shouldn't, but the
Check_Userid($newdata{'userid'},$borrowernumber) -check must have it.

REPLICATE THE ISSUE:
1. Find a Borrower.
2. Go to the details-tab.
3. Under "Patron messaging preferences" click edit to edit just the messaging
   preferences.
4. Just save, no need to make any modifications.
5. Observe the error "Username/password already exists"
   This happens to all/most of the sub-forms in the Borrower's Details-view.

AFTER THIS PATCH:
Check_Userid() get the $userid from the new parameters, or if not present, from
the koha.borrowers-table.

TEST FOR REGRESSION:
1. Edit a borrower normally. Notice no errors.
2. Edit a sub-form (Alternate address/Patron messaging preferences). Notice no
   errors.

DISCLAIMER: You are free to steal/rip/modify/hack this patch to your soul's
extent without my permission. As long as it makes to Koha I am happy.

Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13638 - Batch patron modification tool missing option to switch language

The batch patron modification tool's template was created without the
footer include at the bottom. Without the footer include the language
toolbar will not be displayed.

All page templates should include the global footer include:

[% INCLUDE 'intranet-bottom.inc' %]

To test:
 - Install at least a second language
 - Enable at least 2 languages (language system preference)
 - Go to tools > Batch patron modification
 - Verify that at the bottom of the page the language chooser is shown

Followed test plan. Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13418: Clean C4::VirtualShelves::Page - Part 2

I don't understand at all the usefulness of $privshelflist and
$privtotshelves in C4::VirtualShelves::Page
They look useless.

If you deep in the git history, you will find
    commit 25eb36f21fe952118327d2c84dfef5d38eee31a7
    Bug #2531 Cannot delete private shelf

I am not able to reproduce this issue after this patch

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

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

Bug 13418: [QA Follow-up] Use unshift instead of push

Some SQL parameters are added to the wrong side of the list.
This effectively clears all private lists from view :)

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

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

Bug 13418: Clean C4::VirtualShelves::Page a bit

I tried to reuse this package for a new development, I did not manage.
It's a really hard to read and to understand.

Here my first try, this patch only remove the call to _shelf_count in
the GetShelves subroutine.
Someone might want to get this value for another reason and from
somewhere else.

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

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

Bug 13450 [QA Followup] - Remove tab character from start of line

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

Bug 13450 - Title and breadcrumb of Patron form can look weird in translations

This patch moves the patron category name or category type at the end of title and breadcrumb for better translatability.

Note: It does not change anything in functionallity, it only changes display of strings.

To test:
Apply patch
Search a patron and edit it in all possible ways: 'Edit' button on top of details page, 'Edit' links in details page, 'Edit' links in search result table if multiple patrons are found)
Duplicate a patron
Create a new patron
Make sure that title and breadcrumb appear in a meaningful way with pattern similar to the following:
Modify patron Hansli Meier (Juvenile)

(Missing </div> added)

Signed-off-by: Aleisha <aleishaamohia@hotmail.com>

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

Bug 7904: (QA followup) fix tests

The package name for SIP wasn't fixed in the tests by the original patches.

This patch fixes it.

To test:
- Run
  $ prove t/db_dependent/SIP_ILS.t
- Tests should pass with the patch.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13262: (QA followup) fix duplicate variable definition

Running

 $ prove t/db_dependent/XSLT_Handler.t

raises a couple warnings because of variable redefinitions. This small patch fixes that.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13262 - Add parameters to XSLT Handler transform method

This patch adds an optional hashref argument to the XSLT_Handler
transform() method. It allows you to send key => value pairs
parameters to the XML::LibXSLT object, which you can reference
in a XSLT via <xsl:param name="XXX" />.

The parameter value is evaluated as an XPath query, so you can only
pass quoted strings (i.e. "'test'") or numbers. Otherwise, the
XSLT engine will interpret it as a Xpath query and will run it
on the XML that you're transforming.

The most common use case is sending strings to a XSLT. In my case,
this is an OAI-PMH identifier that comes in a OAI response but not
the actual metadata. See the following link from the official POD:
http://search.cpan.org/~shlomif/XML-LibXSLT-1.92/LibXSLT.pm#Parameters

_TEST PLAN_

1) Run "perl t/db_dependent/XSLT_Handler.t". If all tests pass,
you should be free to sign off. Feel free to inspect the last
test in XSLT_Handler.t and the XSL in test04.xsl to see how it
works.

If you really want to be thorough, you could write your own test
cases using mine as an example.

Alternatively, you could go into C4::XSLT, and try to pass a
value to a parameter in the search results or the detail page,
but that might be a bit over the top.

It's a pretty simple patch.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 12601: (RM followup) DBIx updates

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 12601: DBRev 3.19.00.008

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 12601: Add a foreign key for aqorders.budget_id

When creating an order, the fund value is mandatory but the DB structure
does not show this constraint.

The aqorders.budget_id field should be linked to aqbudgets.budget_id.

The updatedatabase.pl (entry 3.01.00.077) commented this constraint,
certainly for a retro compatibility reason.

Actually I found some cases (in production) where aqorders.budget_id is
set to "0". To add this constraint, we should add a temporary fund to
linked orphan orders.

Test plan:
0/ Verify it is not possible to create an order not linked to a fund via
the Koha interface.
1/ Using your SQL CLI (or equivalent), create or update 1+ orders and set
"0" in the budget_id field.
2/ Execute the updatedabase script.
3/ Verify that your order is linked to a new fund (which is linked to a
new budget).
4/ Verify the constraint has correctly been added (show create table
aqorders).

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

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

Bug 12905: DBRev 3.19.00.007

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 12860: the export tools explodes if a field is not exported

To reproduce:
1/ Go on tools/export.pl
2/ Export some records and specify you don't want the fields 245 (the
whole field, do not specify a subfield).
3/ Export, BOOM
Software error:
Arguments must be MARC::Field object at /home/koha/src/tools/export.pl line 400.

Test plan:
Apply the patch and confirm the fields 245 are not exported and the
export works as expected.

Signed-off-by: Robin Sheat <robin@catalyst.net.nz>

Export fails to asplode with this patch

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

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

Bug 13270: Don't display "vendor note" label if nothing to display

Bug 12111 removes the vendor note edition on receiving.
The label should not be displayed when it's empty.

Test plan:
1/ Receive an order without a vendor note and verify that the label is not
displayed.
2/ Receive an order with a vendor note and verify that the note is
displayed.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Works as described, small template change.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13441 - Branchcodes should not be allowed to have spaces in them

In multiple cases I've seen issues arise in Koha where a librarian
accidentally puts a space at the end of a new branchcode. This of course
causes endless confusion because the branchcode looks perfectly fine in
every case unless you wrap the code with some characters to reveal the
hidden space!

Test Plan:
1) Try creating a new branch with one or more spaces in the branchcode
2) Note you are able to
3) Apply this patch
4) Repeat step 1
5) Note you are no longer able to

Followed test plan with cache cleared. Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, passes tests and QA script.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 12905: funds with children could not be deleted

The interface should prevent to delete funds with children.
Otherwise the relationship is broken and problems occur:
1/ You don't see the orphan fund in the fund list
2/ You cannot edit the orphan fund amount ('Fund amount exceeds parent
allocation').

This patch:
- adds a JS check, template side
- adds a check in the perl script (should never be true)
- adds an updatedatabase check, in order to alert users with inconsistent data.

Test plan:
Verify you are not allow to delete a fund with children.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

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

Bug 13333: Fix Display basket group for already received orders

Bug 11111 adds a basket group column on the parcel page.
But it seems that the already received orders never contain the value
(always 'no basket group').

Test plan:
Receive an order which is in a basket group and verify the basket group
column is correctly filled.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

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

Bug 13457 - Followup for CPL and S codes

Based on comment #3, this corrects CPL and S issues if they do
not exist in the DB.

TEST PLAN
---------
0) Backup your DB.
1) Clear CPL and S from your DB.
   - delete from borrowers where categorycode='S';
   - delete from categories where categorycode='S';
   - delete from borrowers where branchcode='CPL';
   - delete from branch where branchcode='CPL';
2) prove t/db_dependent/Suggestions.t
   -- This should fail.
3) Apply patch
4) prove t/db_dependent/Suggestions.t
   -- This should work.
5) Intentionally add categorycode 'S' and branchcode 'CPL' back
   into the database.
6) prove t/db_dependent/Suggestions.t
   -- This should work.
7) run koha qa test tools.
8) Restore your DB :)

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13457: Suggestions.t expects sample itemtypes

TEST PLAN
---------
1) Make sure you have more than 8 item types, and preferably
   something with a non-sample default code for itemtypes.
2) prove t/db_dependent/Suggestions.t
   -- this will fail
3) Apply patch
4) prove t/db_dependent/Suggestions.t
   -- this will succeed
5) run koha qa test tools

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 10241 - Easy analyticals creates two 773 fields. Search/link from host to children is broken

Hidden 'New child record' if 'EasyAnalytics' is set to display.

Testing plan:

-Turn on 'EasyAnalytics'. Check the drop down menu from the records page
*The drop down menu should include 'Analyze items' and not include 'New child record'

-Turn off 'EasyAnalytics'. Again check the drop down menu from the records page
*The drop down menu should include 'New child record' and not inlucde 'Analyze items'

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13268: the size should not be emptied in pl script

It duplicates what the first patch does.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13268 - biblioitems.size value not correctly displayed (more)

Bug partially corrected by Bug 11357.

The size column in biblioitems is a bit problematic when used in TT, because instead of the size value from the biblio column it will give you the size of the variable or current loop.

It's currently used in the templates like opac-topissues.tt :
[% IF results_loo.size %][% results_loo.size %][% END %]

This patch corrects by using item() TT method.
See http://stackoverflow.com/questions/2311303/how-can-i-handle-hash-keys-containing-illegal-identifier-characters-in-template.

Test plan :
- Be sure there is a mapping between a MARC field and biblioitems.size
- Create a record A with biblioitems.size defined : like "10x12"
- Create a record B with no value in biblioitems.size
- Check each modified page :
=> Without this patch : you see a number (loop size) for both records
=> With this patch : you only see the correct value for A and nothing for B

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>