Commit graph

72 commits

Author SHA1 Message Date
caa1712d9f Bug 29976: (Bug 21729 follow-up) fix holds unit tests
Bug 21729 added to holds table the column patron_expiration_date.

Currently test suite is failing :
https://jenkins.koha-community.org/view/master/job/Koha_Master_D10/516/testReport/

We must fix where TestBuilder is creating with source Reserve and data expirationdate :
  t/db_dependent/Holds/WaitingReserves.t
  t/db_dependent/Reserves/CancelExpiredReserves.t

Note that t/db_dependent/Reserves/CancelExpiredReserves.t does not fail
without this patch but surely we prefere change it also.

Test plan :
prove t/db_dependent/Holds/WaitingReserves.t
prove t/db_dependent/Reserves/CancelExpiredReserves.t

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-02-01 21:39:39 -10:00
9e249435f9 Bug 28931: Use EXPORT_OK from Koha::DateUtils
It has been missed on bug 17600.

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-07 11:01:05 +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
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
8c68fa762e Bug 27058: Add test for IsAvailableForItemLevelRequest and notforloan
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-08 15:15:49 +01:00
Joonas Kylmälä
da81064b8b Bug 27058: Remove confusing and unnecessary $biblionumber1 variable
The code is much more understandable now because with $biblio2 and
$biblionumber1 variables use to point to the same biblio one might
have thought $biblionumber1 points to $biblio1 which in this case is
not true. Let's just drop the extra variable because the object
notation of accessing is just as simple.

To test:
 1) prove t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t => passes

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-08 15:15:49 +01:00
Joonas Kylmälä
1c0acc36ba Bug 27058: Add test to show ordered items cannot be checked out
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-08 15:15:49 +01:00
f1f9c6dc74 Bug 26384: Fix executable flags
.pm must not have -x
.t must have -x
.pl must have -x

Test plan:
Apply only the first patch, run the tests and confirm that the failures
make sense
Apply this patch and confirm that the test now returns green

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-09-11 09:56:56 +02:00
Agustin Moyano
93fdca5209 Bug 19889: Add tests
Sponsored-by: Cooperative Information Network (CIN)

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-08-31 16:17:58 +02:00
9c9dddeaa4 Bug 26250: Fix tests when SearchEngine=Elastic
Most of the time the tests are failing because the item is not created
correctly (missing biblio and/or biblioitem).
The usual error is:
 t/db_dependent/selenium/regressions.t ..... 5/5 Can't call method "leader" on an undefined value at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch.pm line 534.

In this patch we are making sure $builder->build({ source => 'Item' })
is replace with $builder->build_sample_item

Test plan:
Turn on Elastic and confirm that all the tests pass!

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

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:26 +02:00
Andrew Nugged
3c4f475d1a Bug 24683: whole test formatted by 'perltidy'
This is complementary patch using styling from
bundled /xt/perltidyrc file

Almost no code change except a few long constant strings
broken to parts by concatenation.

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-08-24 10:12:46 +02:00
Andrew Nugged
4599fcc59d Bug 24683: Fix for take smart rules into account in "if all unavailable"
Inside of ItemsAnyAvailableAndNotRestricted was no effect from main set
of smart rules (per record and other limits): i.e. call to
"CanItemBeReserved" was absent totally.

Because of this there was a bug: for example none of two items were
allowed to be held when first was allowed by one smart rule, BUT on loan,
and second was disallowed by another smart rule (for example,
0 "Holds per record"),

i.e. in this case both items unavailable: so on-shelf holds setting
"allow hold if all unavailable" should allow to hold first one, and not
the second one. But it was that both wasn't allowed to be held.

Solution: call to sub "CanItemBeReserved" added so it checked for
"...->{status} ne 'OK'" so now if item restricted by smart rule it also
accounted as "unavailable" and "AnyAvailavble" not counts it.

How to reproduce:

1. Add 2 smart rules (/cgi-bin/koha/admin/smart-rules.pl) with "on shelf
   holds": "if all unavailable" for all rules, no "item level holds", and
   set "holds per record" to 2 for "books" and "0" for "computer files".

2. Create only 2 items for one biblio, but different types, "book"
   and "computer file". For example in misc4dev env:
   /cgi-bin/koha/cataloguing/additem.pl?biblionumber=1#additem

3. Check out that item of type "book" to some person, for example,
   in misc4dev:
   /cgi-bin/koha/circ/circulation.pl?borrowernumber=2&barcode=3999900000001

4. Open reserve/request, for example, for item 1 and patron 1 in misc4dev
   env (/cgi-bin/koha/reserve/request.pl?biblionumber=1&borrowernumber=1)

5. It does not allow to hold, both red crossed, but computer file says
   "Exceeded max holds per record" because of "0" limit set on step 1.

6. Apply the patch.

7. Reload page on step 5 and see that "book" will be available for hold,
   but "computer file" still will be red-crossed "Exceeded max holds
   per record", now that's correct because both items unavailable:
   one because on load, another because of "0" limit for computer files.

8. Check-in book from step 3 so it will be returned to the library,

9. Reload page on step 5 and see that again no any holds available,
   but it's now also correct: "book" now returned but "on shelf holds"
   set to "if all unavailable".

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-08-24 10:12:45 +02:00
Andrew Nugged
618cf80df9 Bug 24683: Subroutine name changed (fix), no code logic changed This is the intermediate refactor: renamed subroutine only.
Naming mistake came because this sub is used to detect if anything
available for hold, but it used in "if ANY UNAVAILABLE rule", so actually
results of this sub negated (see below "return" in the code).

In details:

when previous refactor was done, name for subroutine was chosen
wrongly in "opposite" direction from what it actually does:

it was named "ItemsAnyAvailableForHold", but this subroutine gave
truth (1) if at least one of the items available on shelf, not lost,
not on loan, not held, and not restricted by smart rules and damaged
status. So, if this sub says that item is still "available", this
actually PREVENTS item from hold in parent sub (see negated return):

    sub IsAvailableForItemLevelRequest {
        ...
        my $any_available = ItemsAnyAvailableAndNotRestricted...
        return $any_available ? 0 : 1;
             # ^^^ if any available and not restricted - we don't allow
             #     on-shelf holds
        ...

I.e. like it named now: "ItemsAnyAvailableAndNotRestricted".

Small aside fix: white space for '&&' inside brackets added to join
operation by priority visually.

Testing plan not needed: all places where sub used it just renamed.
More: all this places/code was introduced in one older commit so there
is also no overlaps or other calls/uses for this subroutine.

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-08-24 10:12:45 +02:00
2849b188c8
Bug 22001: Remove the RaiseError occurrences from tests
Unless it is needed!
Also remove $dbh when not used later.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-27 08:52:56 +00:00
Andrew Nugged
d3a37911cf
Bug 24185: Make holds page faster: Preparatory refactoring
This is just refactoring. extracting logically independent code
to separate sub + tests update. No logic change yet.

Searching for "any_available" item among all biblionumber items was done
inside of "elsif on_shelf_holds == 2", and it is logically very independent
piece of code (this "@items" loop), it needs just biblionumber and patron
as parameters so it can be extracted into separate subroutine, and
later also called/reused from somewhere else.

This ability to call from another place also made for future patch
to remove O(n^2) problem with nested loops.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-25 09:40:39 +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
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
0d37a93661
Bug 18936: (QA follow-up) Fix inconsistencies in IssuingRules.t
* get_effective_rule should not pass rule_value
* indentation fix
* add comment to list the rules we have when the test _is_row_match
is executed
* compare ->rule_name and ->rule_value returns

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:29 +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
Jesse Weaver
1c43a26525
Bug 18936: (follow-up) Fix tests, replace old get_onshelfholds_policy method
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
f261dc4df6
Bug 18928: Update new occurrences
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:57:10 +01: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
917a506ffc Bug 19302: Send koha::objects to C4::Reserves::IsAvailableForItemLevelRequest
Almost everywhere we call IsAvailableForItemLevelRequest we already have
a Koha::Patron and Koha::Item object. It makes sense to use them to
avoid a refetch

Test plan:
It would be good to test this patch on top of 19300 and 19301 and make
sure everything works as expected

Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-05-10 18:57:20 +00:00
5a63c5baa2 Bug 15496: (QA follow-up) Fix remaining cases
This patch fixes remaining cases of the list => scalar change.

To test:
- Run:
  $ kshell
 k$ prove t/db_dependent/Holds/HoldFulfillmentPolicy.t t/db_dependent/Reserves.t
=> FAIL: Tests fail
- Apply this patch
- Run:
 k$ prove t/db_dependent/Holds/HoldFulfillmentPolicy.t t/db_dependent/Reserves.t
=> SUCCESS: Tests pass
- Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-04-26 17:52:53 +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
a57723fc59 Bug 22330: Transfer limits should be respected for placing holds in staff interface and APIs
Branch transfer limits are respected for placing holds in the OPAC but nowhere else. This should be remedied.

Test Plan:
1) Set up a branch transfer limit from Library A to Library B
2) Verify you cannot set up a hold for an item from Library A for pickup at Library B from the staff interface ( without overriding )
3) Verify you cannot place that hold via ILS-DI
4) Verify you cannot place that hold via SIP
4) Verify a forced hold from Library A to Library B will not show up in the holds queue

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-03-21 16:22:56 +00:00
566bb29766 Bug 18925: Update new occurrences
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-03-05 20:42:22 +00:00
26a779eded Bug 18925: Move maxissueqty and maxonsiteissueqty to circulation_rules
This patch set moves maxissueqty and maxonsiteissueqty to the
circulation_rules table.

Test Plan:
1) Apply this patch
2) Run updatedatabase
3) prove t/db_dependent/Circulation.t
4) prove t/db_dependent/Circulation/Branch.t
5) prove t/db_dependent/Circulation/GetHardDueDate.t
6) prove t/db_dependent/Circulation/Returns.t
7) prove t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
8) prove t/db_dependent/Circulation/TooMany.t
9) prove t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
10) prove t/db_dependent/Reserves.t
11) Note no changes in circulation behavior related to check out limis
    both on and off site

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-03-05 20:41:42 +00:00
31c29fd31f Bug 21206: Replace C4::Items::GetItem
Note: This is here for information purpose, feel free to test it if you
wan to play with it.

TODO: C4::Reserves::_get_itype is not longer in use

No more GetItem must be returned by:
git grep GetItem|grep -v GetItemsAvailableToFillHoldRequestsForBib|grep
-v GetItemsForInventory|grep -v GetItemsInfo|grep -v
GetItemsLocationInfo|grep -v GetItemsInCollection|grep -v
GetItemCourseReservesInfo|grep -v GetItemnumbersFromOrder|grep -v
GetItemSearchField|grep -v GetItemTypesCategorized|grep -v
GetItemNumbersFromImportBatch|cut -d':' -f1|sort|uniq

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-02-26 13:24:07 +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
ca124b5c14 Bug 21817: Centralize the mock of userenv from tests
This patch adds a new method mock_userenv from t::lib::Mocks in order to
simplify the mock of the userenv.

Test plan:
prove all the test files modified by this patch

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-01-02 20:18:29 +00:00
489c636316 Bug 21597: Incorrect date value: '0' for column 'onloan'
Fix t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t

items.onloan
  `onloan` date DEFAULT NULL,

DBIx::Class::Storage::DBI::_dbh_execute(): Incorrect date value: '0' for
column 'onloan' at row 1 at /home/vagrant/kohaclone/t/lib/TestBuilder.pm
line 288

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-18 14:18:00 +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
ef410fd62f Bug 20287: Replace occurrences of AddMember with Koha::Patron->new->store->borrowernumber
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-18 15:49:47 +00:00
7ac45d00b6 Bug 4319: (QA follow-up) Consistency in IsAvailableForItemLevelRequest
[1] For consistency going back to IsItemOnHoldAndFound in this sub.
    This call is used in the on_shelf_holds == 2 case too.
    The routine will be refactored quite soon.
    Adding the else branch for on_shelf_holds == 0 for more clarity.
[2] Removing the test for found==F in reserves. In Koha F is only used
    when the hold is filled and moved to oldreserves.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-02-13 13:02:23 -03:00
Alex Arnaud
a9779b67d6 Bug 4319: [OPAC] Allow holds on waiting/transit items
Test plan:

 - Checkout an item
 - Place hold on this item,
 - Return the item
 - Make sure the hold is waiting (found W) and AllowOnShelfHolds is
   not to 'Allow'
 - Check that the button "Place hold" appears in opac detail page of
   the biblio

 - do the samewith items/reserves in transit

Changes on C4::Reserves::IsAvailableForItemLevelRequest

Make sure this tests pass:
  - t/db_dependent/Reserves.t
  - t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t

Rebased - 2017-12-12 - Alex Arnaud

Bug 4319 - [QA fix] Create Koha::Biblio->hasItemswaitingOrInTransit

Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-02-13 13:02:23 -03:00
8d7dd0c69d Bug 18547: (QA follow-up) Add comments to make tests clearer
Tests had some confusion with names and results, though they do seem to
cover the changes here.

I believe the tests highlight that ReserveControlBranch has no effect,
and possibly it should? Beyond the scope here but left those tests
for future work to be done

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-11-26 11:30:22 -03:00
9c7356aab4 Bug 18547: Add tests
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-11-26 11:30:07 -03:00
511b325357 Bug 19437: Rearranging tests for CancelExpiredReserves
This patch originates from a QA Follow-up on bug 19260.

The first 19260 patch adds CancelExpiredReserves tests to Reserves.t.
But note that we already have some tests in Holds/CancelReserves.t.

This patch does:
Renames Holds/CancelReserves.t to Reserves/CancelExpiredReserves.t.
Rearranges modules there.
Moves its existing tests into a first subtest.
Moves the new subtest from Reserves.t to CancelExpiredReserves.t.
Replaces $dbh->do('DELETE FROM reserves').
Adds some TestBuilder statements for missing data (by the move): adding
biblio, item, borrower (removing slow AddMember call).

Test plan:
Run Reserves.t and Reserves/CancelExpiredReserves.t.

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-09 13:47:02 -03:00
725a3022c9 Bug 19260: (followup) Fix CancelReserves.t test
Test plan:
Run t/db_dependent/Holds/CancelReserves.t

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-06 16:45:20 -03:00
82115d164a Bug 19059: Move C4::Reserves::CancelReserve to Koha::Hold->cancel
This patch adds a new Koha::Hold->cancel method and replaces the calls
to C4::Reserves::CancelReserve with it.

Test plan:
- Add and cancel holds
- Change priority of holds

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-09-12 12:42:58 -03:00
2b90ea2cb0 Bug 17829: Move GetMember to Koha::Patron
GetMember returned a patron given a borrowernumber, cardnumber or
userid.
All of these 3 attributes are defined as a unique key at the DB level
and so we can use Koha::Patrons->find to replace this subroutine.
Additionaly GetMember set category_type and description.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-07-10 13:14:19 -03:00
07de44bcd1 Bug 12063: [QA Follow-up] Small change of two test scripts
Removing dbh from one script, changing rollback in the other.
Schema is leading now.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-09 08:59:40 -04:00
Alex Arnaud
a08b309c4f Bug 12063 - Remove checking of ExpireReservesMaxPickUpDelay in CancelExpiredReserves(). Koha::Hold::set_waiting calculate expiration date from today instead of hold's waiting date.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-09 08:59:40 -04:00
Alex Arnaud
26634151db Bug 12063 - Fix QA failures
- Remove expiration date calculation in C4::Letter since it's done
    when setting the reserve waiting,
  - remove expiration date calculation in circ/waitingreserves.pl. Use
    the one in DB,
  - add a new atomic update that calculate expiration date for
    waiting reserves,
  - add tests for days_foward function and fix the infinite loop.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-09 08:59:39 -04:00
Alex Arnaud
6add80083b Bug 12063 - Fix unit tests
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-09 08:59:39 -04:00
Alex Arnaud
8fc7744258 Bug 12063 - Keep patron's requested expiration date if it is prior to the calculated one
Signed-off-by: Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-09 08:59:39 -04:00