Bug 10917: allow staff to update hold pickup location without modify_holds_priority

A librarian cannot modify a hold's pickup location unless he or she has
the permission modify_holds_priority. This appears to be a bug and not
by design. The reason the update fails is due to the priority not being
passed to ModReserve. The priority is not displayed when a librarian
does not have the modify_holds_priority permission.

Test Plan:
1) Remove the modify_holds_priority from your logged in user
   * User cannot be a superlibrarian
2) Attempt to change the pickup location for a hold
3) Note the change fails
4) Apply this patch
5) Repeat step 2
6) Note the change succeeds

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Template change only.
Passes all tests and QA script.

Tested:
- Confirmed that changing the pickup location is not
  possible in master without the modify_holds_priority
  permission
- Confirmed that the patch fixes the problem - pickup
  location can now be changed with and without the
  permission
- Pickup location can still not be changed, when
  IndependentBranches is activated

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 9611: (follow-up) move new password hashing routines to separate module

The move avoids a problem where many modules would gain
a dependency on C4::Auth just because C4::Members needs access
to hash_password().

This patch also adds a couple unit tests for the new password
hashing code.

To test:

[1] Verify that there are no regressions on the test plan for bug
    9611.
[2] Verify that t/AuthUtils.t and t/db_dependent/Auth.t pass.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10685: remove inappropriate uses of finish in C4::Accounts.pm

This patch gets rid of finish.

From the man page

finish()
Indicate that no more data will be fetched from this statement handle
before it is either executed again or destroyed.
You almost certainly do not need to call this method.

Adding calls to "finish" after loop that fetches all rows is a common
mistake, don't do it, it can mask genuine problems like uncaught fetch errors.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment: Similar to other patches from the same author.
Run prove t/db_dependent/Accounts.t without errors
No koha-qa errors

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10682: remove inappropriate uses of finish() from C4::Reserves

This patch gets rid of finish.

From the man page

finish()
Indicate that no more data will be fetched from this statement handle
before it is either executed again or destroyed.
You almost certainly do not need to call this method.

Adding calls to "finish" after loop that fetches all rows is a common
mistake, don't do it, it can mask genuine problems like uncaught fetch errors.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment: Similar to other patches from the same author
I run prove t/db_dependent/Reserves.t without errors
don't know if more tests are needed.
No koha-qa errors

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10681: remove inappropriate uses of finish() from C4::Circulation

This patch gets rid of finish().

From the man page

finish()
Indicate that no more data will be fetched from this statement handle
before it is either executed again or destroyed.
You almost certainly do not need to call this method.

Adding calls to "finish" after loop that fetches all rows is a common
mistake, don't do it, it can mask genuine problems like uncaught fetch errors.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10711: make GetOfflineOperation() return $sth->fetchrow_hashref instead of $result

To test :
prove t/db_dependent/Circulation_OfflineOperation.t
t/db_dependent/Circulation_OfflineOperation.t .. ok
All tests successful.
Files=1, Tests=7, 19 wallclock secs ( 0.02 usr  0.01 sys +  0.24 cusr  0.02 csys =  0.29 CPU)
Result: PASS

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment: applied 10681 and 10710 before 10711
prove t/db_dependent/Circulation_OfflineOperation.t run without errors
koha-qa reports only 2 tabulations, fixed in a follow-up

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10711: (follow-up) removing tabs

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10698: give C4::Circulation::DeleteTransfer() a return value

This patch adds return values to DeleteTransfer:
Undef if no parameters are given
1 if a Transfer is deleted
0E0 if a wrong parameter is given

It also fixes some unit tests in t/db_dependent/Circulation_transfers.t

To test:
prove t/db_dependent/Circulation_transfers.t
t/db_dependent/Circulation_transfers.t .. ok
All tests successful.
Files=1, Tests=14, 20 wallclock secs ( 0.03 usr  0.00 sys +  0.39 cusr  0.02 csys =  0.44 CPU)
Result: PASS

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment: Applied 10681 and 10692 before 10698
Run prove t/db_dependent/Circulation_transfers.t without errors
No koha-qa errors on all 3 patches

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10698: (follow-up) remove now-superfluous FIXME comment

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10683: add unit tests for CRUD routines in C4::Circulation

This patch adds some unit tests wrapped in a transaction for
C4::Circulation.pm.

Circulation_Branch.t adds tests for routines which deal with
branch_item_rules,branch_borrower_circ_rules,
default_branch_circ_rules, default_circ_rules, and
 default_branch_item_rules in the database.

Circulation_issue.t adds tests for routines which deal with accountline
and issues in the database.

NOTE: Some commented tests have to be fixed, and some tests can be added.
More, other routines of Circulation.pm are tested in the patches:

10692	UT: Routines about transfers in Circulation.pm need unit tests
10710	UT : OfflineOperation's routines in C4/Circulation.t need unit tests
10767   UT: Routines which interact with the table issuingrules in C4/Circulation need unit test

Test plan:
prove t/db_dependent/Circulation_issue.t
t/db_dependent/Circulation_issue.t .. ok
All tests successful.
Files=1, Tests=16,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.40 cusr  0.02 csys =  0.44 CPU)
Result: PASS

prove t/db_dependent/Circulation_Branch.t
t/db_dependent/Circulation_Branch.t .. ok
All tests successful.
Files=1, Tests=10,  2 wallclock secs ( 0.06 usr  0.00 sys +  1.02 cusr  0.06 csys =  1.14 CPU)
Result: PASS

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

Comment: Well, I don't know exactly what to do with this, so
I left it to QA

a) prove t/db_dependent/Circulation_Branch.t works well and without erros

b) prove t/db_dependent/Circulation_issue.t works without errors for me
ONLY if I have a issuingrule for All, All with 1 as renewals allowed, in
other cases it fails.

No koha-qa errors

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10683: (follow-up) improvements to the unit tests

The borrowers table needs to be cleared after the items table
(last_returned_by column).

Some checks were missing for GetRenewCount and AddRenewal.

Now the tests simulated a renewal for a item and check that the renews
left is decremented.

Moreover the issuingrules tables should be cleared and filled with known
values.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10795: Improvements for GetOpenIssue in C4::Circulation

This patch adds some improvements for the routine GetOpenIssue().
Now, it verifies if the parameter is given (if not it returns undef)
and it returns $sth->fetchrow_hashref() instead of a $issue.

To test:
prove t/db_dependent/Circulation_issue.t
t/db_dependent/Circulation_issue.t .. ok
All tests successful.
Files=1, Tests=16,  2 wallclock secs ( 0.06 usr  0.01 sys +  1.09 cusr  0.07 csys =  1.23 CPU)
Result: PASS

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

Comment: Same situation as the one noted in comment of
Bug 10683, test fails unless there is an issuingrule
All, All with 1 as renewals allowed.

With that condition, it succeeds

No koha-qa errors

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10641 - GetBooksellerWithLateOrders in C4::Bookseller.pm has some incoherences

This patch fixes some incoherences of the routine
GetBooksellerWithOrders().

Now it considers the field $estimateddeliverydateto and it replaces it
by now() only if it is undef.

Also, it doesn't test if $aqbookseller.deliverytime is not Null anymore
but if $deliverytime = null or undef, it replaces it by 0.

It also verifies if $delay is >= 0 and return undef if it is a negative
value.

To Test:
Before, this routine sorts out the BookSellerWithLateOrders. If a
bookseller did not specify a deliverytime, it would never appears in
the list of LateOrders.  Moreover, if the field "Estimated delivery
date to" was specified, it didn't take care of the value and it
returns the late order up to today's date.

Now, the returned list considers all the fields give and if the
delivery time of the bookseller is not specified, it calculates the
late orders as if the deliverytime is 0.  By default, all booksellers
which have orders in late until today are listed unless "estimated
delivery date to" is specified.

prove t/db_dependent/Bookseller.t
t/db_dependent/Bookseller.t ..
[Some warnings about uninitialized values]
WARNING: GetBooksellerWithLateOrders is called with a negative value at C4/Bookseller.pm line 135.
t/db_dependent/Bookseller.t .. ok
All tests successful.

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests and QA script pass.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 11012 - update package dependencies for koha-perldeps

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10794: fix sorting on billing date column in invoices table

This patch adds the sorting by title string option to the table of
invoices. This allows column data to be sorted based on the
ISO-formatted date rather than the formatted-for-display date. A "blank"
(0000-00-00) date is added to cells which contain no date data.

To test, view the Acquisitions Invoices page (acqui/invoices.pl) and
confirm that the "Billing date" column is sorted correctly regardless of
the dateformat system preference.

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Template only change, passes all tests and QA script.
Sorting of the billing column now works as expected.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 9747: Fix NSB/NSE sorting issues on z3950 search results

At least the BNF server returns results containing non-sorting
characters (NSB/NSE).
In order to sort results according these characters, this patch adds a
new Datatable function.

Test plan:
- search 'tintin' on the z3950 search (cataloguing/z3950_search.pl)
- sort on title (default sort) and check that results are not well
  sorted.
- apply this patch
- do the same search and check that the first result is "Hergé. Les
  Aventures de Tintin..."
  The value of the cell is:
      <td>Hergé. Les Aventures de Tintin...</td>

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

Works as advertised and doesn't break existing searching

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 9747: sorting z3950 search results in acquisition

Adds NSB/NSE sort on z3950 seach results in acquisition module.
Also adds default sorting on title column like in cataloguing module.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10430 - status filter not working in serial claims when translated

With a translated intranet (ie fr-FR) the status filter does not work
for "Late" status.  It is because status in combobox filter is
translated "Retard" and status in table is translated "En retard".

This patch changed javascript filter to work on a status code instead
of status name.
The new classes may be used to change CSS depending on status.

Test plan :
- Use a translated intranet (ie fr-FR)
- Go to serials claim of a vendor with issues of multiple status
- Check that status filter does its work

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Patch passes all tests and QA script.
The status filter should probably be changed to only
allow filtering on status that can appear on the page.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10430: arrived and stopped status are not possible in serial claim

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10624: add 'status' option switch for the packages init script

This patch makes the init script return the status of the relevant
processes.

To test:
- Apply the patch, build package and install
- Run
 $ service koha-common status

Note: it can be tested just copying the debian/koha-common.init script
to a server running koha-common instances and calling it

 $ ./koha-common.init status

Sponsored-by: Universidad Nacional de Cordoba
Signed-off-by: Robin Sheat <robin@catalyst.net.nz>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>