Commit graph

4679 commits

Author SHA1 Message Date
Galen Charlton
b324e893e8 Bug 11630: (follow-up) tighten up code
This patch tightens up the initialization of the $restriction_age
variable and uses $bibvalues throughout.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-16 15:32:29 +00:00
31995cbcd1 Bug 11630: (QA Followup) Move code to subroutine, add unit tests
Patch behaves as expected.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-16 14:55:08 +00:00
Pasi Kallinen
19badb2db4 Bug 11630: improve parsing of age restriction markers
This patch makes the parsing of AgeRestrictionMarker values consider
the case where the marker is immediately followed by the age, e.g.
"K16" in Finland.

How I tested:

[1] Configure Age Restricition (see Syspref AgeRestrictionMarker) and
have a biblio record with e.g. PEGI 99 in age restriction field.
[2] Try to check out to a patron with age < 99
[3] Check out should be blocked
[4] Change entry in age restriction field to PEGI99
[5] Checkout should be possible
[6] Apply patch
[7] Checkout schould now be blocked

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-16 14:43:30 +00:00
Galen Charlton
cc7a99ba7f Bug 11797: (follow-up) fix MARC21 branch of bug more idiomatically
This patch puts the MARC21 variant of the bugfix in alignment
with the UNIMARC variant, removing the use of unnecessary
temporary variables.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-15 16:01:31 +00:00
Jonathan Druart
fa989631f6 Bug 11797: fix odd number of elements in hash (UNIMARC)
I got the same warning for my UNIMARC DB.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-15 16:01:31 +00:00
Mark Tompsett
129bc32811 Bug 11797: fix odd number of elements in hash warning (MARC21)
This was discovered when someone triggered an authority search
on an authority record that was missing what is assumed the
default subfield for a given field.

It, however, also can be triggered in an OPAC authority search
by looking at the record that lacks the default subfield for a
given field.

TEST PLAN
---------
1) Create an authority record with 180$x and NOT 180$v.
   See C4::AuthoritiesMarc::BuildSummary in the 1.. foreach loop
   for known tags and default values. The default subfields are
   the first letter of the $subfields_to_report string.
2) Trigger the bug:
   Method 1: /cgi-bin/koha/opac-authoritiesdetail.pl?authid=#
             Where # is the authority id of your tweaked record.
             The error occurs in Normal view.
   Method 2: Home -> Cataloging -> + New record
                  -> Click the 'Tag Editor' on 100$a
                  Editing of $a to $b and back may be required.
3) Notice there is an error log entry.
4) Apply the patch
5) Attempt to trigger the bug again
6) That specific error log entry is not generated.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Could generate the warning with a missing 151$a with both methods.
No warning anymore after applying this patch.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-15 16:01:31 +00:00
Bernardo Gonzalez Kriegel
c212b9328e Bug 5052: make it possible to pick a language if all choices are sublanguage
This was tricky to catch. In current implementation, Bug 6755
introduced in C4/Templates.pm as condition to send the array of
hashrefs of languages that (@$languages_loop<2), but with one
language group that condition is false, there is only one
element in that array.

This patch changes that condition to have more than one language
selected, grouped or not.

Also send $bidi value always, that was only sent if there is
more than one group language.

To test:
1. Translate to en-GB and en-NZ, or simply do mkdirs
on intranet-tmpl/prog and opac-tmpl/bootstrap

2. Go to Administration > System preferences > I18N
enable those languages on staff/opac

3. Check that language chooser is nowhere to be found

4. Apply the patch

5. Reload staff/opac, now you can see language chooser

NOTE: I made little changes on staff, but can't replicate
bootstrap colors for selected/unselected language. Someone
need to touch css files to make it happen. But that is
current behavior.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Good catch!

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-15 14:36:29 +00:00
Robin Sheat
95056d17b7 Bug 11947 - renumber reserves when hold is confirmed
Currently when a reserve is moved to "waiting" status because it's
acknowledged on checkin, the reserve priorities aren't renumbered. This
causes things to go a bit haywire in the UI, in particular, some
reserves can unjustly end up with priority 1 when they shouldn't. It
also seemed to mess with the logic of who should get it next, but I
didn't look too closely at that.

This patch forces a renumbering so that all the priorities remain
copacetic.

Test plan:
* have a few borrowers, say 4.
* have a biblio with a single item (you can scale this up, it should
  work just the same.)
* issue the item to borrower A
* have borrowers B, C, and D place a hold on the item
* return the item, acknowledge that it'll be put aside for B.
* view the holds on the item.
Without the patch:
* the hold priorities in the UI end up being "waiting, 2, 1" when they
  should be "waiting, 1, 2".
* in the database "reserves" table, they're really "0, 2, 3" when they
  should be "0, 1, 2".
With the patch:
* the hold priorities in the UI end up being "waiting, 1, 2"
* in the database, they're "0, 1, 2"

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

Test plan confirms that the problem exists and that the patch corrects
it.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script, especially t/db_dependent/Reserves.t.
Improves priority calculation.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-15 14:18:19 +00:00
Jonathan Druart
9132f9befb Bug 11960: replace unnecessary call of GetMemberDetails by CanBookBeRenewed
C4::Circulation::CanBookBeRenewed called C4::Members::GetMemberDetails to
retrieve categorycode and branchcode.

- categorycode is used to retrieve the issuing rule
- the borrower information is passed to
  C4::Circulation::_GetCircControlBranch. Which only uses the branchcode
  parameter.

GetMemberDetails does a lot of calls/queries (patronflags,
account, etc.) that are not needed by CanBookBeRenewed.

This patch replaces it with a call to C4::Members::GetMember.

Note: I presented this small optimisation during a quick introduction to
NYTProf (hackfest 14 in Marseille).

Test plan:
- launch member unit tests
- check the code

Checking the code resulted in the following:
CanBookBeRenewed builds a hash reference from the borrowernumber
(2482). Note it is only used in this function and not passed in.

_GetCircControlBranch (2485) requires that hashreference to
have a branchcode key. As stated above.

The following line (2486) requires it have a categorycode key.

As such, C4::Members::GetMemberDetails is confirmed to be
overkill, and C4::Members::GetMember is sufficient.

Testing Done
------------
0) Back up DB
1) Make sure MPL is in the list of libraries.
2) Apply the patch.
3) run the koha qa test tool
4) prove -v t/db_dependent/Circulation.t

Patch applies cleanly. QA Test tool was all OK. All tests ran successfully.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-11 16:22:43 +00:00
612b33ae69 Bug 11711 - Use new DataTables include in circ templates
Bug 10649 introduced a new include file for adding DataTables-related
JavaScript assets. This patch adds use of this include file to all
circ-related pages which use DataTables.

Apply the patch and test the following pages to confirm that table
sorting works correctly:

- Circulation
  - The UseTablesortForCirc system preference must be enabled.
  - Check out to a patron with existing checkouts. Choose a patron who
    is a guarantor to another patron with checkouts in order to test the
    relatives' checkouts table.
  - The checkouts and relatives' checkouts tables have been modified to
    exclude articles when sorting of titles.

- Hold ratios - The title column has been configured to exclude articles
  from sorting

- Transfer to receive

- Holds queue
  - The title column has been configured to exclude articles when
    sorting
  - The date column has been modified to use the title-string filter for
    sorting. An unformatted date is now passed from C4::HoldsQueue.pm to
    the template, where the KohaDates filter is used for formatting.
    Sorting is based on the unformatted date.

- Holds awaiting pickup
  - The "available since" column has been configured for sorting on an
    unformatted date. waitingreserves.pl now passes the unformatted
    date to the template, and formatting is done using the KohaDates
    filter.
  - The title column has been configured to exclude articles when
    sorting.

Edit: Rebased on current master following commit of Bug 11605
Signed-off-by: A. Sassmannshausen <alex.sassmannshausen@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-08 23:47:27 +00:00
Galen Charlton
f4efda5bc5 Bug 7413: (follow-up) improve return of GetSoonestRenewDate()
This patch modifies GetSoonestRenewDate() so that it returns
undef if the patron, item, or loan cannot be found.  This
better reflects the usage of this routine GetSoonestRenewDate(),
as none of its callers tried to check the second return
value containing an error code.

This patch also updates the POD to match.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-08 22:52:01 +00:00
Holger Meißner
065926d224 Bug 7413: Code and intranet template changes
This patch modifies CanBookBeRenewed, so that based on
issuingrules.norenewalbefore a new error "too_soon" can be returned.
Also adds a new subroutine GetSoonestRenewDate.

To test:
1) Create an issuing rule with "No renewal before" set to value X
   and "Unit" set to days.
2) Test the following steps for both:
   Home > Patron > Patron details
   Home > Circulation > Checkouts
3) On the checkout page, test for today's issues as well as previous
   issues. (Check something out on one day and something else on the
   next day, then do the testing.)
4) Confirm that items can't be renewed if current date is more than
   X days before due date.
5) Confirm that the date and time of the soonest possible renewal are
   displayed in the format specified by global sysprefs "dateformat"
   and "TimeFormat".
6) Confirm that items can be renewed if "No renewal before" is
   undefined or current date is X or less days before due date.
7) Confirm that if the number of allowed renewals is exceeded
   "Not renewable" is displayed, no matter what "No renewal before"
   is set to.
8) Test the same things with "Unit" set to hours.

Sponsored-by: Hochschule für Gesundheit (hsg), Germany
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
2014-04-07 18:28:57 +00:00
Galen Charlton
c095003b86 Bug 7567: (follow-up) construct values clause in SQL statement more idiomatically
To test:

[1] Verify that prove -v t/db_dependent/NewsChannels.t passes

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-07 18:14:21 +00:00
Jonathan Druart
ea6cffdd9f Bug 7567: QA followup: FIX SQL query
The WHERE clause should not erase $query.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-07 18:14:19 +00:00
Mark Tompsett
53f15f678f Bug 7567 - Use, display, filter News by library
This adds the ability to specify whether staff, OPAC,
or slip news entries apply to all libraries or just a
particular library.

With the branch parameter added to key functions in
C4/NewsChannels.pm, function calls in C4/Members.pm,
mainpage.pl, opac/opac-main.pl, tools/koha-news.pl, and
t/db_dependent/NewsChannels.t were needed.

Some license texts were updated.

Templates were modified to display, allow for entry and editing
of the branches selected.

TEST PLAN
---------
1) Having logged into the staff client, is the news displaying
   correctly? Have you entered a news item which should not
   display for this branch of logged in user?
2) Find a patron (with some items checked out?)
3) Print a slip
   - News which is labelled 'All Branches' or for the same branch
     as the one printing the slip should display on the slip.
   - THIS DOES NOT AFFECT QUICK SLIPS
4) Home -> Tools -> News
   - Can you edit a news item?
   - Does the change save correctly?
   - Can you filter based on location and branch correctly?
   - Can you add a new entry correctly?
   - Can you delete an entry correctly?
5) Open an OPAC client.
   - Does only the news for all branches display?
6) Log into the OPAC client.
   - Does the news for all branches and the specific branch display?
7) prove -v t/db_dependent/NewsChannels.t
   - Does it run and all succeed?
   - Does the code seem to catch the required cases?
8) Comparing the patched and unpatched versions of files affected,
   are the license changes missing anything?

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-07 18:14:19 +00:00
Mark Tompsett
2f2a0cf53e Bug 7567: convert news add/update routines to take hashref; fix bugs
Changed the add and update functions to use a hash reference
for the parameter, so that adding or subtracting parameters
should be easier. Added some POD for the add_opac_news and
upd_opac_news functions, so that developers would know how to
call it.

The hashref changes resulted in being able to return 0 for
failure and 1 for success. This meant adding a couple tests
to the test file.

And while testing, there was some sort of logic problem with
the matter of '' being all, but selecting all only showed
things set for all, and excluded particular languages, or other
interfaces.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-07 18:13:56 +00:00
Mark Tompsett
d3eaa62717 Bug 7567: (code cleanup) remove finish calls
"When all the data has been fetched from a SELECT statement,
the driver will automatically call finish for you. So you should
not call it explicitly except when you know that you've not
fetched all the data from a statement handle and the handle
won't be destroyed soon."
(http://search.cpan.org/~timb/DBI-1.627/DBI.pm#finish)

All the $sth variables were scoped within the functions,
and would be destroyed immediately. Additionally, there was
one after a SELECT, for only a single idnew, and so it was
not necessary.

TEST PLAN
---------
1) prove -v t/db_dependent/NewsChannels.t
2) apply patch
3) prove -v t/db_dependent/NewsChannels.t

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

ok 1 - use C4::NewsChannels;
ok 2 - Successfully added the first dummy news item!
ok 3 - Successfully added the second dummy news item!
ok 4 - Successfully updated second dummy news item!
ok 5 - Successfully tested get_opac_new id1!
ok 6 - Successfully tested get_opac_new id2!
ok 7 - Successfully tested get_opac_news!
ok 8 - Successfully tested GetNewsToDisplay!

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-07 18:05:41 +00:00
Mark Tompsett
b56f092299 Bug 7567: (code cleanup) update license and use Modern::Perl
Grabbed the current license from
  http://wiki.koha-community.org/wiki/Coding_Guidelines#Licence
and changed the use strict; use warnings; into a
use Modern::Perl instead.

TEST PLAN
---------
1) Log into staff client.
    - Does news look okay?
2) Apply patch
3) Refresh staff client.
    - Does news look the same?

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

Safe no op action

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-07 18:05:14 +00:00
cb62a47bbf Bug 11694: [QA Followup] strip out time portion when setting suspension date for individual hold
This patch fixes an issue originally reported by bug 11702.

RM note: the patch is clear enough and doesn't break existing tests,
but on the other hand, I have been completely unable to reproduce
the original issue.

To test:

[1] Verify that prove -v t/db_dependent/Holds.t passes

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-26 16:51:03 +00:00
Julian Maurice
9b63f0d018 Bug 11701: Fix C4::Context::Zconn
When calling C4::Context::Zconn twice with different parameters,
the same ZOOM::Connection object is returned twice (parameters
of 2nd call are not used) This patch fixes that.

This is in part because the connection cache is keyed on server
name only. This patch corrects this by keying on all parameters.

TEST PLAN
---------
1) apply patch
2) run koha qa test tools
3) prove -v t/Context.t

The unit tests properly triggers the modified routine for
testing. Additionally, in hunting for ways it could break,
no nested synchronous or asynchronous Zconn's were found.
And even if they were, the keying on all parameters should allow
it to function properly without messing up the other connection.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-25 14:43:03 +00:00
183d4a55ca Bug 11923: fix option to sort contents of MARC record batches by citation descending
When the ability to stage authority records was added to Koha, sorting
record batches by citation ( i.e. title ) caused the addition of
"authorized_heading" to be added to the sort. When sorting by title
descending, this causes the order by clause to be "title,
authorized_heading DESC" which means sort by title ASC, then
authorized_heading DESC. This is incorrect and causes regular biblio
batches to always be sorted ascending.

Test plan:
1) Stage a batch of biblio records from a file
2) View the staged batch
3) Attempt to sort by title descending
4) Note it is still sorted by title ascending
5) Apply this patch
6) Note the sorting now works correctly

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Works as advertised. The code pertaining to sorting in routine
GetImportRecordsRange will probably not win beauty prizes.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-13 15:10:56 +00:00
Julian Maurice
65b62137fc Bug 11861: Add ISSN column in serials claims table
Signed-off-by: Koha Team Lyon 3 <koha@univ-lyon3.fr>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Added Sign off line.
Passes all tests and QA script, including t/db_dependent/Serials.t

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-12 17:42:41 +00:00
d6c40ebe5a Bug 11718 - Use new DataTables include in reports templates
Bug 10649 introduced a new include file for adding DataTables-related
JavaScript assets. This patch adds use of this include file to all
circ-related pages which use DataTables.

Apply the patch and test the following pages to confirm that table
sorting works correctly:

- Reports -> Guided reports -> Use saved
  (reports/guided_reports.pl?phase=Use saved):
  "Creation date" sorting has been reconfigured to use the title-string
  method for sorting on an unformatted date. C4:Reports::Guided.pm has
  been modified to pass an unformatted date to the template. Sorting
  should work correctly for all settings of the dateformat system pref.

- Reports -> Catalog by item type
  (reports/manager.pl?report_name=itemtypes)

- Reports -> Serials statistics wizard (reports/serials_stats.pl):
  The subscription begin and subscription end columns have been modified
  to use the title-string filter for sorting. An unformatted date is now
  passed from reports/serials_stats.pl to the template, where the
  KohaDates filter is used for formatting. Sorting is based on the
  unformatted date. Sorting should work correctly for all settings of
  the dateformat system pref.

  - Sorting of titles should now exclude article from sorting.
  - Minor template improvements:
    - Vendor name now links to vendor details.
    - Subscription title now links to subscription details.
    - Library name is now shown instead of branchcode.

Signed-off-by: Aleisha <aleishaamohia@hotmail.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.
Checked all pages, no regressions or Javascript errors detected.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-12 17:33:25 +00:00
Sophie Meynieux
b5d85741a7 Bug 11760: In notices, output road type description rather than code
Test plan :
  - Define a notice containing <<borrowers.streettype>>
  - Trigger an event that generate this notice

Without patch <<borrowers.streettype>> is replaced by ROADTYPE
authorised value code. With the patch it is resplaced by its
description

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
This works as described, passes tests and QA script.

Note: it seems it's not possible currently to use B_streettype from
the interface, but it might be worth adding it as a follow up for later
use.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-12 17:26:55 +00:00
Jonathan Druart
0b35290011 Bug 11550: Add UT for C4::Acquisition::GetOrder
If GetOrder is called with a nonexistent ordernumber or without any
ordernumber, it should return undef.

Test plan:
prove t/db_dependent/Acquisition.t

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Updated number of tests to 68, tests and QA script all happy now.
Looked at a few pages in aquisition using GetOrder as well.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-12 14:16:20 +00:00
Galen Charlton
556eb67718 Bug 108661: (follow-up) enshrine letting CardnumberLength specify just a maximum
This patch finishes the work started in one of the previous
follow-ups and allows CardnumberLength to be set to a value
like ',5'.  In conjunction with not including cardnumber in
BorrowerMandatoryField, this allows a cardnumber to not be
required but, if present, to not exceed the specified length.

This patch also updates t/db_dependent/Members.t so that
it runs in a transaction, tests the new return value
of checkcardnumber, and manages the CardnumberLength syspref.

To test:

[1] Verify that prove -v t/db_dependent/Members.t and
    prove -v t/Members/cardnumber.t pass.
[2] Set CardnumberLength to ",5" and take cardnubmer out of
    the BorrowerMandatoryField list.
[3] Verify that you can save a patron record without a cardnumber,
    but if you supply one, that it can be at most 5 characters long.
[4] Add cardnumber back to BorrowerMandatoryField.  This time, the
    minimum length is 1 even though CardnumberLength is ",5".

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-12 04:07:03 +00:00
Jonathan Druart
3d938ffc82 Bug 10861: (follow-up) various refactoring
This patch refactors the previous code and moves the logic from the pl
to a new routine.

Same test plan as previous patch.

/!\ new unit test filename.

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

Bug 10861: Reintroduced the cardnumber length check (client side)

Previous patches has removed the pattern attribute of the input, it was
not needed. This patch reintroduces it. It will only work for new
browser version.

Moreover, it manages with the ',XX' format (see UT).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Squashed the last two follow-ups. The pattern test did not work fully for me
in Firefox 26 (very recent). But I see the message when I clear the field.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-12 02:17:34 +00:00
Jonathan Druart
834abff631 Bug 10861: Add a check on cardnumber length
Some libraries would like to add a check on the cardnumber length.
This patch adds the ability to restrict the cardnumber to a specific
length (strictly equal to XX, or length > XX or min < length < max).
This restriction is checked on inserting/updating a patron or on importing
patrons.

This patch adds:
- 1 new syspref CardnumberLength. 2 formats: a number or a range
  (xx,yy).

- 1 new unit test file t/Members/checkcardnumber.t for the
C4::Members::checkcardnumber routine.

Test plan:
1/ Fill the pref CardnumberLength with '5,8'
2/ Create a new patron with an invalid cardnumber (123456789)
3/ Check that you cannot save
4/ With Firebug, replace the pattern attribute value (for the cardnumber
input) with ".{5,10}"
5/ You are allowed to save but an error occurred.
6/ Try the same steps for update.
7/ Go to the import borrowers tool.
8/ Play with the import borrowers tool. We must test add/update patrons
and the "record matching" field (cardnumber or a uniq patron attribute)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested adding, updating; importing and ran unit test.
Preliminary QA comments on Bugzilla

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-12 02:16:18 +00:00
Jonathan Druart
1e0375e91c Bug 11755: ensure that SearchOrders returns booksellerid
The SearchOrders routine should return the booksellerid and this
patch adds it.

This fixes several problems:

[1] The link to the vendor on the order receive page breadcrumbs
    was broken.
[2] The tax calculation in finishreceive.pl didn't run.
[3] The item booksellerid field never got updated during
    receipt.

Booksellerid was returned before bug 10723.

Quick test plan:
Go on orderreceive.pl and verify that the vendor link is correct.

Followed test plan. Vendor link is now correct.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-11 16:18:48 +00:00
Jonathan Druart
87330b90f4 Bug 11170: (follow-up) improve a bit of unnecessarily convoluted logic
This patch also adds POD and UT for the change in SearchOrders()

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-11 15:11:00 +00:00
Jonathan Druart
fe520d5b14 Bug 11170: make the pending order list only display orders to receive
The order status ordered is set when the basket is closed.
The parcel page should only display status "ordered" and "partial".

Test plan:
- create a basket.
- create an order.
- verify the order is not listed on the parcel page (i.e. you cannot
  receive it).
- close the basket.
- verify the order is listed on the parcel page.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-11 15:09:56 +00:00
Jonathan Druart
f10ee65d88 Bug 11751: remove dead code related to handling members of institution patrons
This patch removes some dead code concerning the handling of patrons
that are members of other, institutional patrons.  This code did not
work; removing it clears the field if somebody wants to do a better
implementation of such relationships between patrons.

This patch:

[1] Removes the memberofinstitution system preference.
[2] Removes the following routines:

    C4::Members::get_institutions()
    C4::Members::add_member_orgs() (and removing this routine
    removes a reference to a borrowers_to_borrowers table that
    does not exist).

There should be no changes whatsoever to system functionality with this
patch (with the trivial exception of the absence of the
memberofinstitution system preference).

Test plan:

[1] Look at the code and use grep, git grep, etc. verify this patch
    does not remove something in use.
[2] Verify that there are no regressions upon adding or editing
    a patron record.
[3] Verify that the memberofinstitution system preference has been
    removed

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-11 15:05:14 +00:00
Galen Charlton
7d39b3c72e Bug 10611: (follow-up) cache DBMS driver code in context object
Rather than use a package-level variable, use an object-level
one.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-10 23:17:26 +00:00
Jonathan Druart
4ae3665ad8 Bug 10611: Use mysql_auto_reconnect instead of ping
DBD::Mysql provides a mysql_auto_reconnect flag. Using it avoids
the time required to do a $dbh->ping().

Benchmarks:

use Modern::Perl;
use C4::Context;
for ( 1 .. 1000 ) {
    $dbh = C4::Context->dbh;
}

* without this patch on a local DB:
perl t.pl  0,49s user 0,02s system 98% cpu 0,525 total
* without this patch on a remote DB:
perl t.pl  0,52s user 0,05s system 1% cpu 37,358 total
* with this patch on a local DB:
perl t.pl  0,46s user 0,04s system 99% cpu 0,509 total
* with this patch on a remote DB:
perl t.pl  0,49s user 0,02s system 56% cpu 0,892 total

Testing the auto reconnect:
use Modern::Perl;
use C4::Context;
my $ping = $dbh->ping;
say $ping;
$dbh->disconnect;
$ping = $dbh->ping;
say $ping;

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment: Real improvement. No koha-qa errors

prove t/db_dependent/Circulation_issuingrules.t produces no error
prove t/db_dependent/Context.t produces no error

Test
1) dumped Koha DB, load it on a non-local server
2) run sample script whit and without patch, local and remote

use Modern::Perl;
use C4::Context;
for ( 1 .. 100000 ) {
    my $dbh = C4::Context->dbh;
}

Main difference I note is with remote server
a) without patch
real    0m16.357s
user    0m2.592s
sys     0m2.132s

b) with patch
real    0m0.259s
user    0m0.240s
sys     0m0.012s

I think this could be good for DBs placed on
remote servers

Bug 10611: add a "new" parameter to C4::Context->dbh

When dbh->disconnect is called and the mysql_auto_reconnect flag is set,
the dbh is not recreated: the old one is used.

Adding a new flag, we can now force the C4::Context->dbh method to
return a new dbh.

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

Bug 10611: Followup: remove useless calls to dbh->disconnect

These 3 calls to disconnect are done at the end of the script, they are
useless.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-10 23:15:35 +00:00
Frédérick
54a17d97dc Bug 11359: Add the borrower's cardnumber and phonenumber to the branch overdues report
The report also known as "Overdues with fines"

Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>

All tests pass, this adds data to the Patron column on the
overdues with fines report to show the patron's cardnumber
and phone number.

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

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-10 20:25:26 +00:00
Galen Charlton
67d3985929 Bug 10452: (follow-up) remove duplicate check of damaged status
While reviewing the main patch for this bug to verify that the
holds queue routines and C4::Reserves had the same conception of
when a damaged item could fill a hold request, I noticed that
GetItemsAvailableToFillHoldRequestsForBib() duplicated the code
for adding an SQL clause to filter out damaged items.  This patch
removes the duplication and improves the POD for that routine.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-10 18:15:44 +00:00
cb45d4c218 Bug 10452: make AllowHoldsOnDamagedItems control using damaged items to fulfill holds
AllowHoldsOnDamagedItems will stop item-specific holds from being placed
on damaged items, but does not stop Koha from using damaged items to
fill holds. This seems like incorrect behavior.

Test Plan:
1) Set 'AllowHoldsOnDamagedItems' to "Don't Allow"
2) Pick an item, set it to damaged
3) Place a bib-level hold on this item's record
4) Scan the item though the returns system
5) Koha will ask to use this item to fill the hold, click "ignore"
6) Apply this patch
7) Repeat step 4
8) Koha will not ask to use this item to fill the hold

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-10 18:15:12 +00:00
Galen Charlton
e51b441781 Bug 8918: (follow-up) tidy code and description of CalculatePriority()
This patch improves the formatting and the description of the new
CalculatePriority() routine.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-10 17:48:54 +00:00
Julian Maurice
3e344d7e07 Bug 8918: Fix reserve priority in ILS-DI
The priority of new hold requests was not calculated when using ILS-DI.

A new routine is added, C4::Reserves::CalculatePriority(), to calculate
the priority prior to placing a request.

A separate bug report, 11640, covers the changes in reserves to
use this new routine more generally.

This patch does therefore only affect ILS-DI.

Note: ILS-DI already allows you to generate multiple holds on a biblio or
item for the same patron. This patch does not change that behavior.

Test plan:
[1] Place multiple holds using ILS-DI HoldTitle service:
    /cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=BORROWERNUMBER&bib_id=BIBLIONUMBER&request_location=test
    Check the priority.
[2] Do the same using HoldItem service:
    /cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=BORROWERNUMBER&bib_id=BIBLIONUMBER&item_id=ITEMNUMBER
    Check the priority again.
[3] Use a biblio with multiple items. Place item level holds on both.
    Check in one of these items in another branch. Confirm transfer.
    Check in the other item in the original branch. Confirm hold.
    Now you have a waiting and a transit hold.
    Test HoldTitle and HoldItem service again a few times.
[4] Enable AllowHoldDateInFuture and add a future hold.
    Now test HoldTitle and HoldItem again and check if these holds are
    inserted before the future hold (lower priority).

January 29, 2014: Rebased this patch and amended it to make a distinction
between fixing the ILS-DI bug and using the new routine.
Updated commit message and test plan (marcelr).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-10 17:31:05 +00:00
Mathieu Saby
575aa91ffa Bug 11224 : Add UT to routines of C4::Acquisition returning order(s)
C4::Acquisition need more UT, and more robust ones.  This patch
adds some.

This patch adds UT to
- GetOrder
- GetOrders
- GetCancelledOrders
- GetLateOrders

It refactors UT for SearchOrders

New UT use 2 new routines, used for check the list of fields returned
by a routine:
    _check_fields_of_order
    _check_fields_of_orders
These 2 routines could later be used by other UT

_check_fields_of_order has its own UT (tests n°14,15,16).

to test :
prove -v t/db_dependent/Acquisition.t

Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Unit tests pass, passes koha-qa.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Passes koha-qa and t

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-10 16:27:21 +00:00
Mark Tompsett
bf3b1aac7b Bug 11587: get rid of warnings generated by IsSuperLibrarian with anonymous sessions
This corrects line 1250 of C4/Context.pm to be:
    return ($userenv->{flags}//0) % 2;
And thus avoids an uninitialized value used in the modulus.

TEST PLAN
---------
1) Apply the first patch (to update t/Context.t)
2) prove -v t/Context.t
-- This should fail tests 7 and 8
3) Apply this patch (to fix C4/Context.pm)
4) prove -v t/Context.t
-- All tests should succeed

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-10 16:05:57 +00:00
0713f3bf03 Bug 10558 [QA Follow-up]
This patch addresses a number of issues with the main patch:

- regression on bug 2060 (i.e., displaying authority import batches
  correctly)
- regression on bug 10170 (translation of import record states)A
- use of datatables.inc
- lack of clarity as to the licensing of tools/batch_records_ajax.pl
- insufficent sanitizing of input used to generate an SQL statement

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-10 15:48:32 +00:00
4d32421634 Bug 10558 - Convert records table in manage-marc-import.pl to Ajax DataTable
Some libraries would like to sort by columns for the records of an
import batch. This seems like a good use of Ajax DataTables.

Test plan:
1) Apply this patch
2) Import a record batch into Koha
   a) Use some form of matching
   b) Have some records that will match and some that won't
   c) Have at least 30 records so you can test the pager
3) Verify the new table is functionally equivalent to the old static one

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

Tests fine and looks good with the exception of the corrections I put in
a follow-up.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-10 15:46:49 +00:00
daf2ebc4f5 Bug 11096: support the retrieval of large MARCXML records
This patch makes Koha <-> Zebra use MARCXML for the serialization when
using DOM, and USMARC for GRS-1.

* The following functions are modified to set the Zebra record syntax
according to the current sysprefs and configuration:

- C4::Context->Zconn
- C4::Context-_new_Zconn

* A new function 'new_record_from_zebra' is introduced, which checks the
context we are in, and creates the MARC::Record object using the right
constructor.

The following packages get touched to make use of the new function:
- C4::Search
- C4::AuthoritiesMarc

and the same happens to the UI scripts that make use of them (both in
the OPAC and STAFF interfaces).

* Calls to the unsafe ZOOM::Record->render()[1] method are removed.

Due to this last change the code for building facets was rewritten. And
for performance on the facets creation I pushed higher version
dependencies for MARC::File::XML and MARC::Record (we rely on
MARC::Field->as_string).

* Calls to MARC::Record->new_from_xml and MARC::Record->new_from_usmarc
are wrapped with eval for catching problems [2].

* As of bug 3087, UNIMARC uses the 'unimarc' record syntax. this case is
  correctly handled.
* As of bug 7818 misc/migration_tools/rebuild_zebra.pl behaves like:

- bib_index_mode (defaults to 'grs1' if not specified)
- auth_index_mode (defaults to 'dom')

here we do exactly the same.

To test:
 - prove t/db_dependent/Search.t should pass.
 - Searching should remain functional.
 - Indexing and searching for a big record should work (that's what the
   unit tests do).
 - Test an index scan search (on the staff interface):
    Search > More options > Check "Scan indexes".
 - Enable 'itemBarcodeFallbackSearch' and try to circulate any word, it
   shouldn't break.
 - Searching for a biblio in a new subscription shouldn't break.
 - Running bulkmarcimport.pl shouldn't break.
 - And so on... for the rest of the .pl files.

[1] http://search.cpan.org/~mirk/Net-Z3950-ZOOM/lib/ZOOM.pod#render()
[2] a record that cannot be parsed by MARC::Record is simply skipped (bug 10684)

Sponsored-by: Universidad Nacional de Cordoba
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-02-28 19:50:09 +00:00
a4a014e365 Bug 11736 - Use new DataTables include in Koha news templates
Bug 10649 introduced a new include file for adding DataTables-related
JavaScript assets. This patch adds use of this include file to the Koha
news page.

To test you should have existing news items with varying creation and
expiration dates. Apply the patch and confirm that table sorting works
correctly for all settings of the dateformat system preference.

C4::NewsChannels.pm has been modified so that it now passes an
unformatted date to the template, where the KohaDates plugin is used to
apply the correct formatting. Sorting is based on the unformatted date.

Also corrected: Capitalization errors.

Signed-off-by: wajasu <matted-34813@mypacks.net>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, no problems found.
Also passes tests and QA script.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-02-27 15:42:34 +00:00
a07b32f4f9 Bug 11803: use $dbh consistently in _koha_modify_item
This is just some code cleanup, no behavior change expected.
Also replacing errstr with err in testing the results. (See DBI.)

Test plan:
Modify an item and save it.

Followed test plan. No problems found.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-02-25 14:55:32 +00:00
5e0d450875 Bug 11799: Housekeeping: Remove _biblionumber_sth from VirtualShelves.pm
This routine is no longer used.

Test plan:
Do a grep on the name.
(Bonus points:) Verify if you can perform some actions on lists.

No more occurences of _biblionumber_sth found
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-02-21 18:24:11 +00:00
Jonathan Druart
a691ebc3f1 Bug 7372: Move road types from the roadtype table to the ROADTYPE AV
Currently road types are stored in a specific table in DB. Moreover, an
admin page is present in order to manage them.
This patch proposes to remove this table and this page in favour of a
new authorised value category 'ROADTYPE'.

This patch:
- adds a new AV category 'ROADTYPE' (created from the roadtype table
  content).
- remove the roadtype table.
- remove the .pl and .tt file admin/roadtype
- remove the 2 routines C4::Members::GetRoadTypes and
  C4::Members::GetRoadTypeDetails

Test plan:
1/ Execute the updatedatabase entry and verify existing roadtypes are
now stored in the AV 'ROADTYPE'.
2/ Verify you can add/update a streettype for patrons.
3/ Verify on following pages the streettype is displayed in patron
information (top left):
  circ/circulation.pl
  members/memberentry.pl
  members/moremember.pl
  members/routing-lists.pl

Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-02-21 16:00:53 +00:00
e7286e0513 Bug 11796: fix display of search result facets if facet happens to have exactly six entries
If a search gives results with 6 facets, one of those facets won't be
displayed. This is due to a bug in the code that only considers great
than 6 facets in one area, and less than 6 in another.

Test Plan:
1) Perform a search that should give results for 6 different libraries
2) Note you only see 5 libraries in the facets with no option to expand
3) Apply this patch
4) Repeat step 1
5) Note you now have the option to expand the facets list

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
This patch should provide a regression test but I really don't know how
to write it.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-02-20 18:57:35 +00:00
Colin Campbell
11faf9fde8 Bug 11479: Remove experimental given/when keywords
Replace constructs using given and when by if/else
feature now generates compilation warnings in 5.18
and is liable to change behaviour.

This patch:

* replaces the construct with if/else
* reformats the if branching using perltidy
  to remove the now redundant indent

To test:

[1] Verify that prove -v t/db_dependent/MarcModificationTemplates.t
    passes.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-02-20 15:55:21 +00:00