Commit graph

99 commits

Author SHA1 Message Date
a84deb0446 Bug 29306: Fix warning on undefined $_ in Holds.t
Use of uninitialized value $_ in concatenation (.) or string at t/db_dependent/Holds.t line 1658.

Results from copying line 70.

Test plan:
Run Holds.t

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-25 14:08:06 +02:00
70d25adbe7 Bug 25619: (QA follow-up) Fix subtest description
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-08-27 15:28:23 +02:00
bcd43bb62f Bug 25619: Unit Tests
Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-08-27 15:28:23 +02:00
9d6d641d1f Bug 17600: Standardize our EXPORT_OK
On bug 17591 we discovered that there was something weird going on with
the way we export and use subroutines/modules.
This patch tries to standardize our EXPORT to use EXPORT_OK only.

That way we will need to explicitely define the subroutine we want to
use from a module.

This patch is a squashed version of:
Bug 17600: After export.pl
Bug 17600: After perlimport
Bug 17600: Manual changes
Bug 17600: Other manual changes after second perlimports run
Bug 17600: Fix tests

And a lot of other manual changes.

export.pl is a dirty script that can be found on bug 17600.

"perlimport" is:
git clone https://github.com/oalders/App-perlimports.git
cd App-perlimports/
cpanm --installdeps .
export PERL5LIB="$PERL5LIB:/kohadevbox/koha/App-perlimports/lib"
find . \( -name "*.pl" -o -name "*.pm" \) -exec perl App-perlimports/script/perlimports --inplace-edit --no-preserve-unused --filename {} \;

The ideas of this patch are to:
* use EXPORT_OK instead of EXPORT
* perltidy the EXPORT_OK list
* remove '&' before the subroutine names
* remove some uneeded use statements
* explicitely import the subroutines we need within the controllers or
modules

Note that the private subroutines (starting with _) should not be
exported (and not used from outside of the module except from tests).

EXPORT vs EXPORT_OK (from
https://www.thegeekstuff.com/2010/06/perl-exporter-examples/)
"""
Export allows to export the functions and variables of modules to user’s namespace using the standard import method. This way, we don’t need to create the objects for the modules to access it’s members.

@EXPORT and @EXPORT_OK are the two main variables used during export operation.

@EXPORT contains list of symbols (subroutines and variables) of the module to be exported into the caller namespace.

@EXPORT_OK does export of symbols on demand basis.
"""

If this patch caused a conflict with a patch you wrote prior to its
push:
* Make sure you are not reintroducing a "use" statement that has been
removed
* "$subroutine" is not exported by the C4::$MODULE module
means that you need to add the subroutine to the @EXPORT_OK list
* Bareword "$subroutine" not allowed while "strict subs"
means that you didn't imported the subroutine from the module:
  - use $MODULE qw( $subroutine list );
You can also use the fully qualified namespace: C4::$MODULE::$subroutine

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-07-16 08:58:47 +02:00
0f23622041 Bug 16787: 'Too many holds' message appears inappropriately and is missing data
This patch alters C4/Reserves.pm to pass back 'noReservesAllowed' when
allowedreserves=0. This allows passing to the user an appropriate
message about the availability of items for holds

This patch also fixes a FIXME about using effective_itemtype to fetch item rules

To test:
1 - Set one itemtype to allow no holds
2 - Set 'Holds per record' to 0 for another itemtype/patron combination
3 - Create or find 2 records, each with items only of the itemtypes above
3 - Attempt to place a hold for a patron on each record above
4 - The message will be 'Too many holds'
5 - Apply patch and repeat
6 - Message should be "Cannot place hold: no item are available to be placed on hold"
7 - Try placing a multihold with either record above and a holdable record,
  message should end "Cannot place hold on some items'
8 - prove -v t/db_dependent/Holds.t

Rebase - Fix test expectation

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-22 14:37:14 +02:00
a620c1bb11 Bug 28078: (follow-up) Add param check to 'itemAlreadyOnHold' test
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-07 16:28:06 +02:00
d3adcec676 Bug 28078: Add 'ignore_hold_counts' param to CanItemBeReserved
This patch adds an optional param 'ignore_hold_counts' to the routine, while still forbidding holds when 0 are allowed

To test:
1 - prove -v t/db_dependent/Holds.t

Signed-off-by: David Nind <david@davidnind.com>
JK: Commit message amended: Fixed title formatting
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-07 16:28:06 +02:00
b46d69de37 Bug 27069: Adjust tests
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-07 16:08:04 +02:00
5f595aab02 Bug 17229: Move tests to t/db_dependant/ILSDI_Service.t
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

JD Amended commit message
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-01-19 16:03:18 +01:00
Julian Maurice
a369a9c8c1 Bug 17229: Add unit test for BlockExpiredPatronOpacActions check
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-01-19 15:45:11 +01:00
de4d6983ab Bug 26634: Remove GetHoldRule subroutine in C4::Reserves
This routine is only used internally and incorrectly overrides
the precedence of holds rules - it should be removed

This patch removes the routine, adjusts tests, and adds test to
confirm correct precedence is followed

To test:
1 - At the All Libraries level, create a circ rule for a specific patron category and a specific item type that only allows 1 hold
2 - At the branch-specific level for Branch A, create an All/All rule that allows 2 holds
3 - confirm ReservesControll is set to patron's library
4 - find a patron from Branch A of the category for which you made your rule
5 - find two bibs with items of the itype got which you made your rule
6 - place a hold on one bib. success!
7 - try to place a hold on the second bib. you're told you cannot because the patron is only allowed 1 hold
8 - apply patch, restart services
9 - try to place your second hold again, success!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-12-21 10:07:38 +01:00
a2652135f7 Bug 26462: Don't delete existing data for tests
These tests generate their own items, patrons, and circ rules. Existing
system data will not affect the results of the tests. We do not need
to delete information.

You may be able to notice the difference in speed of tests depending on your system. It should
be faster after applying

To test:
1 - Before applying patch:
    prove -v t/db_dependent/Holds.t
2 - It passes
3 - Apply patch
4 - prove -v t/db_dependent/Holds.t
5 - It passes

Signed-off-by: David Nind <david@davidnind.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-09-30 16:15:34 +02:00
Agustin Moyano
4b43c886a0 Bug 22789: (follow-up) Fix atomic update, GUI and more than one hold
This patch
* sets one check for reserves and another for old_reserves in
atomic update
* Adds a message below the checkbox and adds detail when a hold is non
  priority
* Fixes issue when there are more than one hold, but the first is non
  priority
* Adds test case for this last scenario

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-08-31 16:10:25 +02:00
Agustin Moyano
ee50e86784 Bug 22789: Add tests
Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-08-31 16:10:25 +02:00
80a611797b Bug 26043: Fix random failure from Holds.t
#   Failed test 'Test ModReserveMinusPriority()'
 #   at t/db_dependent/Holds.t line 202.
 #          got: undef
 #     expected: '1605'
 # Looks like you failed 1 test of 66.

It is coming from Koha::Patron->holds that is ordering by reservedate,
so "sometimes" they are ordered in reverse (at least it's my
understanding of the problem).

Test plan:
Run the test file several times (from 20 to 60x), it must never fail
with this patch

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-08-05 17:36:28 +02:00
326c0159a0 Bug 25232: Add ability to specify multiple notforloan values to skip
Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-25 10:50:02 +02:00
465e5562fc Bug 25232: Add ability to skip trapping items with a given notforloan value
This is a companion/alternative to bug 25184, in that it allows an
explicit workflow for placing returned books into temporary storage for
a few days for decontamination purposes.

The idea here is to create a specific notforloan value for "In
Decontamination" or something along along those lines. This notforloan
value would never be trappable. At the end of decon,
UpdateNotForLoanStatusOnCheckin  could be used to remove the
notforloan status and allow checkins to be trapped to fill holds.

Test Plan:
1) Apply this patch
2) Restart all the things!
3) Give an item a negative notforloan value
4) Place a hold on the item
5) Check the item in
6) Note the item is trapped for hold
7) Set SkipHoldTrapOnNotForLoanValue to the same notforloan value
   you used in step 3
8) Check the item in again
9) Note Koha did not ask you to trap the item for hold!

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-25 10:50:02 +02:00
82e866fcb5
Bug 25184: Add syspref
It's entirely possible that some libraries are relying on the current
before for part of their workflow. Do to this possibility, it seems like
a good idea to control this behavior via a system preference.

Test Plan:
1) Apply this patch set
2) Run updatedatabase.pl
3) Set TrapHoldsOnOrder to "don't trap"
4) Set an item's notforloan value to -1
5) Place a hold on that item
6) Check in the item
7) Note the item is not trapped for hold
9) Set TrapHoldsOnOrder to "trap"
10) Check in the item
11) Koha should now ask if you'd like to trap the item for the hold!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-11 08:55:13 +01:00
cc938e6be1
Bug 25184: Items with a negative notforloan status should not be captured for holds
Negative notforloan statuses should allow holds to be placed but not captured.

Due to coronavirus, we have libraries setting all returned materials to a negative notforloan value of Quarantine for several days.
They're using UpdateNotForLoanStatusOnCheckin to set that status automatically. However, those items are still capturing for holds,
even though those items cannot be checked out until the notforloan status is removed.

In cases like an On Order item where we do want the hold to fill at checkin,
UpdateNotForLoanStatusOnCheckin should be used to clear that notforloan status so the hold can fill.

In master, if I set an item to a not for loan but holdable status ( < 0 ) I can place the hold,
capture the hold and set it to waiting, but *not* check it out to the patron!
This does not make sense. I should not be able to trap an item for checkout unless it can be checked out.

Test Plan:
1) Set an item's notforloan value to -1
2) Place a hold on that item
3) Check in the item
4) Trap the item for that hold
5) Attempt to check the item out to the patron, you will be unable to
   because it is notforloan
6) Apply this patch
7) Restart all the things!
8) Repeat steps 1-3
9) The screen should no longer ask if the item should be trapped
   to fill the hold!

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Catherine Ingram <Catherine.Ingram@cedarparktexas.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-11 08:55:05 +01:00
974caa4304
Bug 23463: Fix tests
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-23 09:26:34 +00:00
bbb504c86f
Bug 23463: Replace ModItem with Koha::Item->store
Starting to replace the ModItem calls with Koha::Item->store

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-23 09:26:30 +00:00
2075a22ed3
Bug 23463: Replace AddItem calls with Koha::Item->store
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-23 09:26:26 +00:00
Agustin Moyano
acc38cbc04
Bug 24657: Fix t/db_dependent/Koha/Item.t and t/db_dependent/Holds.t
on t/db_dependent/Koha/Item.t on line 172 I created 2 Koha::Library::Groups like this

my $root1 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } );
my $root2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } );

I didn't realize this was creating 2 new libraries that sometimes messed up with tests, so I changed it to this

my $root1 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1, branchcode => undef } } );
my $root2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1, branchcode => undef } } );

on t/db_dependent/Holds.t on line 1058 I created 3 libraries like this

my $library1  = $builder->build_object( { class => 'Koha::Libraries' } );
my $library2  = $builder->build_object( { class => 'Koha::Libraries' } );
my $library3  = $builder->build_object( { class => 'Koha::Libraries' } );

but they needed to be pickup_locations, and sometimes they wheren't set as such, so I changed it to this

my $library1  = $builder->build_object( { class => 'Koha::Libraries', value => {pickup_location => 1} } );
my $library2  = $builder->build_object( { class => 'Koha::Libraries', value => {pickup_location => 1} } );
my $library3  = $builder->build_object( { class => 'Koha::Libraries', value => {pickup_location => 1} } );

To test:
1. do not apply this patch
2. in bash:
   for i in {1..300}; do echo "loop $i"; prove t/db_dependent/Koha/Item.t t/db_dependent/Holds.t; if [ "$?" = "1" ]; then break; fi; done
3. Grab a cup of coffee (or tea if you are healthy) and wait for a while
4. Whithin 300 iterations there should be an error in any of both scripts and for loop should exit
5. Apply this patch
6. repeat step 2 and 3 (decaff this time!)
7. All 300 loops should pass
8. Sign off

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-20 09:13:14 +00:00
11b44869d9
Bug 14711: Change prototype for AddReserve - pass a hashref
The number of parameters of AddReserve makes it hard to read and
maintain.
This patch replace it with a hashref, which will make the calls more
readable.

Moreover the bibitems has been removed as it was not used by the
subroutine.

Test plan:
- Make sure the tests pass
- Read the diff and search for typos
- Place a hold on few items

Note for QA: reservation_date and expiration_date do not match the DB column's names,
should we?

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-11 14:32:47 +00:00
25848e5af3
Bug 18936: Fix some more tests
* CanItemBeReserved
Prior to "Bug 18936: Convert issuingrules fields to circulation_rules",
GetHoldRule returned holds_per_record even if no reservesallowed was
defined. This change restores this behavior.
FIXME Note: In GetHoldRule we return itemtype only if reservesallowed is set,
not sure it is correct.

* t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
When setting returnbranch, holdallowed and hold_fulfillment_policy, we
should not provide categorycode.

* t/db_dependent/Holds.t
Prefer to keep the existing rules instead of removing them. It got quite
hard to understand what was going on here because of the mixup with
the rule reservesallowed that was in issuingrules, and the other rules
we used for the tests. Also, categorycode should not be passed to set
those 3 rules (holdallowed, hold_fulfillment_policy and returnbranch)

* t/db_dependent/Circulation.t
Setting lengthunit to 'hours', no need to make sure the rule has been
correctly be saved

* t/db_dependent/Circulation/CalcDateDue.t
It uses hardcoded data that is not in the sample data (categorycode=C).
Let use K that exists and postpone a refactore of the whole script (to
make it create the data it needs).

* t/db_dependent/Circulation/ReturnClaims.t
* t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t
Simple replace Koha::IssuingRule with Koha::CirculationRules

* t/db_dependent/Koha/Charges/Fees.t
=> FIXME Still failing, stuck here, need help

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-04 09:56:27 +00:00
72442dc8d2
Bug 18936: More fixes
Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-04 09:56:26 +00:00
a10c961c67
Bug 18936: Remove last bit of Koha::IssuingRule
Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-04 09:56:25 +00:00
Jesse Weaver
f2dfa17f0e
Bug 18936: (follow-up) Add foreign key and scope enhancement to circ rules
This necessitates moving the circ rules from using '*' to using
undef/NULL.

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-04 09:56:24 +00:00
58fda20e85
Bug 18936: Convert issuingrules fields to circulation_rules
Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-04 09:23:55 +00:00
Agustin Moyano
da620fdf91
Bug 22284: (follow-up) Squash multiple follow-ups
* Bug 22284: (follow-up) Remove commented warn and address test failures
  * Bug 22284: (follow-up) fix test count after merge
  * Bug 22284: (follow-up) fixes after 15496
  * Bug 22284: (follow-up) fixes after 18936
  * Bug 22284: (follow-up) Remove HomeOrHolding from reserves

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-01-03 12:58:05 +00:00
Agustin Moyano
a998ba5714
Bug 22284: Control hold groups in C4::Reserves
This patch modifies C4::Reserves to control when hold group options where selected
in smart rules.
In CanItemBeReserved adds 2 new error status messages
1) branchNotInHoldGroup: when a patron's homebranch is not in item's hold group
2) pickupNotInHoldGroup: when a selected pickup location is not in item's hold group
Also CheckReserves is modified when item's priority is defined, to control by hold
group when required.
Finally, IsAvailableForItemLevelRequest was also modified to control by hold group when
required.

To test:
1) Apply this patch
2) prove t/db_dependent/Holds.t
SUCCESS => Result: PASS
3) Sign off

Sponsored-by: VOKAL
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-01-03 12:58:03 +00:00
dc94b4d05b
Bug 18928: Move holdallowed, hold_fulfillment_policy, returnbranch to circulation_rules
Test Plan:
1) Apply dependancies
2) Apply this patch set
3) Run updatedatabase.pl
4) Ensure holdallowed and hold_fulfillment_policy rules behavior remains unchanged

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-07-01 14:56:44 +01:00
90a15e2d82 Bug 20837: (RM follow-up) fix test count
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-04-25 14:25:06 +00:00
28e536892a Bug 20837: Unit tests
To test:
prove -v t/db_dependent/Holds.t
prove -v t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-04-25 10:26:21 +00:00
313df03ed0 Bug 17978: Unit tests for changes
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-04-10 19:59:28 +00:00
c194d0e147 Bug 22330: Fix tests
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-03-22 13:06:49 +00:00
2b76548770 Bug 21798: replace gimme_a_biblio with build_sample_biblio
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-01-28 18:53:28 +00:00
9ba6125f72 Bug 21798: Unify the creation of bibliographic record in tests
Using the newly created subroutine

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-01-28 18:53:27 +00:00
Olli-Antti Kivilahti
07c91b2116 Bug 7614: Fix my variable earlier declaration bug.
Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-01-25 14:46:59 +00:00
Lari Taskula
b028a2d1a5 Bug 7614: Check transfer limit in CanBookBeReserved and CanItemBeReserved
This patch adds $branchcode_to parameter to CanBookBeReserved and
CanItemBeReserved. It represents the pickup location for the hold. This patch
checks if the library is configured to be a pickup location (see Bug 7534), and
also if the item can be transferred into the given library (see Bug 18072).

To test:
1. prove t/db_dependent/Holds.t

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-01-25 14:46:58 +00:00
dad1324b1b Bug 15486: Unit tests
This patch introduces unit tests for the new circulation rules option
that allows setting a max holds per day limit.

To test:
- Apply the patch
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/db_dependent/Holds.t
=> FAIL: CanItemBeReserved doesn't check the amount of holds per day
   and the introduced error code is not returned. OK is returned
   instead.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-24 13:24:04 +00:00
35265c0a9b Bug 18887: Port max_holds rules to new CirculationRules system
This is the first step in the circulation rules revamp as detailed
in the RFF https://wiki.koha-community.org/wiki/Circulation_Rules_Interface_and_Backend_Revamp_RFC

This patch moves the recent max_holds rule to the new circulation_rules table.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Go to the circ rules editor, note the new max holds rules
   by patron category in the "Checkout limit by patron category".
   ( Should we rename this section? )
4) Create find a patron that is allowed to place a hold, count the
   number of holds that patron has. Lets make that number 'X'.
5) Set the new max holds rule to X for "All libraries"
6) Note the patron can no longer place another hold
7) Set the new max holds rule to X + 1 for the patron's home library
8) Note the patron can again place another hold
9) Set the new max holds rule to X for the patron's home library
10) Note the patron can no longer place another hold

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Jesse Maseto <jesse@bywatersolution.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-03 17:58:12 +00:00
42c94d185a Bug 19469: Add ability to split view of holds view on record by pickup library and/or itemtype
It is possible to set up circulation rules to limit trapping of holds by pickup library and itemtype.
To make it easier to understand which holds will be trapped in a given circumstance,
it would be nice if we could optionally group holds by pickup library and/or itemtype.

Test Plan:
1) Apply this patch set
2) Run updatedatabase.pl
3) Enable AllowHoldItemTypeSelection
4) Pick a record and create holds with various pickup libraries and itemtype combinations
5) Enable HoldsSplitQueueNumbering
6) Try the different combinations of HoldsSplitQueue
7) Ensure the hold "arrows" move the items correctly
   * Up and down arrows should move hold above or below the adjacent hold within a hold fieldset
   * Top and borrom arrows should move hold to the top or bottom within a hold fieldset

Sponsored-by: Stockholm University Library

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

Followed test plan, patch worked as described. Also passed QA test tool

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-09-14 17:36:32 +00:00
b40070f4ad Bug 7534: (RM follow-up) Fix tests and adjust after bug 15524
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-09-06 17:32:26 +00:00
Kyle M Hall
c556ad49b8 Bug 7534: Fix number of tests for Holds.t
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-09-06 17:32:25 +00:00
Lari Taskula
ce411c186d Bug 7534: Validate pickup location in CanBook/ItemBeReserved
This patch adds $branchcode_to parameter to CanBookBeReserved and
CanItemBeReserved. It represents the pickup location for the hold.

To test:
1. prove t/db_dependent/Holds.t

Signed-off-by: Koha Team AMU <axelle.clarisse@univ-amu.fr>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-09-06 17:32:25 +00:00
Kyle M Hall
5f485e476b Bug 15524: (QA follow-up) Change Can[Book|Item]BeReserved to return hashref, pass limit to template
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-24 16:27:27 +00:00
038e86a395 Bug 15524: (QA follow-up) Fix meaningless test
The last test claims to allow a hold when branch=5 and patron=5, but look
at the preceding statements:
    $rule_branch->max_holds(5);
    $rule_branch->update();
    $rule_branch->max_holds(5);
    $rule_branch->insert();
The last insert will not be done, since the record is already present.
A create should have triggered an error on the primary key.
Obviously, we should use $rule_all.

Test plan:
Run the test again.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-24 16:27:26 +00:00
7e956e85b1 Bug 15524: Set limit on maximum possible holds per patron by category
It's possible to set a limit on the maximum number of holds for a particular branch/category/itemtype, but not on the total number of holds for a given patron (by branch/category).
This new rule works in conjunction with the existing branch/borrower/item rules in that Koha will use the lower of the two limits. This new rule counts all holds of all types, which prevents bib-level holds from not being counted for the purpose of these limits. This makes the most sense and was also requested by the sponsor.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Go to the circ rules editor, note the new max holds rules
   by patron category in the "Checkout limit by patron category".
   ( Should we rename this section? )
4) Create find a patron that is allowed to place a hold, count the
   number of holds that patron has. Lets make that number 'X'.
5) Set the new max holds rule to X for "All libraries"
6) Note the patron can no longer place another hold
7) Set the new max holds rule to X + 1 for the patron's home library
8) Note the patron can again place another hold
9) Set the new max holds rule to X for the patron's home library
10) Note the patron can no longer place another hold

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-24 16:23:27 +00:00
ce96080f30 Bug 21133: Fix use statements order
Basically the idea is:
1. Undefined subroutine &C4::Items::ModZebra called at /home/vagrant/kohaclone/C4/Items.pm line 302.

=> Then use C4::Items before C4::Biblio

2. Undefined subroutine &C4::Circulation::GetItem called at /home/vagrant/kohaclone/C4/Circulation.pm line 1290

=> Then use C4::Circulation before C4::Items

And sometimes these 2 rules do not work...

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2018-07-31 16:28:02 -03:00