Commit graph

1089 commits

Author SHA1 Message Date
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
Jonathan Druart
f63aaf2242 Bug 13242: Remove warning if no parameter given
Without any parameter, dt_from_string should not raise a warning
message.

Test plan:
Verify that the test file t/DateUtils.t displays a warning:

  Use of uninitialized value $date_string in pattern match (m//) at
  Koha/DateUtils.pm line 58

if the change in dt_from_string is not applied (manually edit the file).

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-26 11:17:08 -03:00
9a386c7cdb Bug 12851: (QA followup) tests should not expect the <order> tag
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-25 16:05:24 -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
Srdjan
c6b7275bfe Bug 5304: Use branch codes from the database rather than hardcoded CPL and MPL
http://bugs.koha-community.org/show_bug.cgi?id=5304
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
No commit message
No test plan.

prove t/db_dependent/Items.t pass
No koha-qa errors

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-21 20:44:53 -03:00
5b71f3c5cd Bug 13281: t/ImportBatch.t shouldn't depend on the DB
Signed-off-by: Robin Sheat <robin@catalyst.net.nz>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-20 09:53:36 -03:00
0f92fbbb0c Bug 13279: t/Breeding.t shouldn't depend on the DB
Signed-off-by: Robin Sheat <robin@catalyst.net.nz>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-20 09:39:53 -03:00
7fdd7f560b Bug 13280: t/Circulation_barcodedecode.t shouldn't depend on the DB
Signed-off-by: Robin Sheat <robin@catalyst.net.nz>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-20 09:38:28 -03:00
e00cbe8091 Bug 13282: t/Message.t shouldn't depend on the DB
Signed-off-by: Robin Sheat <robin@catalyst.net.nz>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-20 09:38:02 -03:00
6c2cb7cec8 Bug 13283: t/Overdues.t shouldn't depend on the DB
Signed-off-by: Robin Sheat <robin@catalyst.net.nz>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-20 09:37:39 -03:00
1cec3e82dc Bug 13284: t/RotatingCollections.t shouldn't depend on the DB
Signed-off-by: Robin Sheat <robin@catalyst.net.nz>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-20 09:37:20 -03:00
95ce6945d8 Bug 13277: (QA followup) use t::lib::Mocks
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-19 18:12:03 -03:00
98fcda28ff Bug 13277: t/SuggestionEngine_AuthorityFile.t shouldn't depend on the DB
To reproduce:
- Stop your MySQL server:
  $ sudo service mysql stop
- Run
  $ prove t/SuggestionEngine_AuthorityFile.t
=> FAIL: some tests fail because of mysql stopped

To test (MySQL still stopped)
- Apply the patch
- Run
  $ prove t/SuggestionEngine_AuthorityFile.t
=> SUCCESS: tests pass because the ycan be loaded regardless of
            the absence of the DB server
- Sign off :-D

Regards

Signed-off-by: Magnus Enger <digitalutvikling@gmail.com>
Turned off MySQL and ran the tests before and after the patch.
Works as advertized.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-19 18:12:00 -03:00
b8a0eefc23 Bug 13278: (QA followup) use t::lib::Mocks
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-19 18:11:58 -03:00
48da54977a Bug 13278: t/Search.t shouldn't depend on the DB
To reproduce:
- Stop your MySQL server:
  $ sudo service mysql stop
- Run
  $ prove t/Search.t
=> FAIL: some tests fail because of mysql stopped

To test (MySQL still stopped)
- Apply the patch
- Run
  $ prove t/Search.t
=> SUCCESS: tests pass because the ycan be loaded regardless of
            the absence of the DB server
- Sign off :-D

Regards

Signed-off-by: Magnus Enger <digitalutvikling@gmail.com>
Turned off MySQL and ran the tests before and after the patch.
Works as advertised.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-19 18:11:54 -03:00
Katrin Fischer
bb186e09a6 Bug 13223: [QA Follow-up] Adding some unit tests for wrapper
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Trivially amended. Thanks, Katrin.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-19 13:28:28 -03:00
e2a169dc9d Bug 13242: Add a UT to t/DateUtils.t for testing DateTime bug
A bug in DateTime slow down drastically date parsing when the dates are in the
far distant future:

https://metacpan.org/pod/DateTime#Determining-the-Local-Time-Zone-Can-Be-Slow

This UT tests this situation which affects Koha::DateUtils function
dt_from_string() and output_pref().

TO TEST:
- Apply the patch containing the UT
- prove -v t/DateUtils.t
- You see that parsing a 9999-01-01 that take forever (ie more than 1s)
- Apply the patch containing the fix
- prove -v t/DateUtils.t
- No more complain.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Works as described - check-ins are now much faster.
Passes tests and QA script.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-19 11:32:32 -03:00
Jonathan Druart
752c8bf043 Bug 13276: use t::lib::Mocks::mock_dbh
To use this patch, patch from bug 13274 should be applied too.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-19 11:30:45 -03:00
206157beb1 Bug 13276: t/XSLT.t shouldn't depend on the DB
To reproduce:
- Stop your MySQL server:
  $ sudo service mysql stop
- Run
  $ prove t/XSLT.t
=> FAIL: some tests fail because of mysql stopped

To test (MySQL still stopped)
- Apply the patch
- Run
  $ prove t/XSLT.t
=> SUCCESS: tests pass because the ycan be loaded regardless of
            the absence of the DB server
- Sign off :-D

Regards

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Tests pass without db connection for me now.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-19 11:30:41 -03:00
Jonathan Druart
428a744843 Bug 13274: Mock new_dbh in t::lib::Mocks
This patch suggests to create a routine to mock C4::Context::_new_dbh.

NOTE: Works the same with and without this secondary patch.
      koha-qa tests fine. Less cutting and pasting in the future.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-19 09:50:32 -03:00
340e23ece5 Bug 13274: t/00-load.t shouldn't depend on the DB
To reproduce:
- Stop your MySQL server:
  $ sudo service mysql stop
- Run
  $ prove t/00-load.t
=> FAIL: some tests fail because of mysql stopped

To test (MySQL still stopped)
- Apply the patch
- Run
  $ prove t/00-load.t
=> SUCCESS: tests pass because the ycan be loaded regardless of
            the absence of the DB server
- Sign off :-D

NOTE: Even seems to grab more than expected, which is good.
      349 tests in master vs 364 in this branch = 16,
      but removed block is only 13 (lines 20-32).
      Also ran koha-qa test tool. :)

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Works as described, tests passing now without database available.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-19 09:50:27 -03:00
348bca3dbb Bug 11401: (followup) make tests run on absent deps
The current code breaks if a dependency is missing. The evals are
rearranged so there's no error on missing dependency.

To reproduce:
- Have a dependency for t/NorwegianPatronDB.t removed
- Run
  $ prove t/NorwegianPatronDB.t
=> FAIL: You see an error similar to this (may vary depending on the lib you removed):

t/NorwegianPatronDB.t .. You tried to plan twice at t/NorwegianPatronDB.t line 37.

- Apply the patch
- Run
  $ prove t/NorwegianPatronDB.t
=> SUCCESS: Tests are skipped on missing lib

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-14 15:32:25 -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
aa4777dbc9 Bug 11413: UT to show up the issue
These UT reflect this change:
- deletion of the field 245 if 245$a='Bad title'
- move of the 650 field to 651 if 650$9=499

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:42 -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
Magnus Enger
85255ef18f Bug 11401: QA followup - Make the tests pass
The configs in koha-conf.xml needed to be mocked. There was also
a problem with how the NorwegianPatronDBEndpoint syspref was
getting checked in the .pm.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-14 09:42:56 -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
900670bd3b Bug 13075: (followup) remove remaining warnings
There's no point creating a MARC record with undef subfields
for testing holds. This patch avoids that so no warnings are shown.

To test:
- Run
  $ prove t/db_dependent/Holds.t
=> FAIL: verify several warnings show
- Apply the patch
- Re-run
=> SUCCESS: no warnings showed.
- Sign off :-D

Regards

NOTE: Not noticable under Ubuntu 12.04 LTS, but verifiable under
      Debian Wheezy.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-14 09:35:50 -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
9e07b59d5b Bug 11634 [QA Followup] - Make unit tests pass
* Allow on shelf holds needed to be enabled
* Added some error supression code for undefined string comparison

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-12 14:56:41 -03:00
c9816945fd Bug 11319: [QA follow-up] Add test message to MarcModificationTemplates.t
The last test (#74) did not print anything. It now does..

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-12 12:04:20 -03:00
Jonathan Druart
2de657d4ec Bug 11319: Add specific UT for nonexistent field/subfield
This patch only adds unit tests for the copy and move actions.
They test if the action does not create a field/subfield if the source
did not exist.

Also it adds a unit tests for the existing behavior (in order not to
lost it): we can use the '^' and the '$' character in regex for
substituing. For example: Copy 245$a to 245$a with the regex s/^/BEGIN /
This will add the string "BEGIN " at the beginning of the 245$a fields.

To test: prove t/SimpleMARC.t

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-12 12:04:17 -03:00
Jonathan Druart
041c247124 Bug 11319: Add UT for the fields management
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-12 12:04:14 -03:00
Jonathan Druart
c48639ee27 Bug 11319: Koha::SimpleMARC should take a hashref for parameters
In order to avoid a long list of parameters, it should be better to
pass all of them into a hashref.

This patch does not add or modify a behavior.

Test plan:
Verify the unit tests still pass
- prove t/SimpleMARC.t
- prove t/db_dependent/MarcModificationTemplates.t

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-12 12:04:12 -03:00
3b300146b2 Bug 11634 [QA Followup 3] - Found holds should be considered unavailable
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-12 11:27:42 -03:00
d77f6519f4 Bug 11634 [QA Followup] - Unit Tests
These new unit tests will fail due to the fact that Koha::Database
uses a separate dbh handle than C4::Context->dbh

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-12 11:27:34 -03:00
Olli-Antti Kivilahti
51f0a0b722 Bug 13116 - Make it possible to propagate errors from C4::Reserves::CanItemBeReserved() to the web-templates.
This patch changes the way CanBookBeReserved() and CanItemBeReserved() return error
messages and how they are dealt with in the templates. This change makes it possible
to distinguish between different types of reservation failure.

Currently only two types of errors are handled, all the way to the user, from the CanItemBeReserved():
-ageRestricted
-tooManyReserves which translates to maxreserves

 #############
 - TEST PLAN -
 #############
((-- AGE RESTRICTION --))
STAFF CLIENT
1. Find a Record with Items, update the MARC Subfield 521a to "PEGI 16".
2. Get a Borrower who is younger than 16 years.
3. Place a hold for the underage Borrower for the ageRestricted Record.
4. You get a notification, that placing a hold on ageRestricted material is
   forbidden. (previously you just got a notification about maximum amount of reserves reached)

((-- MAXIMUM RESERVES REACHED --))
0. Set the  maxreserves -syspref to 3 (or any low value)
STAFF CLIENT AND OPAC
1. Make a ton of reserves for one borrower.
2. Observe the notification about maximum reserves reached blocking your reservations.

((-- MULTIPLE HOLDS STAFF CLIENT --))
3. Observe the error notification "Cannot place hold on some items"

((-- MULTIPLE HOLDS OPAC --))
1. Make a search with many results, of which atleast one is age restricted to the current borrower.
2. Select few results and "Place hold" from to result summary header element.
       (Not individual results "Place hold")
3. Observe individual Biblios getting the "age restricted"-notification, where others can be
   reserved just fine.

Updated the unit tests to match the new method return values.
t/db_dependent/Holds.t & Reserves.t

Followed test plan. Works as expected and displays meaningful messages for the reason why placing a hold is not possible.

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-12 11:23:41 -03:00
Jonathan Druart
299a8a6997 Bug 8218 : Add a maintenance script to sanitize biblio records
This patch adds:
- a new maintenance script batch_sanitize_records
- a new subroutine C4::Charset::SanitizeRecord
- new unit tests for the new subroutine

Test plan:
1/ prove t/db_dependent/Charset.t
2/ Create a record containing "&amp;amp;" (could be follow with as many
'amp;' as you want) in one of its fields and the same for the field
linked to biblioitems.url.
The url should not be sanitized, it may contain "&amp;".
3/ Launch the maintenance script with the -h parameter to see how to use
it.
4/ Launch the script using the different parameters:
 --filename=FILENAME
 --biblionumbers='XXX'
 --auto-search

The auto-search permits to sanitize all records containing "&amp;amp;" in
the marcxml field.

Use the verbose flag for testing.
Without the --confirm flag, nothing is done.

5/ Use the --confirm flag and verify in the biblioitems.marcxml field
that the record has been sanitized.

6/ Try the --reindex flag to reindex records which have been modified.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-11 15:38:36 -03:00
4f4eb98523 Bug 13113 [QA Followup] - Fix unit test
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-11 09:54:04 -03:00
Olli-Antti Kivilahti
7767f2e53b Bug 13113 - Prevent juvenile/children from reserving ageRestricted material
There is no reason for underage borrowers to reserve ageRestricted material and
then be denied it's check-out due to ageRestriction.

This patch prevents reserving material for borrowers not suitably aged.

 # # # # # #
 # A PRIORI #
 # # # # # #
BOTH THE STAFF CLIENT AND THE OPAC
1. Find a Record with Items, update the MARC Subfield 521a to "PEGI 16".
2. Get a Borrower who is younger than 16 years.
3. Place a hold for the underage Borrower for the ageRestricted Record.
4. You can reserve an ageRestricted Record with ease.
STAFF CLIENT ONLY
5. Check-in an Item from the ageRestricted Record and catch the reservation.
6. Check-out the ageRestricted Item for this underage Borrower.
7. You get a notification about being unable to check-out due to age restriction.
   How lame is that for a 12 year old?

 # # # # # # # #
 # A POSTERIORI #
 # # # # # # # #
STAFF CLIENT
1. Find a Record with Items, update the MARC Subfield 521a to "PEGI 16".
2. Get a Borrower who is younger than 16 years.
3. Check-out an ageRestricted Item for this underage Borrower.
4. You get a notification about having the maximum amount of reserves.
5. Place a hold for the underage Borrower for the ageRestricted Record.
6. You get a notification, that placing a hold on ageRestricted material is
   forbidden.

Includes Unit tests.

Followed test plan. Patch behaves as expected. (Note: Propagating error messages to template will be handled in Bug 13116 or 11999)

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-11 09:53:55 -03:00
Olli-Antti Kivilahti
ae7d5fd11b Bug 13106 - Encapsulate Circulation::GetAgeRestriction() and modify it to check borrowers age as well.
This patch moves the logic of deciding whether or not a borrower is old enough to access this material
to its own function GetAgeRestriction.

This makes it easier to use AgeRestriction elsewhere, like with placing holds.

This feature adds a new function C4::Members::SetAge() to make testing ages a lot easier.
A ton of Unit tests included.

C4::Circulate::CanBookBeIssued() fixed and issue with undefined $daysToAgeRestriction per Marc Véron's
suggestion.

Test plan:
(See comment #10 for screenshots about using age restriction)

1) Without patch

Configure Age Restricition (see Syspref AgeRestrictionMarker) and have a biblio record with e.g. PEGI 99 in age restriction field
Try to check out to a patron with age < 99
Check out should be blocked
Change entry in age restriction field to PEGI99
Check out schould now be blocked

2) With patch
Try checkouts again, behaviour should be th same.

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-11 09:52:59 -03:00
561b474bc9 Bug 13230: t/Biblio.t should run without a DB
This patch makes t/Biblio.t mock the DB connexion setting
a void one. All tests in this file currently call the C4::Biblio
functions with invalid (absent) parameters, so they are expected to
raise warnings and return undef values.

If someone writes mocked tests for the rest of the functionality,
it is expected to mock the DB calls to match the desired scenario.

To test:
- Turn of MySQL
- Run
  $ prove t/Biblio.t
=> FAIL: Tests fail because of missing MySQL server
- Apply the patch
- Run
  $ prove t/Biblio.t
=> SUCCESS: Tests now pass
- Start MySQL
- Run
  $ prove t/Biblio.t
=> SUCCESS: Tests still pass
- Sign off

Regards
Tomas

Sponsored-by: The will to procrastinate
Signed-off-by: Robin Sheat <robin@catalyst.net.nz>
Sponsored-by: Wanting the darn packages to build
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Sponsored-by: Not wanting to read in my maths book
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-11 09:43:31 -03:00
5b0b78c316 Bug 12264: [QA Follow-up] Fixing biblionumber/biblioitemnumber mixup
While testing the string change on 12264, I had this result:
DBD::mysql::st execute failed: Column 'biblioitemnumber' cannot be null at /usr/share/koha/testclone/C4/Items.pm line 2191.
DBD::mysql::st execute failed: Column 'biblioitemnumber' cannot be null at /usr/share/koha/testclone/C4/Items.pm line 2191.
    # Child (GetItemsInfo tests) exited without calling finalize()
not ok 4 - GetItemsInfo tests

This was caused by this line in the unit test:
 my $biblionumber = get_biblio();
This routine returns:
    return ($bibnum, $bibitemnum);
So instead of the bibnum the bibitemnum is saved in biblionumber.
In my test database bibnum and bibitemnum somehow got out of sync, revealing
this bug :)
The fix is just adding parentheses:
 my ( $biblionumber ) = get_biblio();

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-10 12:00:22 -03:00
Katrin Fischer
3740b2684c Bug 12264: Correct tiny typo in t/db_dependent/Items.t
One of the tests claims 'item-level_itypes is disabled'
when it should be 'item-level_itypes is enabled'.

Patch fixes this.

To test:
- run t/db_dependent/Items.t
- tests should all pass
- verify the last test reads 'enabled'
- look at the code and see it's true

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-10 12:00:22 -03:00
Jonathan Druart
a71b96295f Bug 12627: Fix default values
The default value for *by and *date fields is NULL.
But without this patch, the values are 0 or 0000-00-00.
It comes from the fact that the form set to an empty string the values
and DBIX::Class does not consider them as undefined.

This patch is very ugly, not sure how we can fix that.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
No regressions found, adding and editing suggestions from
OPAC and staff.
Passes tests and QA script.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-07 15:17:10 -03:00
Yohann Dufour
1c2744a83f Bug 12627: SQLHelper replacement - C4::Suggestions
With this patch, the subroutines NewSuggestion and ModSuggestion use DBIx::Class instead of C4::SQLHelper.
Moreover, the tests and the .pl have been adapted.

Test plan:
1) Apply the patch.

2) Execute the unit tests by launching :
prove t/db_dependent/Suggestions.t

3) The result has to be a success without error or warning :
t/db_dependent/Suggestions.t .. ok
All tests successful.
Files=1, Tests=91,  2 wallclock secs ( 0.05 usr  0.01 sys +  1.65 cusr  0.09 csys =  1.80 CPU)
Result: PASS

4) Log in the intranet, create a suggestion and verify the created suggestion.

5) Edit a suggestion from the intranet and verify the suggestion is correctly modified.

6) Log in the OPAC and verify you can add a suggestion.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Test pass, suggestion created on staff and opac,
suggestion edited without problems, no koha-qa errors.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes tests and QA script:
Also tested:
- adding suggestion from staff and OPAC
- edit suggestion from staff
- deleting suggestion from OPAC
- changing to a normal status (email got created)
- changing to a custom status (SUGGEST_STATUS)
- display of custom status in OPAC

No problems found.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-07 15:17:00 -03:00
1cabaa32b4 Bug 8836: (RM followup) unit tests should use the new API
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-11-07 11:09:14 -03:00