Commit graph

895 commits

Author SHA1 Message Date
a08e946459 Bug 7021: (QA follow-up): Minor code cleanup
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-09-22 09:33:17 -03:00
7d87128026
Bug 7021: Catch new introductions of UpdateStats
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-09-22 09:31:28 -03:00
791702a363
Bug 7021: Terminology - usercode -> categorycode
Lets stick to standard terminology and use categorycode rather than
usercode here.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-09-22 09:31:28 -03:00
Olli-Antti Kivilahti
b9158641d4
Bug 7021: Patron category in the statistics table
This patch populates the koha.statistics.usercode with borrowers.categorycode where it is easily available.
Currently for statistics.type 'issue' OR 'localuse' OR 'renew'.

Supplied a script to UPDATE the old statistics records.

Have fun!

To test:
1. Add loan for patron.
2. Check statistics table
=> 'usercode' column for this issue should now contain patrons categorycode

To test add_statistics_borrowers_categorycode.pl:
1. Run add_statistics_borrowers_categorycode.pl
2. Check statistics table
=> all statistics which are type 'issue' OR 'localuse' OR 'renew'
should now contain patrons categorycode in 'usercode' column

Also prove that tests in t/db_dependent/Circulation.t still pass.

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-09-22 09:31:27 -03:00
02326ebf24
Bug 30016: Remove GetOpenIssue subroutine
This patch adjusts the code that uses GetOpenIssue to use/find a Koha::Checkout object
instead

To test:
1 - Add a course to course reserves
2 - Create an item with barcode TESTKOC
3 - Add the item to a course
4 - Checkout the item
5 - View course details on stff and opac and confirm item shows as checked out and due date displays
6 - prove t/db_dependent/Circulation/issue.t t/db_dependent/Circulation.t t/db_dependent/CourseReserves.t
7 - Browse to Circulation->Upload offline circulation
8 - Upload a file to return the item: https://wiki.koha-community.org/wiki/Koha_offline_circulation_file_format
9 - Confirm item is returned

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-08-31 08:50:37 -03:00
73c3c5d2f1
Bug 31112: (QA follow-up) Reduce database queries
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-08-26 15:59:24 -03:00
Joonas Kylmälä
507edad61f
Bug 31112: (follow-up) Don't return "on_reserve" when there are 0 possible holds
It's possible that there could be 0 possible reserves, for example
when the hold has already been filled, thus the check would fail as
the item count can never be less than 0.

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-08-26 15:59:20 -03:00
Joonas Kylmälä
8ad9926301
Bug 31112: (follow-up) Bring back the check for non-priority holds
Before the changes from bug 31112 when CheckReserves returned a
non-priority hold we didn't return "on_reserve" status but checked in
addition to that whether there are any priority holds and if there
were, only then we returned the "on_reserve" error.

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-08-26 15:59:16 -03:00
2eb053603d
Bug 31112: (QA follow-up) Fetch patrons with reserves
Previously we fetched all in a single call using biblionumbers

Fetching each individually could be a performance hit on systems
with large numbers of holds

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-08-26 15:59:12 -03:00
4676ce1a03
Bug 31112: (QA follow-up) Restore check to avoid extra processing
We now count all holds for all patrons, we can still eject if we have more
holds than we do items

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-08-26 15:59:07 -03:00
Joonas Kylmälä
15f622ecc1
Bug 31112: CanBookBeRenewed: take into account patrons with more than 1 hold to a biblio
If a single patron had more than 1 hold to a biblio and there was only one
available item we allowed incorrectly renewing the checkout when
AllowRenewalIfOtherItemsAvailable was set to "Allow". This
changes CanBookBeRenewed so that it makes sure all the holds are
filled and not just one per patron.

To test:
 1) prove t/db_dependent/Circulation.t
 2) (Optional, as unit test is provided)
    - Set AllowRenewalIfOtherItemsAvailable = Allow
    - Create biblio with three items
    - Checkout one item to patron A
    - Add two biblio-level holds for patron B
    - Try to renew patron A's checkout with and without this patch.
    - Notice that without this patch the renewal succeeds even though we
      one unfilled hold left. After applying the patch the renewal should
      fail.

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-08-26 15:59:03 -03:00
Joonas Kylmälä
8ba1a9a534
Bug 31112: Remove unnecessary if-clause
To test:
 1) Please check manually that the logic stays the same, use git's -w command line parameter to
    ignore whitespace changes in the diff output.
 2) prove t/db_dependent/Circulation.t

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-08-26 15:58:58 -03:00
9316c123aa
Bug 24295: Finally remove GetTransfers from C4/Circulation
This patch handles the final removal of GetTransfers from
C4::Circulation.

Test plan
1/ Check that there is no mention of the GetTransfers method codebase
wide now
2/ Run the circulation and transfers tests and check nothing fails..
perhaps even run the full test suit in k-t-d
3/ Signoff

Rebased-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-08-26 15:42:56 -03:00
3b0bc03ad4
Bug 12225: Use ProcessOfflineIssue
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-08-25 08:22:35 -03:00
5f471308ad
Bug 30718: Fix renewal with due date
This was a test added by the following commit:

commit 99eccc18ed
Date:   Thu Jun 16 10:10:09 2011 +0100
    Bug 5549 : Handle datetimes on return

It's no longer needed now, we can pass a DateTime or an ISO-formatted
date

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-08-19 08:26:39 -03:00
48bf9b1d91
Bug 30718: Use flatpickr's altInput
The idea rely on the KohaDates TT plugin for the date formatting. We
should not have any output_pref calls in pl or pm (there are some
exceptions, for ILSDI for instance).

Also flatpickr will deal with the places where dates are inputed. We
will pass the raw SQL value (what we call 'iso' in Koha::DateUtils), and
the controller will receive the same value, no need to additional
conversion.
Note that DBIC has the capability to auto-deflate DateTime objects,
which makes things way easier. We can either pass the value we receive
from the controller, or pass a DT object to our methods.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-08-19 08:26:31 -03:00
Kevin Carnes
cf397ac3bc
Bug 29012: Add default values for blank circulation rules that weren't saved to the database
There are 5 fields that are not set if no value is provided when saving/editing a rule in Administration->Circulation and fines rules
- issuelength
- hardduedate
- unseenrenewalsallowed
- rentaldiscount
- decreaseloanholds

This is problematic because it gives the impression these rules are set as blank, but in reality they don't exist and the rule will fal back to the higher level

To test:
1 - Set a rule for
    Patron category: Teacher
    Itemtype: All
    Hard due date: (Today)
    Lona period: 10
2 - Set a rule for
    Patron category: Teacher
    Itemtype: Books
    Hard due date: (leave blank)
    Loan period: 10
3 - Expected behaviour is Book item will checkout to teacher for 10 days, all other types will be due yesterday at 25:59:00
4 - Checkout an non-book item type to teacher
5 - Hard due date applies
6 - Checkout a 'book' item type to teacher
7 - Hard due date applies - FAIL

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-07-29 15:44:39 -03:00
9b010d99df
Bug 31120: Items will renew for zero ( 0 ) days if renewalperiod is blank/empty value
If the rule renewalperiod is the blank empty string instead of being null/undefined or non-existant, Koha will interpret the renewal period as being zero days instead of falling back to the issuelength rule.

It makes sense to me that a literal 0 here should make it renew for zero days even though that is nonsensical.

Test Plan:
1) Delete all your rules
2) Create an all/all/all rules with an empty string for renewal base
   period
3) Note that renewing an item does nothing
4) Apply this patch
5) Restart all the things!
6) Renew again
7) Note the renewal uses the issuelength rule as intended

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-07-27 10:03:37 -03:00
d5a4d782d8
Bug 28854: Update circulation functionality for bundles
This patch updates the circulation system to account for bundle
checkins. We add a content verification step to ensure bundle content is
all present at checkin and we use this comparison to mark missing items
as lost.

Test plan
0) Apply patches up to this point
1) Checkin an item that belongs to a bundle
   * An alert should be triggered noting that the item belongs to a
     bundle
   * The option to remove the item from the bundle should be clear
   * Click remove should result in the alert dissapearing and the item
     having been removed from the bundle.
2) Checkin an item bundle
   * A modal confirmation dialog should appear requesting each item
     barcode be scanned
   * As items are scanned they should be highlighted in yellow in the
     bundle content table
   * Upon submission;
     * The user will be alerted to any unexpected items that were
       scanned and told to put them to one side.
     * The user will be alerted that any missing items in the validation
       will have been marked as lost.
     * The bundle item will be marked as checked in.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-07-13 10:35:27 -03:00
f8c9160284
Bug 30275: (follow-up) Drop renewer_id constraint
This patch fixes some unit tests by ensureing we set a valid userid for
mock userenv setting so that the foreign key constraint doesn't fail and
it also removes the exception class and check for renewer_id from the
store method as, for example with autorenewals, the renewal may not have
been triggered by a actual user.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-07-05 09:46:18 -03:00
61e41c4c41
Bug 30275: (QA follow-up) Rename columns to match API
This patch performs the following column renames:

* id => renewal_id
* issue_id => checkout_id

The idea is that no translation is needed for the API, and also, being a
new table, we can educate the users into the 'to be' terminology we are
leaning towards, instead of having them learn one naming to create
reports and then need to translate them once we normalize things in a
future.

That said, this is simple to review.

Apply this patch and repeat the test plan.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-07-05 09:46:16 -03:00
2d64b3ce8a
Bug 30275: Rename issues.renewals to issues.renewals_count
Rename the issues.renewals field to renewals_count to prevent a method
name collision with the new relation accessor introduced by this
patchset.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-07-05 09:45:55 -03:00
abaa61ab61
Bug 30275: Record renewals
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-07-05 09:45:48 -03:00
ed1a8812c1
Bug 24239: (QA follow-up) Rename date_due => due_date
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-06-23 15:43:18 -03:00
Lari Taskula
e9bab63e80
Bug 24239: Let the ILL module set ad hoc hard due dates
The Swedish Libris ILL backend lets librarians store a specific due date
when an ILL loan is received.

This patch set adds a new date_due column to the illrequets table that can be
used by the different backends to store a due date. If an illrequest has the
date due set, it will be used when the item is checked out instead of the calculation
using the circulation conditions.

To test:
- Apply the patch and make sure the atomic database update is run
- Use the FreeForm backend to add one ILL request. Take note  of the
  illrequest_id of the request you created. We refer to this as
  "x" below.
- Connect a biblio (with biblionumber y), that has an item with a
  barcode, to the ILL request directly in the database:
  UPDATE illrequests SET biblio_id = y WHERE illrequest_id = x;
- Next we set the due date, this would normally be done by or from the backend.
  UPDATE illrequests SET date_due = "2023-01-01" WHERE illrequest_id = x;
- Go to circulation and issue the barcode of the item to the
  patron associated with the FreeForm ILL request. Verify that the
  loan gets a due date of 2023-01-01.
- Ideally: return the item and issue it again through SIP2 and SCO,
  and verify that the due date is still 2023-01-01.
- Verify that there are no regressions, so that regular calculation
  of due dates still work.
- prove t/db_dependent/Circulation.t

(Patch description, test plan and partial code credits to Magnus Enger)

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

(Patch description and test plan rewritten to reflect changes in development)

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-06-23 15:30:06 -03:00
Joonas Kylmälä
7f60d88797
Bug 30947: (follow-up) Remove unnecessary object cloning
CalcDateDue() works on its own copy of the $startdate parameter
so the cloning in the calling end is not necessary.

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-06-23 11:38:45 -03:00
Joonas Kylmälä
9b4b43656e
Bug 30947: Simplify CanBookBeIssued date handling
1) This removes support for passing string dates to CanBookBeIssued. The
function didn't publicly even document support for string dates, only
DateTime objects.

2) We get a $duedate always at least from CalcDateDue so having

 $issuingimpossible{INVALID_DATE} = output_pref($duedate);

was unneccesary and thus removed.

3) The check "duedate cannot be before now" was needlessly complex: if
the due date really cannot be before now we should check seconds too
and warn the librarian! Thus the truncation to minutes can be dropped
safely.

To test:
 1) prove t/db_dependent/Circulation.t
 2) prove t/db_dependent/Illrequests.t
 3) Enable OnSiteCheckouts and disable SpecifyDueDate syspref. Create
 on-site checkout for any patron and verify the due date is your
 current date at 23:59, you can check the exact minute with sql:
  > select * from issues

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-06-23 11:38:44 -03:00
c373a253e6 Bug 29623: Cache circulation rules
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-06-14 08:26:59 -03:00
730719c90c Bug 30409: (QA follow-up) Avoid uninitialized variable warnings
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-06-14 08:26:58 -03:00
Petro Vashchuk
e2611c919d
Bug 30409: barcodedecode() should always trim barcode
Barcode is trimmed of leading/trailing whitespaces in many instances
before the barcodedecode sub was called. This patch instead makes that
barcodedecode sub is going to trim it itself and removes unnecessary,
and repetitive code that was used before barcodedecode was called.

Steps to test:
1. Edit item with any barcode, add a bunch of whitespaces at the start
and at the bottom of it. Save the item. Ensure that this action ruins
the barcode and ensure that the spaces are still there by editing the
same item again.
2. Apply the patch.
3. Edit the same item again in the same fashion. Ensure that now all
whitespaces are getting trimmed and it doesn't affect the barcode in
any negative way.

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

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2022-06-14 07:54:58 -03:00
96622e8c3f Bug 30714: Do no log update to holdingbranch
When an item from another branch is checked in, the holdingbranch is
updated, and this change is logged.  We record the checkin in
statistics, we do not need the cataloguing update logged

To recreate:
1 - Enable cataloguing log
2 - Check in an item with a different holding branch
3 - Check the action logs
4 - There is a cataloguing log for the holdingbranch update
5 - Apply patch
6 - Repeate with another item
7 - No log added!

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-05-18 17:22:09 -10:00
af1f8d34db Bug 30728: Only trigger real-time holds queue update if enabled
This patch makes the places in which Koha enqueues holds queue for
real time updates verify the feature is enabled.

To test:
1. Apply this patches up to the unit tests
2. Run:
   $ updatedatabase
   $ kshell
  k$ git diff origin/master --name-only | grep -e '\.t$' | xargs prove
=> FAIL: tests fail, the code doesn't care about the syspref
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Be happy!
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-05-12 22:17:46 -10:00
213abbe819 Bug 30668: Do not record itme location updates in cataloguing log
To test:
1 - Set a rule in UpdateItemLocationOnCheckin
2 - Enable cataloguing log
3 - Check in an item that will trigger a change
4 - Check the modification log for item - there is a cataloguing entry
5 - Apply patch
6 - Repeat
7 - No log!

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-05-09 10:02:35 -10:00
Aleisha Amohia
50d917dbfc Bug 30291: Changes to controller scripts
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-05-05 11:17:36 -10:00
763d73fbf0 Bug 29346: Add more fine-grained control of holds queue updates
This patch deals with the fact that high-level circualtion methods like
`AddIssue`, `AddReturn` and `ModDateLastSeen` all eventually call
lower-level methods like ModBiblio, Koha::Item->store of
UpdateTotalIssues which are expected to trigger holds queue updates (for
the object CRUD operations use cases). As the circulation methods need
to trigger holds queue update as well, duplicate updates were being
requested which is suboptimal, of course.

In order to prevent this, and because circulation methdos could trigger
holds queue updates several times, actually, I added a new parameter
*skip_holds_queue* to the low-level methods, so when they are called
from circulation, the trigger is skipped and we have greater control on
when and how holds queue updates are scheduled.

This patch introduces the `skip_holds_queue` parameter to the following
methods:

* C4::Biblio::ModBiblio
* C4::Biblio::UpdateTotalIssues
* Koha::Item->store

Calls to those methods from the following methods will include the new
parameter, and thus duplicated holds queue updates avoided:

* C4::Circulation::AddIssue
* C4::Circulation::AddReturn
* C4::Items::ModDateLastSeen

Tests are added, to verify that the (mocked) BatchUpdateBiblioHoldsQueue
task is only scheduled once when they are called.

To test:
1. Apply up to the previous patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Biblio.t \
           t/db_dependent/Biblio_holdsqueue.t \
           t/db_dependent/Circulation_holdsqueue.t
=> FAIL: Tests fail!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-05-05 11:17:36 -10:00
90b76521bf Bug 29346: Circulation actions triggers
This patch introduces triggers for real-time updating the holds queue at
check out and check in.

The following high-level methods are involved:

- C4::Circulation::AddIssue
- C4::Circulation::AddReturn

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Circulation_holdsqueue.t
=> SUCCESS: Tests pass! Triggers are triggered
3. Sign off :-D

Note: I put the tests on a separate file because the other one was too
big already.

Sponsored-by: Montgomery County Public Libraries
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>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-05-05 11:17:35 -10:00
8de0a02bd2 Bug 30167: (follow-up) Return a hash with soonest_renew_date
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-05-04 14:29:23 -10:00
4cf759259f Bug 30167: Return soonest renewal date when renewal is to soon
This patch adds an 'info' return param to CanBookBeRenewed and passes
the soonest renewal date when returning too_soon errors

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

Fix whitespace

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-05-04 14:29:23 -10:00
00a73846c0 Bug 29537: Remove unnecessary if-else checks for soonest renew date
GetSoonestRenewDate() calculates the renewal due date now also
correctly for auto-renewals after the fixes from bug 29476 so we don't
have to add any if-else checks because of it being buggy anymore.

To test:
 1) prove t/db_dependent/Circulation.t

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-05-03 11:19:50 -10:00
aa8e9fe5c5 Bug 29483: Further improve performance of script
This patch adds a few tests to cover more cases, and to highlight current functionality.

The script only allows renewal if all outstanding holds can be filled by available items.
This means we can return as soon as we have determined that not all holds can be filled.

I add FIXME and some explanatory comments - I will file a follow-up bug for those, but
I feel we can accept these improvements to the performance and deal with the issues
of how it 'should' work versus how it does work on another report.

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

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-04-19 21:25:51 -10:00
68c92b49d1 Bug 29483: Check ItemsAnyAvailableAndNotRestricted once per patron
ItemsAnyAvailableAndNotRestricted can take a long time and create nested loops.
We can check it once per patron, however, this requires us to flip the loops.
Since an item can only be used once, we now add a check to see if this item has already
been assigned to a borrower.

To test:
1 - Find or create a biblio with 100 items
2 - Place ten 'Next available' holds on a biblio
3 - Set preference 'AllowRenewalIfOtherItemsAvailable' to 'Allow'
    Set circ rules 'On shelf holds allowed' to 'If any unavailable'
4 - Checkout one of the items to a patron, backdated to be overdue
5 - Note a long loading time for the patron's checkouts
6 - Apply patch, restart_all
7 - Patron loads much faster

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-04-19 21:25:51 -10:00
4c52488bd9 Bug 30114: Koha offline circulation will always cancel the next hold when issuing item to a patron
If a record has holds on it, and, using offline circulation, an item on
that is checked out to a patron that did not have a hold on the record,
the next hold that would have trapped that item will be canceled.

1) Place two bib level holds on an item for two patrons
2) Find a third patron, note his or her cardnumber
3) Create the file test.koc by copying and pasting the following into a
   text editor

Version=1.0	Generator=kocTest	GeneratorVersion=0.7
2008-06-11 12:24:11 547	issue	PATRONCARDNUMBER	ITEMBARCODE

note the fields are tab delemited, ensure the tabs do not get lost
when copying and pasting!

4) Upload the offline circ action, and commit the action
5) Note the item was checked out to the third patron
6) Note one of the other patron's holds were canceled
7) Reset your database
8) Apply this patch
9) Test again, no hold should be canceled!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-04-04 16:23:45 +02:00
6f8b94f49c Bug 30110: Fix concatenation during assignements
There are several occurrences of `my $var += ` or .= in the code.
It should not cause problems but it's confusing.

Test plan:
Read the patch and confirm that the changes make sense.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-03-24 14:22:10 -10:00
Julian Maurice
b64f96830b Bug 30004: Prevent TooMany from executing too many SQL queries
If a maximum number of checkouts allowed is defined in circulation
rules, C4::Circulation::TooMany will loop over all patron's checkouts.
When a patron has several hundreds of checkouts, it can really slow down
the checkout process by several seconds (or even tens of seconds)

This patch does two things:
- Always prefetch item data so that `$c->item` does not execute an
  additional SQL query at every iteration of the loop. Item data is
  always needed at the first line of the loop, so there is really no
  downside for doing this.
- Build the `@types` array only once, out of the checkouts loop. Since
  it does not depend at all on patron's checkouts data, it does not make
  sense to build it inside the loop.

Test plan:
1. Before applying the patch, create a patron with a lot of checkouts.
   I tested with 1000 checkouts, but the slowness should be noticeable
   with less.
2. Make sure you have a circulation rule (one that apply to your patron
   and the item(s) you will check out for testing) with a maximum number
   of checkouts allowed
3. Check out an item for the patron with a lot of checkouts. Measure the
   time it takes.
4. Apply the patch
5. Check out another item (or check in and then check out the same item
   used in step 3). Measure the time it takes and compare it to step 3.
   It should be faster now.
6. Run `prove t/db_dependent/Circulation/TooMany.t`

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-03-22 10:17:33 -10:00
a0a72c9653 Bug 19532: (RM follow-up) More use of system preference
When system preference is off, call no code related to Koha::Recalls.

Also add some missing module import.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-03-14 23:11:12 -10:00
514cbb809a Bug 19532: (QA follow-up) Simplify resultset accessors
This patch makes the different ->recalls accessors implemented on this
bug be more standard. This means:
- They don't do special things like default sorting or stripping out
  special parameters. That's all left to the caller and the methods are
  clean: they just return the related objects
- Useful filtering methods for Koha::Recalls resultsets are added. The
  only used one (in the end) was ->filter_by_current. It seems like a
  better approach, because it gives devs more control on how they want
  to chain things, and there's a single place in which to maintain the
  criteria of what is 'current' or 'finished'. This clearly makes the
  'old' column obsolete IMHO, at least in the use cases I found. This is
  covered by tests as well.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-03-14 22:45:52 -10:00
ea6eb301a6 Bug 19532: Make recalls.status an ENUM
This patch makes the status attribute an ENUM, setting the default value
as 'requested' as well. The chosen names are easier to read than single
letters. Also, renamed F into fulfilled (this impacts methods names as
well). This is because 'finished' or 'completed' is more a synonym for
old => 1...

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-03-14 22:45:52 -10:00
Aleisha Amohia
1ddde85181 Bug 19532: (follow-up) Fixes along recall workflow
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-03-14 22:45:51 -10:00
Aleisha Amohia
b5d1736295 Bug 19532: (follow-up) Fix undef recall_id preventing fulfillment of recall
Signed-off-by: David Nind <david@davidnind.com>

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-03-14 22:45:51 -10:00
Aleisha Amohia
b800c2e68d Bug 19532: Other objects used in recalls feature
- biblio->recalls
- biblio->can_be_recalled
- item->recall
- item->can_be_recalled
- item->can_set_waiting_recall
- item->check_recalls
- patron->recalls
- Biblio.RecallsCount

and relevant tests
- t/db_dependent/Stats.t
- t/db_dependent/Koha/Item.t
- t/db_dependent/Koha/Biblio.t
- t/db_dependent/Koha/Patron.t
- t/db_dependent/XSLT.t
- t/db_dependent/Search.t
- t/db_dependent/Holds.t
- t/db_dependent/Circulation/transferbook.t
- t/db_dependent/Circulation.t

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

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-03-14 22:45:51 -10:00