Commit Graph

5882 Commits

Author SHA1 Message Date
Jonathan Druart 2819c2edcc Bug 16844: (follow-up of 15656) Remove export of GetMemberRelatives from C4::Members
Bug 15656 removed the C4::Members::GetMemberRelatives subroutine but the
module still exports it.

Test plan:
  git grep GetMemberRelatives
should not return any results.

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 24a32e975c)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 13:31:21 +02:00
Jonathan Druart 4fa7e90b0e Bug 16813: Allow empty string for OPACBaseURL
The pref is prefixed by 'http://', but it should not when the pref is
set to an empty string.
This will fix the bug raised on bug 14790 comment 14.

Test plan:
Empty OPACBaseURL and confirm that it's save as it in DB

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
OPACBaseURL is saved up empty. Prefix http:// is not saved anymore

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit e9b6a5e31e)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 13:31:21 +02:00
Jonathan Druart f30665e1be Bug 16845: Remove unused C4::Members::ModPrivacy
This subroutine is no longer in used and can be removed safely

Test plan:
  git grep ModPrivacy
should not return any occurrences.

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 58138e93d7)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 13:31:20 +02:00
Marcel de Rooy a0a0e057c0 Bug 15006: [QA Follow-up] Only handle login requests in raw_transport
Although mainly hypothetical, it would still be possible to get
response from the server for an acs resend request. (This exception
is allowed in MsgType::handle.)
I also noticed that the response may well be a message from an older
session still.
This patch just removes that exception by only passing login requests
to sub handle in the raw_transport loop.

Test plan:
[1] Verify normal login procedure for raw.
[2] Check a few acs resend requests in raw. They should terminate without
    a response.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit e465d1aa4b)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 11:54:02 +02:00
Marcel de Rooy aee5ba0694 Bug 15006: [QA Follow-up] Satisfy qa tools with one tab less
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Srdjan <srdjan@catalyst.net.nz>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 64fe6e3500)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 11:53:34 +02:00
Colin Campbell ad235daaa7 Bug 15006 Correct indentation of EXPORTs in Sip.pm
Removed tabs and used spaces consistently
Used 'use base' to remove unnecessary BEGIN sub and
explicit setting of ISA at application level

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 4ad4ab6396)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 11:53:17 +02:00
Marcel de Rooy e8a03065af Bug 15006: Centralize timeout logic and allow zero client timeout
Moving timeout logic to one routine (with unit test).

This further implements two suggestions from Kyle and Larry:

[1] You could use a client_timeout of 0 to specify no timeout at all.
[2] Have the client_timeout default to the timeout if not defined.

Test plan:
[1] Run t/db_dependent/SIP/SIPServer.t.
[2] Test login timeout for raw and telnet.
[3] Check ACS status message for timeout value. Should match policy
    timeout from institution.
[4] Test client timeout (zero and non-zero).
[5] Remove client timeout. Test fallback to service.
[6] Remove service timeout too. Test fallback to 30 at login.

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended to incorporate Srdjan's suggestion to move get_timeout to
SIPServer.pm; this requires some additional mocking in the unit test.
And even makes the test db dependent, as documented.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit eb8666357c)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 11:52:51 +02:00
Colin Campbell f5995e7ae9 Bug 15006 Remove tabs from sip_protocol_loop
Remove the tabs causing inconsistent indentation
of sip_protocol_loop and replace with spaces

Reimplements the renaining parts of Marcel de Rooy's
original QA patch

No logic changes in this patch - layout only

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 2b725b6728)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 11:51:30 +02:00
Colin Campbell 4c91e87770 Bug 15006 Drop raw connection if login fails
raw_connection was not behaving correctly if an invalid string was
passed or a login failed.
It was not checking that the login succeeded ( it checked that account
existed not that it contained data and it existed even if login failed)
and so failed logins instead of aborting immediately fell through into
the sip_protocol_loop, forcing that to timeout invalid connections.
It now checks that account has id set and returns if not.

The timeout alarm is now set on the while loop, in normal running this
should not be triggered as the socket is opened and the first data
should be a login message and the while loop should only iterate once,
but lets not go into an infinite loop due to unforeseen circumstances.

I have reindented the routine as the flow was not clear (the while was
not indented at all.

Also if using Net::Server::PreFork when a new connection comes in you
may be handed the the successful login parameters from a preceding call.
Because of this you could successfully transmit transactions and Koha
would carry them out without having received a valid login ( and
possibly with the wrong account details!) We now delete any existing
account for new connections.

NB: This patch requires that the patch for bug 13807 has been applied

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 4f0fd3db55)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 11:51:18 +02:00
Jonathan Druart d4b8efca30 Bug 12178: Update serial status to "claimed" when exporting to CSV
On the same way as late issues, the serial status should be updated to
'claimed' when the issues as exported as csv.

QA note: The updateClaim and UpdateClaimdateIssues subroutine did almost the
same thing, I kick the second on off to centralize the code.

Test plan:
1/ Export some late issues as CSV (serials/claims.pl).
2/ Refresh the page (the export does not do it) and confirm that the
status, the claim date and the claim count have been updated.
3/ Select some others issues, select a notice and send the notification.
Confirm that the behavior is the same.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit b416ac16b5)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 11:49:39 +02:00
Kyle M Hall 878402689e Bug 16917 - Error when importing patrons, Column 'checkprevcheckout' cannot be null
Error:
DBIx::Class::Storage::DBI::_dbh_execute(): Column 'checkprevcheckout'
cannot be null at C4/Members.pm line 697

Test Plan:
1) Attempt to import a patron via csv
2) Note the error
3) Apply this patch
4) Repeat the import
5) No error!

NOTE: Given that all the other tests ran (comment #2), except
      those in comment #3, I resorted to following the test plan
      above using the attachment provided in comment #5. I
      believe the issues in comment #3 constitute other bugs
      which need fixing and are unrelated this bug. Applying the
      patch does resolve the error triggered, and the code is
      good.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit f8eb5e7f0e)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 11:47:05 +02:00
Colin Campbell 0817cf906f Bug 13807 Rework main input loop in SIPServer
Debugging various problems in SIPServer and control of it, found it
could loop on unread buffers (e.g. the LF of a CRLF if it was only
expecting CR) making it unresponsive to signals.
Reworked the input loop with an eye to removing unnecessary whiles
and replacing the while(1) by a while( connection valid)
Enhanced the timeout code by wapping in an eval.
Moved the logic from SIP_read_packet into the server itself
Hopefully this makes the already baroque code easier to navigate
and it did seem the server was the logical place for this
Removed no longer iused SIP_read_packet from Sip.pm

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 94441be435)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 11:37:55 +02:00
Jonathan Druart 5eb5e6ac45 Bug 16819: Remove use of map
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 1aa9f7b92f)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 11:33:10 +02:00
Tomas Cohen Arazi fa4d6a05ef Bug 16819: C4::Members::DelMember should use Koha::Holds to delete holds
This patch makes C4::Members::DelMember proprely use Koha::Holds to delete
holds. This is important as holds actions are started to be logged.

To reproduce:
- Apply the patch
- Run:
  $ prove t/db_dependent/Members.t
=> SUCCESS: Tests pass
- Sign off :-D

Sponsored-by: NEKLS
Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
All tests pass successfully

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit b7f15db41a)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 11:32:35 +02:00
Mark Tompsett e45ed6d2f4 Bug 16805: Log in with database admin user breaks OPAC
TEST PLAN
---------
1) Apply Jonathan's test patch
2) prove t/db_dependent/Letters.t
   -- dies before finishing tests
3) Apply second test patch
4) prove t/db_dependent/Letters.t
   -- dies before finishing tests
   -- 'addalert' is changed to 'getalert'
5) Log into OPAC with database admin user.
   -- see error given in comment #0
6) Apply this patch
7) prove t/db_dependent/Letters.t
   -- says 'getalert'
   -- all tests pass.
8) Log into OPAC with database admin user.
   -- logs in, but gives warning with a nice logout button.
9) run koha qa test tools.

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
No kaha qa errors
In debian display diferent error:
Can't use an undefined value as an ARRAY reference at /usr/lib/perl5/DBI.pm line 2054.
Works as advertised

NOTE: Revised test plan, as Jonathan added useful test case.
      Works as I've tested above.
      Hector tested older test plan which was steps
      5,6,8 and 9.

      Revised test plan again while tweaking to address comment #9.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit dee29d9f0b)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 11:31:06 +02:00
Aleisha 3e6e92ad66 Bug 9259: Ability to delete a staged file once it has been cleaned
To test:
1) Go to Tools -> Staged MARC Management and clean a file. If you have no files to clean, go to 'Stage MARC for import' and upload one to clean following the necessary steps.
2) Confirm that once the file has been cleaned, the Action column now shows a Delete button. Confirm this button only shows for cleaned files.
3) Click the Delete button.
4) Confirm that clicking Cancel exits the pop-up message and does not delete the file.
5) Confirm that clicking OK refreshes the list of staged records and the one you just deleted is no longer on it (has been deleted). You can confirm this by checking for the file in mysql (SELECT * FROM import_batches WHERE import_batch_id = X;)
6) Run prove -v t/db_dependent/ImportBatch.t (have written unit tests for CleanBatch and DeleteBatch)

Sponsored-by: Catalyst IT
Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Catalyst sign off, so needs another one but YAY this is great.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 08e448ee4b)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 10:59:18 +02:00
Jonathan Druart 39838698a1 Bug 16431: Use Koha::Cache to cache marc subfield structure
The marc subfield structure is currently cached using a global variable
of C4::Context. The infos are retrieved every time a new context is
created.
This patch suggests to use Koha::Cache instead.

To achieve this goal, a new subroutine is created
C4::Biblio::GetMarcSubfieldStructure, it will be called from code which
needs to get the marc subfield structure. GetMarcFromKohaField,
GetMarcSubfieldStructureFromKohaField, TransformKohaToMarc and
_get_inverted_marc_field_map are modified accordingly and the cache is cleared
when the table is updated (from the 3 pl scripts modified by this patch).

The caching done in C4::Context (marcfromkohafield) is removed.

Test plan:
Play with the marc subfield structure (in the administration module),
then add and edit records and make sure everything went fine.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Everything works as expected on my functional tests. I'm really happy to see the
patch introduces relevant tests for previously untested functions.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 172ebfe520)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 10:34:57 +02:00
Lyon3 Team 3bed66a3e2 Bug 14968 - found shouldn't be set to null when cancelling holds
Signed-off-by: JM Broust <jean-manuel.broust@univ-lyon2.fr>

Signed-off-by: Broust <jean-manuel.broust@univ-lyon2.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 83328c1061)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 10:32:51 +02:00
Jonathan Druart 998ea5d57a Bug 16770: Remove Memoize::Memcached dependency
This module is no longer in use and can be removed.

Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>

This part involves some changes in a bunch of mysterious debian|ubuntu
related files, not quite sure if I know what I'm doing
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 67e6efab5d)
2016-08-01 10:20:05 +02:00
Jonathan Druart 0a09cb7d09 Bug 16770: Remove wrong caching of 3 subroutines in C4::Lancuages
ooking at this code, you might think these subroutines are cached, but
actually they are not.

The eval surrounding the code hides a bug, if you remove it, you will
get:
  Invalid memcached argument (expected a hash)

Test plan:
Do not apply this patch and confirm that the code does not work

Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 44fb6e7225)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 10:19:58 +02:00
Jonathan Druart 917054eaf6 Bug 16624: Fix regression displaying a letter with hours
Bug 13622 has introduced a bug, if pref TimeFormat is 12hr and a date is
displayed in both title and content of the letter.

Test plan:
1 - Checkout an item (with default time 11:59:00 PM)
2 - Generate a quickslip
3 - Notice the time is 'AM'
4 - Apply patch
5 - Generate quickslip
6 - Note time is correct

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 6f0ce92543)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 09:19:21 +02:00
Kyle M Hall 16d14e1467 Bug 16534: (followup) - Tidy AddIssue
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit f1fa65f637)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 08:51:22 +02:00
Jonathan Druart 2cc01d139a Bug 16534: Block AddIssue from issuing if the return is not possible
To make sure the return can be done, AddIssue must not trust callers (they
should have done their job, but we are not sure) and check that the issue can
be returned before issuing to the patron.

There is no test plan here, this should not be possible from the Koha
interface.
However, looking at the code, it may be possible using SIP.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 1a9674fd3a)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 08:50:23 +02:00
Jonathan Druart df9fbe0aef Bug 16534: Make CanBookBeIssued test if the issue can be returned
If an issue is already checked out, CanBookBeIssued must check if the
issue can be checked in before processing the return.
In such cases (depending of the AllowReturnToBranch pref), the issue
should not be allowed.

Prior to this patch, the checkin was not done and the checkout failed
with "Duplicate entry '1204321' for key 'itemnumber'". Indeed since bug
14978, there is an uniq key on issues.itemnumber. Before bug 14978 the
issue existed but was hidden (and some weird behaviors certainly
happened!).

To avoid Koha to crash, a check is added to CanBookBeIssued (call to
CanBookBeReturned) and the librarian is not able to process the
checkout.

Test plan:

- Set AllowReturnToBranch to anywhere
- Check an item (homebranch Library 1, holding branch Library 1) out from Library 1
- Check the item out from Library 2
=> Confirm the checkout (should work with and without this patch)

- Set AllowReturnToBranch to holdinbranch ("only the library the item
  was checked out from").
- Check an item (homebranch Library 1, holding branch Library 1) out from Library 1
- Check the item out from Library 2
=> Without this patch, Koha crashed
=> With this patch, you will be warned that the checkin is not possible.

Try other combinations of the AllowReturnToBranch syspref

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 9427d44568)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 08:49:47 +02:00
Kyle M Hall dfb6ca49d8 Bug 16692 - Error "No method update!" when creating new serial issue
An improper method call was left over in C4::Serials::NewIssue from the
switch from DBIx::Class to Koha::Object.

Test Plan:
1) prove t/db_dependent/Serials.t
2) Note the errors
3) Apply this patch
4) prove t/db_dependent/Serials.t
5) No errors!

Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 557a598909)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-24 07:26:46 +02:00
Jonathan Druart a50e5f141b Bug 13074: Use Koha::Cache to cache the defaults values of a MARC record
With the global %default_values_for_mod_from_marc variable, the changes
made to the marc_subfield_structure table and especially the links
between MARC and DB fields are not safe and might be outdated (if a
field is linked/unlinked)

Test plan:
Under Plack:
- Link the barcode field, edit a record and set a barcode.
- Remove the mapping for the barcode field and then update again the
  barcode of the record.
The items.barcode DB field must not have been updated.

Without this patch, the field should have been updated.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 8c14c25521)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-24 07:23:12 +02:00
Jonathan Druart 9710f07575 Bug 12748: findSerialByStatus has been renamed with findSerialsByStatus
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 30946eff3c)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-24 07:21:55 +02:00
Jonathan Druart 69e4c39c21 Bug 12748: Code tidy
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 44062c07b7)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-24 07:21:14 +02:00
remi 2351db01d3 Bug 12748 - Add test for step 7 and 8 and rename findSerialByStatus
https://bugs.koha-community.org/show_bug.cgi?id=12478

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 1bd5a177d0)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-24 07:20:08 +02:00
remi 695dc4c370 Bug 12748 - Fixes duplicate serials with an "expected" status bug
Added a new sub to Serials.pm to be able to get serials with their statuses.
Now the sub ModSerialStatus checks for other serials with an "expected" status before doing anything.

Also modified Serials.t to be able to test those changes.

Test Plan
1) Apply patch
2) Run ./t/db_dependent/Serials.t
3) Validate that there are no errors
4) Go on "Serial collection information" page for a serial of your choice
5) Click on "Generate next"
6) Change the status of the original serial from "late" to "expected"
7) Change the newly generated serial from "expected" to "delete"
8) Validate that there are no new serials created by instruction 7 and that the serial was deleted
9) Run ./t/db_dependent/Serials.t

With QA Fixes

- Use the constant instead of the code (1 vs EXPECTED)
- Avoid interpolation in query
- use selectall_arrayref instead of fetchall_arrayref

Signed-off-by: Liz Rea <liz@catalyst.net.nz>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 10fd9f5ec0)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-24 07:19:13 +02:00
Jonathan Druart 0a7dc07cb5 Bug 16088: Introduce Koha::Cache::Memory::Lite to cache the language
The goal of this patch is to avoid unecessary flush of the L1 cache on
creating a new CGI object each time C4::Languages::getlanguage is called
without a CGI object.

The new class Koha::Cache::Memory::Lite must be flushed by the CGI
constructor overide done in the psgi file. This new class will ease
caching of specific stuffs used by running script.

Test plan:
At the OPAC and the intranet interfaces:
Open 2 different browser session to simulate several users
- Clear the cookies of the browsers
- User 1 (U1) an User 2 (U2) should be set to the default language
  (depending on the browser settings)
- U1 chooses another language
- U2 refreshes and the language used must be the default one
- U2 chooses a third language
- U1 refreshes and must be still using the one he has choosen.

Try to use a language which is not defined:
Add &language=es-ES (if es-ES is not translated) to the url, you should
not see the Spanish interface.

Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit f01a07a25e)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-16 12:55:41 +02:00
Mark Tompsett 358e7a6d61 Bug 16637: Dependency for C4::Tags not listed
Added optional dependency, so as to explain why testing
explodes when the Enhanced Content system preference
TagsExternalDictionary is set. It is optional, because not only
does TagsExternalDictionary have to be set, but TagsEnabled
must be 'Allow'.

Also tweaked C4/Tags.pm to ignore TagsExternalDictionary,
if Lingua::Ispell is not installed. A warning is given.

TEST PLAN
---------
 1) Set the Enhanced Content system preference
    TagsExternalDictionary to /usr/bin/ispell

 2) sudo apt-get install liblingua-ispell-perl
    -- should be a new install

 3) prove t/db_dependent/Tags.t
    -- should work fine

 4) sudo apt-get remove liblingua-ispell-perl

 5) prove t/db_dependent/Tags.t
    -- should explode

 6) Clear the Enhanced Content system preference
    TagsExternalDictionary

 7) prove t/db_dependent/Tags.t
    -- should work fine

 8) apply patch

 9) prove t/db_dependent/Tags.t
    -- should work fine

10) Set the Enhanced Content system preference
    TagsExternalDictionary to /usr/bin/ispell

11) prove t/db_dependent/Tags.t
    -- should work, with warning.

12) sudo apt-get install liblingua-ispell-perl

13) prove t/db_dependent/Tags.t
    -- should work fine

14) run koha qa test tools.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Post-hackfest hotel Olympia lobby signoff. Kalimera!
Works as expected.
At this moment the Tags.t test does not need the database btw,
but the module should have much more test coverage.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit c91b16bd9e)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 09:09:11 +02:00
Marcel de Rooy c8ef873ee9 Bug 16636: [QA Follow-up] Make BakerTaylor plack safe
Initialize file level lexicals each call. Do not call _initialize
outside the module.
Adjust test by mocking preferences.

Test plan:
Run t/db_dependent/External_BakerTaylor.t.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested module with trivial script under Plack/memcached by toggling
the associated preferences.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 5361440f65)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 09:08:11 +02:00
Mark Tompsett 9c7c4f02dd Bug 16636: t/00-load.t warning from C4/External/BakerTaylor.pm
Since the tests are expecting an initialize function, the
initialize call was just moved outside of the INIT block.

TEST PLAN
---------
1) prove t/00-load.t
   -- warnings about INIT for BakerTaylor
2) prove `git grep -l BakerTaylor | grep [.]t$`
   -- should all run okay
3) apply patch
4) repeat steps 1 and 2
   -- warning should be gone, and everything else run okay
5) run koha qa test tools

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>

Signed-off-by: Florent Mara <florent.mara@gmail.com>

NOTE: Tweaked test plan based on comment #4,
      Added sign off based on comment #6.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit b4479624de)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 09:07:59 +02:00
Marcel de Rooy 94d77bd9c2 Bug 16635: [QA Follow-up] Eliminate some global package vars
[1] $branch is only related to line 123 as fallback.
[2] $width moved to a constant; sub width is not used.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested the corresponding plugin in item editor.
Test t/db_dependent/Barcodes.t and Barcodes_ValueBuilder.t still pass.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 4fc8b3d6cf)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 09:07:46 +02:00
Mark Tompsett 7fbfa01f78 Bug 16635: t/00-load.t warning from C4/Barcodes/hbyymmincr.pm
According to http://perldoc.perl.org/vars.html, "our" should
be a reasonable substitute for the "use vars". By declaring as
"our", and removing the INIT, prove t/00-load will no longer
generate a warning about INIT for the C4/Barcodes/hbyymmincr.pm
module.

TEST PLAN
---------
1) prove t/00-load.t
   -- warnings about INIT for hbyymmincr
2) prove `git grep -l hbyymmincr | grep [.]t$`
   -- should all run okay
3) apply patch
4) repeat steps 1 and 2
   -- warning should be gone, and everything else run okay
5) run koha qa test tools

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit fd152d9e95)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 09:07:29 +02:00
Jonathan Druart ac00e73077 Bug 16492: Add a FIXME to explain the possible problem with no_set_userenv
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 4f3067184d)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 09:00:20 +02:00
Kyle M Hall 57086a241e Bug 16492 - Checkouts ( and possibly checkins and other actions ) will use the patron home branch as the logged in library
Bug 14507 introduced the use of checkpw in C4::SIP::ILS::Patron so that
non-Koha internal authentication processes would be able to function via
SIP ( LDAP et al ).

The problem is that checkpw changes the userenv to that of the patron!
This is not usually an issue in Koha because most of the time that
patron running through checkpw is the one to be logged in.

Aside from SIP2 the only other area where this may be an issue is in SCO
when using SelfCheckoutByLogin.

Test Plan:
1) On master, check out an item to a patron via SIP2
2) Note the checkout lists the item as having been checked out
   from the patron's home library not matter which library is was
   supposed to be checked out from.
3) Apply this patch
4) Re-checkout the item
5) The item should now be checked out as if it was checked out from
   the library as defined in the SIP configuration file.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 90307bcbfa)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 08:59:30 +02:00
Jonathan Druart 700fbd0a35 Bug 16695: Require Exception::Class 1.38 instead of 1.39
Looking at
http://cpansearch.perl.org/src/DROLSKY/Exception-Class-1.40/Changes
there is no need to require 1.39

Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit a4c44b9f64)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 08:27:39 +02:00
Jonathan Druart a1f1ea0317 Bug 16668: Fix t/Ris.t tests
Caused by bug 16442.
Now we need to mock the marcflavour pref

Test plan:
  prove t/Ris.t
should return green

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 023384811f)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 08:25:23 +02:00
Marcel de Rooy b31ee2eaf9 Bug 16428: [QA Follow-up] Useless call to GetMarcStructure in _build_default_values_for_mod_marc
The routine calls GetMarcStructure and does not use its return value
after all.

Test plan:
Run t/db_dependent/Items.t

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 6a706b86a2)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 08:22:10 +02:00
Jonathan Druart 4ebfb709f6 Bug 16428: Use the biblio framework to know if a field is mapped
The subroutine _build_default_values_for_mod_marc takes the
frameworkcode in parameter, but ModItemFromMarc did not pass it.
It uses it to know if a field is mapped or not to a Koha field
(C4::Koha::IsKohaFieldLinked).
Consequently the default framework ("") was always used.

This bug has been found working on bug 13074 and has been put on a
separate bug report to ease the backport.

Test plan:
Without this change, the tests added by bug 16428 won't pass

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 52b1b8eae1)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 08:21:42 +02:00
Jonathan Druart c63dccba27 Bug 16441: Do not use a package variable to cache C4::Letters::getletter
C4::Letters::getletter use a package variable (%letter) to cache letter
returned by the subroutine.
I have not found any direct issues caused by that but it is safer to
remove it.
It won't be a big deal to hit the DBMS to get a valid letter when
needed.

No test plan here, just confirm that the changes make sense.

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Confirm that performance loss is just a millisecond or so per
subsequent call of getletter.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit c4f388f649)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 08:20:52 +02:00
Jonathan Druart 93ffd58fe3 Bug 16443: Make C4::Members::Statistics plack safe
If the prefs is updated, the fields won't be.
To make sure we already fetch updated values, we should remove the
package variable and define it in the subroutine.

There is not test plan, just make sure the changes are consistent.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 29e402d398)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 08:17:23 +02:00
Jonathan Druart 6ff159e214 Bug 16455: Remove the "Too late to run INIT block" from C4::Tags
The pref TagsExternalDictionary is used to tell Lingua::Ispell to use an
other dictionary, different from the default one (/usr/bin/ispell).

To do so we need to set $Lingua::Ispell::path to the expected path.
It's currently done in the INIT block.

If you try to use C4::Tags, you will get the famous "Too late to run
INIT block at C4/Tags.pm line 74." warning. Plack use the INIT block to
load functions at run time, when we are using C4::Tags when hitting a pl
script, the compilation phase is finished and it's "too late to run INIT
block" from C4::Tags.

I do not really know if it has an impact on the behavior of
Lingua::Ispell (i.e. is the path redefined?), but I know that this INIT
block is not executed when we want.

Test plan:
under Plack,
- hit /cgi-bin/koha/opac-search.pl and confirm that the warning does no
longer appears
- Use another dictionnary (??), fill TagsExternalDictionary with its
  path and confirm that it is used by the tags approval system

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit 7d916c380a)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 08:16:36 +02:00
Jonathan Druart 97a3f03970 Bug 16444: Make C4::Tags plack safe
C4::Tags use a package variable to cache the pref
TagsExternalDictionary, it's not needed and not safe.

There is not test plan, just make sure the changes are consistent.

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit fff754a7fc)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 08:13:55 +02:00
Jonathan Druart a987ff3ba9 Bug 16442: Make C4::Ris plack safe
C4::Ris incorrectly uses 4 package variables:
- $utf: not used, can be removed
- $intype: set to marcflavour once, but later it assumes that it is
  usmarc if not defined
- $marcprint: always 0, so set it to 0
- $protoyear: only used in 1 subroutine, let's define it at this
  level

Test plan:
Just make sure the RIS export works as before this patch

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit 87b181c6d4)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 07:59:57 +02:00
Kyle M Hall 66c9ef12c1 Bug 16610 - Regression in SIP2 user password handling
Previous to bug 14507, SIP2 only did internal authentication. A change
to the way we check empty passwords has caused any empty password to
send back a CQ of Y. Previous to that patch set, a CQ of Y would only be
sent back of the patron password column was NULL. Now, an empty AD field
*always* returns a CQ of Y.

Test Plan:
1) Send a patron information request with an empty AD field
   Note: You must send the AD field or you won't get back a CQ field
2) Note you get back a CQ of Y
3) Apply this patch
4) Repeat step 1
5) Note you now get back a CQ of N

Signed-off-by: Trent Roby <troby@bclib.info>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit 71f0370ddd)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 07:35:58 +02:00
Marcel de Rooy 5712275a46 Bug 16504: [QA Follow-up] Make koha-qa happy with two newlines
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit 567a1a9021)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 07:34:52 +02:00
Jonathan Druart 247bd55876 Bug 16504: (follow-up for bug 15163) Do not remove attributes of other patrons
Simple patch for a silly error, this single line is going to fix a
critical bug.
If a patron attribute is limited to a library, all the values for that attributes
for every patrons will be deleted.

Test plan:
Create a patron attribute limited to a library
Set the the attribute for a patron
Set the the attribute for another patron
=> Without this patch applied, the attribute's value for the first
patron is deleted
=> With this patch applied, the 2 values exist in the DB after the
second edition

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit da94410ab5)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 07:34:38 +02:00