Commit graph

5120 commits

Author SHA1 Message Date
Jonathan Druart
a19958ee2c Bug 12969: Add a subroutine to calculate VAT and prices
This patch adds a new subroutine populate_order_with_prices in the
C4::Acquisition module.

Its goal is to refactore the VAT and prices calculation into Koha.
All scripts will use this subroutine.

Test plan:
Verify that the prices in t/Prices.t are consistent with the values
listed in the file "Prices and VAT calculation - before" submit on bug
12964.
Verify that
  prove t/Prices.t
returns green

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>
2015-01-04 12:45:18 -03:00
Jonathan Druart
936a4f2228 Bug 13504: Remove the '----' marker for CHECKIN and CHECKOUT notices
If only 1 item exist in the message, the marker is not removed.
This marker is removed by render_metadata, but this method is only
called on appending.

Test plan:
1/ Enable the CHECKIN and/or CHECKOUT notices for a patron
2/ check and item in or out and verify that the marker is no longer
displayed in the generated notices.

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>
2015-01-04 11:20:10 -03:00
352d9cd2fd Bug 13167 Stage MARC for Import hangs for biblio containing invalid ISBN-13
If the ISBN of a UNIMARC record begins with 979 then the 'Stage MARC for
import' hangs.  If I use the same UNIMARC record and change 979 to 978 in the
ISBN, 'Stage MARC for import' works perfectly.

The patch deals with the fact that converting an ISBN-13 to ISBN-10 with
Business::ISBN as_isbn10() method fails if the ISBN doesn't begin with 978.

TEST PLAN:

(1) Download, and decompress the ZIP file attached to BZ.
(2) On a UNIMARC Koha instance, go in Tools > Stage MARC for import.
(3) Choose the MARC file containing the record with an ISBN begining with 979.
    Click on Upload file, then Stage to import.
(4) The Job progress bar stay at 0%.
(5) Apply the patch. Repeat steps 2-3. The upload works.

Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Tested in a UNIMARC installation, confirmed that the patch fixes the
problem.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2015-01-04 11:15:24 -03:00
Jonathan Druart
469f36d38f Bug 12896: Move the bookseller-related code into Koha::Acquisition::Bookseller
The C4::Acquisition module should be exploded in order to add
readability and maintainability to this part of the code.

This patch is a POC, it introduces a new Koha::Acquisition::Bookseller module and put in
it the code from GetBookSeller and GetBookSellerFromId.

Test plan:
1/ Create a bookseller, modify it.
2/ Add contacts for this bookseller
3/ Create an order, receive it, transfer it
4/ Launch the prove command on all unit tests modified by this patch and
verify that all pass.

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>
2014-12-31 14:15:58 -03:00
Chris Cormack
426a48dd19 Bug 13502: Code introcduced in 1861 wrongly assumes a null userid is unique
To test

1/ Create a borrower with '' as their userid, you may have to edit a
   row in the db to do this
2/ Run  perl t/db_dependent/Circulation/CheckIfIssuedToPatron.t
3/ Notice some tests fail and you see
   DBD::mysql::st execute failed: Duplicate entry '' for key 'userid'
   at /home/chrisc/git/catalyst-koha/C4/SQLHelper.pm line 184.
4/ Apply the patch
5/ Run the tests again, notice they now pass

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-12-31 14:07:00 -03:00
Chris Cormack
a847067d7e Bug 1861: There is a problem introduced with an earlier patch, on this patchset
-    $data{'userid'} = Generate_Userid($data{'borrowernumber'},
$data{'firstname'}, $data{'surname'}) if $data{'userid'} eq '';
+    $data{'userid'} = Generate_Userid( $data{'borrowernumber'},
$data{'firstname'}, $data{'surname'} )
+      if ( $data{'userid'} eq '' || Check_Userid( $data{'userid'} ) );

Check_Userid returns 1 if it is unique.  So this means unique userids
will always be discarded and new ones created.

This is why all the tests depending on a userid are now failing

To test

1/ run perl t/db_dependent/Serials_2.t
2/ Notice lots of tests fail
3/ OR Add a borrower with a userid set, notice the userid is ignored
and one is generated instead
4/ Apply patch
5/ Add a new borrower, notice the userid sticks (if it is unique)
6/ Run perl t/db_dependent/Serials_2.t notice tests pass
7/ Run perl t/db_dependent/Members.t notice tests still pass

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

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-12-31 12:13:54 -03:00
Colin Campbell
ee02bcb14f Bug 13252 Allow for IPv6 formatted addresses in Port definition
The SIP config has allowed you to specify an interface ip as
part of the listeners/service/port attributei
 e. g.  as port="127.0.0.1:6001/tcp"
with IPv6 the equivalent would normally be
   as port="[::1]:5001/tcp"
However in this case incoming connections will get rejected because
Configuration constructs a string without the brackets
This patch makes tests both formats on incoming connections so that
they are accepted as they were previously

In future the best course is not to include a port identifier in the
port definition then if the server has ipv6 it will bind to all
interfaces and accept both IPv4 and IPv6 traffic

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-12-28 20:46:44 -03:00
Benjamin Rokseth
a5c9947f43 Bug 13252 - SIP2 server should accept IPv6 connections
This small patch adds a check on the SIP2 socket connection if it is
IPv6 and resolves socket address accordingly.

Any newer Debian distro would probably default to IPv6 so it would
eventually affect all SIP servers.

Tests against running SIP server on an IPv6 box:

http://wiki.koha-community.org/wiki/Koha_SIP2_server_setup#Testing_with_Telnet

before patch:
  disconnects immediately. Log output:
  Bad arg length for Socket::unpack_sockaddr_in, length is 28, should be 16

after patch:
  operates normally

Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-12-28 20:46:37 -03:00
Lyon3 Team
c1621de8a5 Bug 12895 repair dropbox mode
One day late patrons were restricted even with dropbox mode activated

1) Check in the calendar (Tools/Calendar), that the
   previous days you are about to use as date due are
   really entered as opening day (never know).
2) Add a suspension in the suspension days parameter
   of the circulation rules (Administration/Circulation
   and fine rules) to the MOST specific category of
   borrower and MOST specific type of document among the
   existing rules of the LOGGED IN Site(cf explications
   in the circ-rules page).
3) Choose a borrower using the search by category and an
   item through the advanced search using the limit by type.
4) Checkout the item selecting the previous opening date
   in the Specify-due-date box.
5) Click on Circulation in the upper menu, then on Checkin
   and check the Book drop mode. The Book drop date showed
   should be the previous opening date.
6) Check in the item : you can see that the patron is restricted
7) apply the patch
8) Redo 1 to 5 : Now, you can see that the patron is not restricted.
9) If you redo the test with two day late, you will see that
   the patron is not restricted : that's ok because his
   restriction of one day is already finished.
10) If you redo the test with more than two day late, you see
    that the patron restriction is, as expected, one day shorter
    than it were if the item had been returned without dropbox mode.

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-12-28 20:35:30 -03:00
fe4da2f721 Bug 13124 - Record titles with parentheses causing label weirdness
Test Plan ( using sample data included with Koha )
1) Catalog a record and item with the title "Oh no! or, (How my
   science project destroyed the world) /"
2) Edit the DEFAULT template
   a) Set layout type to Biblio
   b) Set data fields to "title, author, isbn, issn, itemtype,
      barcode, itemcallnumber"
   c) Set font size to 10
3) Create a batch with just the one item you created
4) Export the PDF with the Avery template and the DEFAULT layout
5) Note the weirdness
6) Apply this patch
7) Re-export the PDF, note it's no longer weird ; )

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-12-28 20:01:42 -03:00
Jonathan Druart
801ba4a920 Bug 13360: C4::Ris assumes that hash keys are ordered - KW
This patch only fixes the KW order.

Test plan:
1/ Choose/create a record with several 6XX (for KW), see the code source
to know which fields you can use
2/ Export this record in RIS format
3/ Verify that the KW lines are ordered following the marc record fields
order.

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

We really should refactor this whole thing into Koha::RIS sometime, it's
a horrible module at the moment.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-12-28 19:56:31 -03:00
7135364dd2 Bug 1861 [QA Followup] - Fix Check_Userid and unit tests
Check_Userid assumes that a borrowernumber will always be passed in
and thus fails to to return 0 for an already used userid when creating
a new patron.

Unit tests must now also me modified to no longer assume it is possible
to create multiple patrons with the same userid.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-12-28 19:50:49 -03:00
Katrin Fischer
22b8d559f9 Bug 13461 - Circ always asking for confirmation if RentalFeesCheckoutConfirmation is used
To test:
- Check RentalFeesCheckoutConfirmation is activated
- Try to check out an item without rental fine
- Verify confirmation message without explanation
  is shown
- Apply patch
- Verify confirmation message is no longer shown
- Configure itemtype to have rental fee
- Veirfy now the confirmation message appears as
  it should

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-12-19 17:04:06 -03:00
Jonathan Druart
fab96202fd Bug 13296: (follow-up) permit grep on AUTHUNIMARC
I would prefer not to hide this "stuff".

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-12-19 14:42:03 -03:00
9ccdbc49c7 Bug 13296 - error when using z3950 with UNIMARC authorities
When using a z3950 connexion with UNIMARC authorities, you get an error :
Unsupported UNIMARC character encoding [ ] for XML output for UNIMARCAUTH; 100$a -> 20141119

I've seen thant Bug 2060 when adds authorities import adds a special behavior for UNIMARC : marc flavor must be UNIMARCAUTH instead of just UNIMARC.

This patch adds the same behavior when using z3950 connexion and import.

Test plan :
 - Use a UNIMARC install
 - Define a z3950 connexion for UNIMARC authorities
 - Go to Authorities module
 - Click on "New from Z39.50"
 - Perform a search
=> Without patch : you get the error
=> With patch : you get results
 - Import one result
=> You get the authoritie creation form with all datas
You may check same plan with MARC21 install

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
NOTE: depending on the target, the syntax in the configuration
might not be UNIMARC, but MARC21/USMARC instead!

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-12-19 14:41:52 -03:00
Jonathan Druart
2f266d94be Bug 13465: Correct the field prefix ambiguity
This is introduced by Bug 12874.

Without this patch, it's not possible to clear (set to an empty string)
an item field.
This appended for field linked to an AV list but even if it's not.

The regex tried to prefix 'my_field' with 'items.' to have
'items.my_field'. It wanted to take care of the case where the prefix
already exists (Actually only 1: 'items.cn_source').
The regex is changed to: "add the prefix only if the string does not
contain a dot".

Moreover an ambiguity existed on the prefix: in marc_subfield_structure,
the kohafield is prefixed, but not in the key of the hash sent to
ModItemFromMarc.

Test plan:
- edit an item, set a status that is controlled by an authorized value
examples tested: damaged, not for loan
- check the status saved correctly
- edit the item again, reset the status to empty
- check the status saved correctly
- edit the item again, reset fields, edit fields
- check the fields saved correctly

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-12-19 14:40:21 -03:00
Jonathan Druart
bf681e28ba Bug 12059: Prefer to list fields in the query
To avoir further issue, it's better to explicitely list the fields we
want to retrieve.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-12-17 20:17:16 -03:00
Katrin Fischer
d9c99b6f5e Bug 12059: Publisher column on invoice page always empty
This patch moves the publisher information out of its own
always empty column into the Summary column below the title,
as it is on other acq pages.

The information was never displaying, as publishercode is in
biblioitems and that table was not selected by GetInvoiceDetails.

Also modified the code to take into account that UNIMARC uses
biblioitems.publicationyear and MARC21/NORMARC use bibio.copyrightdate
for the copyright year.

To test:
- create an invoice for records that
  - have a publication year
  - have no publication year
  - have a publisher...
- 'finish receiving' and check the invoice summary page
   ...acqui/invoice.pl?invoiceid=?
- Make sure all the information displays now but didn't witout the patch.

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

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-12-17 20:17:10 -03:00
Chris Cormack
ef6bc21b2c Bug 13368 Holds priority messed up on checkout
To Test

1/ Create 3 (or more holds) on one biblionumber, make sure at least
one item is not on loan
2/ Check out the not on loan item to a borrower (maybe number 2 in the
queue)
3/ Look in the database (or on the holds tab on the moredetail.pl)
notice the priorities have not been reordered
4/ Apply patch and try again, notice now they have

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Confirmed the problem without the patch, and confirmed that the patch
corrects it.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-12-17 19:59:44 -03:00
Jonathan Druart
46e3f8169c Bug 12980: GetHistory does useless processing
GetHistory iterated on the orders to calculate the quantity and price.
These values are never used by the called.
It can be removed.

Test plan:
Verify there is no regression on acqui/histsearch.pl and
catalogue/detail.pl
Actually you just have to check that the total quantity and price are
not displayed on these views.

QA: note that 'count' and 'toggle' are never used in the template.

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>
2014-12-03 11:42:48 -03:00
85c25c619f Bug 9165: (Followup) Tidied code slightly
Minor code tidy to clean up qa script warning.

http://bugs.koha-community.org/show_bug.cgi?id=9165
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-27 14:58:46 -03:00
7b9082a55b Bug 9165: (Followup) Clear existing sync
A small enhancement to clear existing synced passowrd should this
config option be enbled. This followup is related to bug 12831

http://bugs.koha-community.org/show_bug.cgi?id=9165
Signed-off-by: Robin Sheat <robin@catalyst.net.nz>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-27 14:58:16 -03:00
Robin Sheat
519149a6c7 Bug 9165: Prevent LDAP passwords being stored locally
This adds a configuration option to LDAP that prevents it from storing
user's passwords in the local database. This is useful when users of
hosted Koha wish to prevent any form of offsite password storage for
security reasons.

Notes:
 * if the option is not included in the koha-conf.xml file, then the
   current default behaviour of saving the password locally is retained.
 * this has no impact on passwords that are already in the database.
   They will not be erased.

To use:
 * edit the koha-conf.xml for a system that uses LDAP for
   authentication.
 * in the <ldapserver> configuration, add:
   <update_password>0</update_password>
 * feel a greater sense of security.

To test:
 1) have a Koha system that authenticates using LDAP.
 2) note that when a user logs in, their password is saved (hashed) in
    the database.
 2.5) it is important to note that, for whatever reason, a user's
      password is not stored on a login where their account is created,
      only when they log in after being created. Thus perhaps log in and
      log out a couple of times to be sure.
 3) add the <update_password>0</update_password> option to the
    <ldapserver> section of koha-conf.xml.
 4) login with a new user (or erase the password from the database for
    an existing user) and note that the password field is not populated.
 5) log out and log back in just to be sure, check the password field
    again.

Sponsored-By: National Institute of Water and Atmospheric Research (NIWA)
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-27 14:57:49 -03:00
4c1f0dcecb Bug 12831: Local Only logins with LDAP
Local only logins should continue to function when LDAP is enabled.
This was not the case after bug 8148 [LDAP Auth should FAIL when ldap
contains a NEW password].  For this case, we need to diferentiate
between local accounts and ldap accounts.  This is somewhat challenging
and thus this patch is only part of the story.

The other half can be achieved with bug 9165

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-27 14:46:27 -03:00
Jonathan Druart
3b6b8a4a1e Bug 13215: Fix GetLetterTemplates should return default templates if branchcode is not defined
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-27 11:42:26 -03:00
Jonathan Druart
6f599652b1 Bug 13215: The same letter code can be used for several libraries
This patch fixes a major issue introduced by the
commit 5c4fdcf Bug 11742: A letter code should be unique.

The interface should let the possibility to create a default template
letter and some specific ones, with the same letter code (letter.code).

The patches submitted on bug 11742 tried to fix an issue based on a
(very bad) assumption: letter.code should be considered as a primary key and
should be uniq.

This patch reintroduces this behavior.
Note that the interface will block a letter code used in different
module (this is consistent not to have the same letter code used for different
needs).

This patch is absolutely not perfect, it just tries to change as less
change as possible and to use new tested subroutines.

Test plan:
1/ Verify that the problem raised on bug 11742 does not appears anymore.
2/ Verify there are no regression on adding, editing, copying, deleting
letters.
3/ Verify you are allowed to create a default letter template with a letter
code and to reuse for a specific letter (i.e. for a given library).

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>
2014-11-27 11:42:14 -03:00
Jonathan Druart
b141e6d42a Bug 13332: Fix conflict between 5304 and 10860
These 2 bugs are in conflict.
The first one always join the issue table, the second one join on this
table too if the OnSiteCheckouts pref is enable.
So DBI raises an error if the pref is enabled (2 joins on the same
table).

This patch removes the conditional join.

Test plan:
Go on a detail record page with items and verify that items are list and
that the error no more appears in the log file.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Reproduced the problem, the patch fixes it, no noticeable regression found.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, items are visible again.
Passes tests and QA script.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-26 11:25:04 -03:00
e3fe0bdb31 Bug 13152 - Duplicate phone hold notices when using Talking Tech
If a library is using Talking Tech for phone notices, any waiting hold
phone notice will show up twice!

This is because Koha generates on at the time the hold is set to
waiting, and then the cronjob TalkingTech_itiva_outbound.pl generates
it's own notice as well.

The former notice will always have a status of 'pending', as the
TalkingTech_itiva_inbound.pl script will update the notice the outbound
script created.

The solution is to prevent Koha from creating a phone notice for waiting
holds if TT is enabled, and let the cron script do it.

Test Plan:
1) Enable Talking Tech from the system preferences
2) Set a hold waiting phone notice in the notices and slips editor
3) Choose a patron, enable hold phone notices for that patron
4) Place a hold for a patron, and check it in so it's marked as waiting
5) Note the phone notice generated for the patron
6) Apply this patch
7) Repeat step 4
8) Note that this time, a phone hold waiting notice is not generated

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Amends condition with an additional or statement. Shoudn't affect
anything but phone notices. Change appears logical.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-25 17:33:56 -03:00
Marc Véron
a7a757ba85 Bug 12865 - 'Pay amount toward all fines' does not record payment note
Without patch:
-------------
Make payment for patron who has fines
Select the Pay Amount button and add a note in the note box.
Select confirm
Result: The note does not display in list

With patch:
----------
Result: The note displays in list
Bonus testing: The note is included in system logs as well (Home:Tools:Logs)

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-25 16:38:46 -03:00
Jonathan Druart
8601fe6541 Bug 12851: order tags should be removed from the claiming letter
If you use a claimissue notice to claim serials, the generated letter
will be

<order>Title1, Author1</order>
<order>Title2, Author2</order>
...
<order>TitleN, AuthorN</order>

This patch geds rid of these tags.

Test plan:
1/ Create a claimissue notice with something like:
  <<LibrarianFirstname>>
  <<LibrarianSurname>>
  The following issues are in late:
  <order><<biblio.title>>, <<biblio.author>> (<<biblio.serial>>)</order>

2/ Generated late serial issues.
3/ Send notifications to vendor.
4/ The order tags should not exist anymore in the sent email.

You can see bug 5342 for a more detailled test plan.

Note for QA: This should have been done in GetPreparedLetter, but I did
not find a better way to do.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described. Tested having the <order> tags on one line
and also for a multi-line layout.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-25 16:01:37 -03:00
532b41934c Bug 13157: (QA followup) homebranch is 995$b on UNIMARC frameworks
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
2014-11-25 15:27:12 -03:00
9ebb6ba5d1 Bug 13157: UNIMARC holdingbranch facet is 995$c not 995$b
Fix a typo. Not test plan required, just a look at default UNIMARC framework.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
2014-11-25 15:27:05 -03:00
4c1ae53d86 Bug 13297 - Shelving location PROC does not work according to manual
According to the manual, "Items will stay in the PROC location until
they are checked in".

This is not the actual behavior. Right now items will only change from
PROC to CART, and that is only if InProcessingToShelvingCart is enabled.
Some libraries want to use the PROC to permanent location feature,
without using the CART.

Additionally, the location is only removed if using returns.pl, but
that is not what the manual says either. What if the library uses
SIP2 devices for handling returns? This should be taken into
account.

Test Plan:
1) Apply this patch
2) Set an item's current location to PROC, and it's permananet location
   to a different location.
3) Check the item in any way you wish
4) Note the shelving location is updated to the permanent location
5) prove t/db_dependent/Circulation/Returns.t

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

I tested this with items which had items.location set to 'PROC' and
items.permanent_location set to NULL, '', and a real value, and it
worked correctly in all cases. I tested with check-ins from returns.pl
and from the table of checkouts in circulation and the PROC location was
correctly removed in both cases.

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>
2014-11-23 10:11:28 -03:00
8fd15d90bf Bug 13053 - Do not use template cache when from commandline
You may define in config a folder (usually /tmp) for TT cache :
template_cache_dir in etc/koha-conf.xml.

Some perl pages may be launched from commandline, like tools/export.pl.
Also, the script gather_print_notices.pl uses C4::Templates.

The problem is that when script is launched from Apache, the Unix owner of cache files will be www-data. When script is launched from commandline, like in a cronjob, the Unix owner will be different (like a user named "koha"), causing a crash because cache files can only be read by its owner.

This script disables the template cache if perl script is called from commandline. This cache is certainly only useful for web access.
Using GATEWAY_INTERFACE env var comes from tools/export.pl

Test plan :
- Use a dev install of koha installed in a user home, ie "/home/kohadev"
- Define a folder for template_cache_dir in etc/koha-conf.xml. For example : <template_cache_dir>/tmp</template_cache_dir>
- Check there is no cached templates already in this forder
- Create a file "bib.list" containing a few existing biblionumbers
- As user kohadev, launch : tools/export.pl --record-type=bibs --id_list_file=bib.list
- Look at cache folder
=> Without patch you see cache files owned by user kohadev
=> With patch there are no cache files
- Use the Koha interfaces OPAC and Intranet
=> Without patch you get an error : Template process failed: file error - cache failed to write ...
=> With patch you have no error and cache files are generated with Apache user as owner

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, good test plan!
Passes tests and QA script.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-21 20:56:05 -03:00
Srdjan
421761e274 Bug 5304: GetItemsInfo() - moved issues and serials query from the results loop to the main query
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
No commit message
No test plan.

No regressions found on opac/staff item display
No improvements either, but could be just my test data
No koha-qa errors

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Passes tests and QA script.
Tested detail and item pages in OPAC and staff, no regressions found.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-21 20:45:01 -03:00
Jonathan Druart
2599cd17c7 Bug 12415: Fee receipt: a charges description should be displayed in all cases
Bug 2546 removes the description DB field value in some case (3.15.00.003).
But the receipt generated by scripts members/printfeercpt.pl and
members/printinvoice.pl displays this field.
When the description field is empty, the default value (based on
accountlines.accounttype) should be displayed.

Test plan:
- Generate and pay some different kinds of fees for a patron without
  filling the 'description' field.
- In Fines>Account, click on the 'print' link.
- Before this patch, the "description of charges" values is empty if no
  description was given.
  It is a regression introduced by bug 2546, a default value was
  inserted in the description field depending on the account type
  selected.
- After this patch, the "description of charges" values should be based
  on the account type. The string display on printing receipt should be
  the same as on the account screen (staff and opac).

Note for QA: If removed the "payment" key, it is not used in template
and generated a warning ("odd number of elements...").

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
This fixes the display of payments and other charges on the
fines slip.

Note: This patch fixes a line where the description in the
database was still updated to say "Payment thanks" for partial
payments. It might be worth to do a follow-up correcting the
accountlines table and removing the unwanted comment (see bug 2546).

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-21 20:14:02 -03:00
1993b3090c Bug 13050: Follow-up for bug 12371
This patch simplifies the SQL query in Letters.pm for table
borrower_modifications.
It also addresses the only case this query is used in opac-memberentry.
An unused variable in Letters.pm is removed.

Test plan:
Enable selfregistration on opac.
Set verification by email to required in prefs too.
Self-register two new users.
Check the email notices generated.
Verify the new users with the tokens in their notice.

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

Much cleaner SQL

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Cleaner and works as described, no regressions found.
Passes tests and QA script.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-21 15:29:43 -03:00
66353372cf Bug 12909 - item withdrawn is missing in inventory results
In inventory results, CSV or screen, the item withdrawn information is missing.
This information can be usefull to understand why an item was not scanned.

Test plan :
- Check you have in default framework an item subfield mapped with items.withdrawn
- Create a biblio with default framework
- Create an item with barcode='000AAA1', callnumber='ZZZAAA1' and withdrawn=0
- Create an item with barcode='000AAA2', callnumber='ZZZAAA2' and withdrawn=1
- Go to inventory tool : /cgi-bin/koha/tools/inventory.pl
- Enter item callnumber between 'ZZZ' and 'ZZZZ'
- Submit
=> You see a column 'Withdrawn' with withdrawn value
- Go to inventory tool : /cgi-bin/koha/tools/inventory.pl
- Enter item callnumber between 'ZZZ' and 'ZZZZ'
- Check 'Export to CSV file'
- Submit
- Open exported file
=> You see a column 'Withdrawn' with withdrawn value

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-21 15:27:15 -03:00
simith
819dc6d3d7 Bug 12505 - Variable aqorders.listprice in acq claim notice doesn't work
Modified:

C4/Letters.pm               - remove aqbooksellers.* from SELECT statement

In Letters - SendAlerts subrotine, is safe to remove aqbooksellers.* from SELECT statement
for type=claimacquisition or claimissues. Aqbooksellers is passed to GetPreparedLetter subrotine in tables variable.

Testing:

I Apply the patch

Select Tools -> Notices and slips;
Edit ACQCLAIM;
Add :
<order>Ordernumber <<aqorders.ordernumber>> (<<biblio.title>>) (<<aqorders.quantity>> ordered) ($<<aqorders.listprice>>  <<aqbooksellers.listprice>> each) has not been received.</order>
Save modifications;
Create a vendor (Acquisition module);
Create an order (Acquisition module);
Click Acquisitions -> Late orders;
Select the order created;
Click Claim order button;
Valide <<aqorders.listprice>>;
Valide <<aqbooksellers.listprice>>.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described. It's now possible to output the actual price
in the claim notice.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-19 21:43:29 -03:00
simith
268fc21685 Bug 12505 - Acq claim: Show error message when no order is selected
If no order is selected on the acq claim page when clicking
'Claim order' an ugly perl error message is displayed.

This patch corrects the behaviour to display a human readable
'No order selected'
instead.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Reworded commit message to reflect what the patch achieves.
Works as described and passes tests.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-19 21:43:23 -03:00
Marc Véron
955e836c9b Bug 13261 - Better check in message for patrons with indefinite restricition
This patch adds a better check in message for patrons with indefinite restriction.

To test:
Check out an item to a patron.
Add a manual restriction without expiry date to that patron.
Check in the item.

Without patch, the checkin message reads:
Reminder: Patron was earlier restricted until 9999-12-31

Apply patch and repeat steps above.
The message should now read:
Reminder: Patron has a restriction (no expiry date)

NOTE: Changed wording at two places following Owen's  suggestion.  New: "Patron
has an indefinite restriction"

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

Thanks Marc for catching this case. I was thinking like you that the wording
sounded strange while playing with bug 13242. Merge the original patch and the
followup, containing a better wording, thanks to Owen comment.

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

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-19 21:41:24 -03:00
Olli-Antti Kivilahti
15613a0294 Bug 13168 - "Today's checkouts" sort improperly because issuedate lacks seconds.
TO REPLICATE:

Prepare a bunch of Items (6+) for checking out, or have a set of barcodes ready for copy-pasting.
Check-out those items quickly within one minute and observe that the sorting order is not always from the first checkout to the last.

This is because the issuedate doesn't have seconds defined.

AFTER THIS

The bunch of Items is sorted properly.

Tiny patch, works as expected. Passed QA script.
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>
2014-11-16 21:24:28 -03:00
Jonathan Druart
b969aa3fa3 Bug 11413: Fix field_numbers
This fix is a global fix for the MarcModificationTemplate feature.
Some unit tests were missing and some behaviors were wrong.
For instance, if you tried to update a non existent field, the script
crashed.

The following line was completely stupid:
    if $from_field ne $to_subfield

The field_number equals 1 if the user wants to update the first field
and 0 for all fields.

The field_numbers (note the s) variable contains the field numbers to
update. This array is filled if a condition exists (field exists or
field equals).

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-14 12:05:47 -03:00
Jonathan Druart
f76b5c8141 Bug 11413: Fix return for ModifyRecordWithTemplate
Make sure the ModifyRecordWithTemplate routine returns undef.

This patch also removes a warning if GetModificationTemplates is called
without parameter.

Verify
  prove t/db_dependent/MarcModificationTemplates.t
returns green.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-14 12:05:46 -03:00
Jonathan Druart
6d2e5f58f2 Bug 11413: Change the field number logic
This patch series is a bugfix for the Marc modification templates tool.

Bug description:
If you want to do an action (delete/update/move/...) on a multivalued
field and if a condition is defined on the same field, it is highly
probable the resulted record will not be what you expect.

For example:
Deleting All (or the first) fields 650 if 245$a="Bad title" works with
the current code.
BUT if you want to delete All (or the first) fields 650 with a condition
on 650$9=42, and if at least one field matches the condition :
- if you have selected all, all fields 650 will be deleted, even the
  ones who do not match the condition.
- if you have selected first, the first 650 field will be deleted, even
  if it does not match the condition.
The expected behavior is to delete the fields matching the
condition (and not all the 650 fields).

What this patch does:
This patch introduces 2 changes in the logic of Koha::SimpleMARC.
The first change is a change of the prototypes for the 2 routines
field_exists and field_equals. Now they return the "field number" of the
matching fields.
The second change is the type of the "n" parameter for all routines
using it in Koha::SimpleMARC. Before this patch, the "n" parameter was a
boolean in most cases. If 0, the action was done on all fields, if 1
on the first one only. Now it is possible to specify the "field numbers"
(so the array of field numbers which is returned by field_exists or
field_equals) for all routines which had the n parameter.

Test plan for the patch series:
Note: This test plan describes a specific example, feel free to create
your own one.
0/ Define a marc modification template with the following action:
  Delete field 245 if 245$9 = 42
1/ choose and export a record with several 245 fields.
For ex:
  245
    $a The art of computer programming
    $c Donald E. Knuth.
    $9 41
  245
    $a Bad title
    $c Bad author
    $9 42
2/ import it using the Stage MARC for import tool.
3/ verify the imported record does not contain any 245 field.
4/ apply all the patches from this bug report
5/ do again steps 2 and 3
6/ verify the imported record contains only one 245 field, the one with
245$9=41
7/ verify the unit tests passed:
  prove t/SimpleMARC.t
  prove t/db_dependent/MarcModificationTemplates.t

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

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-14 12:05:40 -03:00
Jonathan Druart
57dff1e63f Bug 13226: 9999-12-31 should not considered as a valid date
DateTime::Format::DateParse (called in Koha::DateUtils::dt_from_string)
does not manage to parse 9999-12-31 if a time zone is given.

my $date = DateTime->new(year => 9999, month => 12, day => 31);
 => OK

DateTime::Format::DateParse->parse_datetime( '9999-12-31' );
 => OK

DateTime::Format::DateParse->parse_datetime( '9999-12-31',
 'America/Los_Angeles' );
 => KO (~20sec on my laptop)

It should not be considered as a valid date when the letter is parsed.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Note that to reproduce the problem you much be checking in items from an
account which has been restricted indefinitely (either manually or by
the overdues process). With this patch such checkins go from taking
around 50 seconds (in my test system) to around 7 to 10 seconds.

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

Good catch! Works as described, no problems found.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-14 11:01:27 -03:00
Magnus Enger
1be9f1a0e7 Bug 11401: QA followup
1) Be more careful when checking the NorwegianPatronDBEnable syspref.

Before:
if ( C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {

After:
if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {

This should avoid complaints if the syspref is not initialized.

2) Fix some empty =head2 POD sections

3) Fix some indentation in patrons.pref, to make xt/yaml_valid.t happy

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
I couldn't find any regressions with adding, editing and deleting members.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-14 09:42:45 -03:00
Magnus Enger
b1d1a034b2 Bug 11401: QA followup - fix pod
The QA script was complaining about some dodgy POD in C4/Members.pm,
that was not introduced by bug 11401. This patch fixes the POD, to
keep the QA script happy.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-14 09:42:38 -03:00
Magnus Enger
290341d8db Bug 11401: Add support for Norwegian national library card
This patch makes it possible to sync patron data between Koha and the
Norwegian national patron database, in both directions.

In order to use this, the following information is necessary:
- a username/password from the Norwegian national database of libraries
  ("Base Bibliotek"), available to all Norwegian libraries
- a special key in order to decrypt and encrypt PIN-codes/passwords,
  which is only available to Norwegian library system vendors
- a norwegian library vendor username/password

See http://www.lanekortet.no/ for more information (in Norwegian).

While this is of course an implementation of a specific synchronization scheme
for borrower data, attempts have been made to prepare the ground for other sync
schemes that might be implemented later. Especially the structure of the new
borrower_sync table might be reviewed with an eye to how it might fit other
schemes.

To test:

Since the password and cryptographic key needed to use this functionality
is only available to Norwegian library system vendors, only regression testing
can be done on the submitted code. Suggested things to check:

- Apply the patch and make sure the database update is done. This should add
  the new "borrower_sync" table and five new systmpreferences under the
  "Patrons" > "Norwegian patron database" category:
  - NorwegianPatronDBEnable
  - NorwegianPatronDBEndpoint
  - NorwegianPatronDBUsername
  - NorwegianPatronDBPassword
  - NorwegianPatronDBSearchNLAfterLocalHit
- Check that patrons can be created, edited and deleted as usual, when
  NorwegianPatronDBEnable is set to "Disable"
- Check that the new tests in t/NorwegianPatronDB.pm run ok, e.g. on a
  gitified setup:
  $ sudo koha-shell -c "PERL5LIB=/path/to/kohaclone prove -v t/NorwegianPatronDB.t" instancename
- Check that all the other tests still run ok
- Check that the POD in the new files itroduced by this patch looks ok:
  - Koha/NorwegianPatronDB.pm
  - members/nl-search.pl
  - misc/cronjobs/nl-sync-from-koha.pl
  - misc/cronjobs/nl-sync-to-koha.pl
  - t/NorwegianPatronDB.t

Sponsored-by: Oslo Public Library

Update 2014-09-18:
- Rebase on master
- Split out changes to Koha::Schema
- Incorporate new way of authenticating with NL

Update 2014-10-21:
- Rebase on master
- Use Module::Load to load Koha::NorwegianPatronDB in non-NL-specific
  scripts and modules
- Fix the version number of Digest::SHA
- Fix a missing semicolon in kohastructure.sql

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>
2014-11-14 09:42:23 -03:00
Mark Tompsett
878fa77c30 Bug 13075: Silence warnings and improve Charset testing.
Calls to C4/Charset.pm's NormalizeString function with an
undefined string were triggering warnings when running:
  prove -v t/db_dependent/Holds.t

Sadly, t/Charset.t was also lacking calls to NormalizeString.

TEST PLAN
---------
1) prove -v t/db_dependent/Holds.t
   -- This should generate the uninitialized string warnings.
      Make sure CPL and MPL are in your branches to save
      yourself from headaches due to expected data.
2) cat t/Charset.t
   -- note there are no function calls to NormalizeString.
      You can see other shortfalls in the tests beyond
      NormalizeString with: grep ^sub C4/Charset.pm
3) prove -v t/Charset.t
4) Apply patch
5) prove -v t/Charset.t
   -- Run as before with more tests.
6) cat t/Charset.t
   -- note there are now function calls to NormalizeString.
7) prove -v t/db_dependent/Holds.t
   -- Nice and clean run! :)
8) koha-qa.pl -v 2 -c 1
   -- all should be Ok.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-14 09:35:44 -03:00