- Remove leftover skeleton boilerplate comments in the atomicupdate
("Do you stuffs here", "# sysprefs")
- Fix typo in atomicupdate description ("preferene" -> "preference")
- Update POD on MapItemsToHoldRequests to mention the new
LocalHoldsPriorityScope gate
- Explicitly mock LocalHoldsPriorityScope in the existing Local Holds
Priority subtests that exercise CreateQueue/MapItemsToHoldRequests,
instead of relying on it falling through to the real DB default
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
To test:
1 - Set LocalHoldsPriority to "Don't give", set HoldsQueuePrioritizeBranch to "home library"
2 - have a bib with two items: both items available, Item 1 home and holdingbranch Centerville, Item 2 home and holdingbranch Fairfield
3 - place a hold (Hold 1) on the bib for a patron from Centerville , for pickup at Fairfield
4 - place a hold (Hold 2) on the bib for a patron from Fairfield, for pickup at Centerville
Establish behavior without LocalHoldsPriority
5 - build holds queue
6 - confirm the queue targets the FPL item for Hold 1 and the CPL item for Hold 2
7 - check both items out to a third patron (not one of your patrons with holds)
8 - check in Item 2, confirm it tries to fill Hold 1, click Ignore
9 - check in Item 1, confirm it also tries to fill Holds 1, click Ignore
10 - reset item locations: UPDATE items SET holdingbranch=homebranch;
Establish behavior with LocalHoldsPriority
11 - Set LocalHoldsPriority to "Give library", LocalHoldsPriorityPatronControl to "home library", LocalHoldsPriorityItemControl to "home library"
12 - build holds queue
13 - confirm the queue targets the CPL item for Hold 1 and the FPL item for Hold 2
14 - check both items out to a third patron (not one of your patrons with holds)
15 - build holds queue, confirm it has no targets
16 - check in Item 2, confirm it tries to fill Hold 2, click Ignore
17 - check in Item 1, confirm it tries to fill Holds 1, click Ignore
18 - reset item locations: UPDATE items SET holdingbranch=homebranch;
Apply patch, updatedatabase, restart_all
19 - confirm new system preference LocalHoldsPriorityScope, set to "checking items in and building the holds queue"
20 - repeat steps 11 - 18, no change in behavior
21 - set LocalHoldsPriorityScope to "checking items in"
22 - build holds queue
23 - confirm the queue targets the FPL item for Hold 1 and the CPL item for Hold 2
24 - check both items out to a third patron (not one of your patrons with holds)
25 - build holds queue, confirm it has no targets
26 - check in Item 2, confirm it tries to fill Hold 2
27 - check in Item 1, confirm it tries to fill Holds 1
Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Comments 43-51 identified a further problem with unconditionally coding
a newly auto-created authority's 008/11 (and 040$f) from the source bib
heading's thesaurus: when LinkerConsiderThesaurus is off (the default),
the linker matches new headings to existing authorities by text alone,
regardless of thesaurus. So a bib heading from one thesaurus (e.g. FAST)
can auto-create an authority coded for that thesaurus, and a later bib
heading with the *same* text but no thesaurus of its own (or a different
one) can then link to that same authority purely on the text match.
Saving that authority afterwards propagates its 008/11 and 040$f coding
back onto every bib heading linked to it - silently adding indicators and
subfields the cataloguer never set, on installs that never opted into
thesaurus-aware behaviour.
Before this bug, AutoCreateAuthorities always created LCSH-coded records,
so that propagation was always onto the coding most sites already expect.
This bug's earlier patches changed that default for every site regardless
of whether they use LinkerConsiderThesaurus, which is what comments 43/45
(reproduced by Pedro Amorim) demonstrated as disruptive.
Sites that haven't turned on LinkerConsiderThesaurus don't benefit from
thesaurus-aware authority creation anyway, since their linker never
searches by thesaurus - C4::Heading::_search and C4::Linker::Default
already gate thesaurus-consideration on this same preference for
matching. This change gates authority-creation coding on it too, so the
behaviour introduced by this bug only applies to installs that already
opted into thesaurus-aware linking.
Test plan:
1. prove t/db_dependent/Biblio.t
Note the new case 7 in the "codes new authorities with the source
thesaurus" subtest: with LinkerConsiderThesaurus off, a FAST heading
no longer codes the new authority as FAST, it keeps the
MARCAuthorityControlField008 syspref default instead.
2. Librarian-facing regression check: with LinkerConsiderThesaurus off
(the default), AutoLinkBiblios and AutoCreateAuthorities on, catalogue
a 650 with ind2=7 $2=fast and save. The new authority's 008/11 should
be whatever MARCAuthorityControlField008 says (LCSH 'a' by default),
not 'z'.
3. Confirm the LinkerConsiderThesaurus=on cases from the earlier patches
are unaffected: with it on, a FAST heading still codes 008/11='z' with
040$f=fast, and a MeSH heading still codes 008/11='c'.
Sponsored-by: Heythrop Library <https://heythroplibrary.co.uk/>
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
QA (comments 23-30) flagged a regression in the previous patches: the
guard around coding a new authority's 008/11 (and 040$f) from the source
heading's thesaurus was `if ( $heading->{thesaurus} )`, but
C4::Heading::MARC21::_get_subject_thesaurus() never returns a false
value - a heading with a blank/unrecognised indicator 2 (by far the
commonest real-world case) comes back as the placeholder string
"notdefined", and indicator 2 = 4 comes back as "notspecified". Both are
truthy, so the override always fired, unconditionally coding every new
authority's 008/11 as 'z' or '|' instead of respecting the site's
MARCAuthorityControlField008 default (which defaults to 'a'/LCSH) - a
disruptive behaviour change for the majority of headings, not just the
non-LCSH ones this bug is meant to fix.
Adds Koha::Authority::$MARC21_UNDEFINED_THESAURUS_VALUES, the subset of
the existing thesaurus table's keys ('notdefined', 'notspecified') that
mean "no thesaurus could be identified" rather than a genuine one, and
uses it in LinkBibHeadingsToAuthorities()'s guard so those two values now
fall through to the site's own syspref default, unchanged, exactly as
before this bug's patches.
Test plan:
1. prove t/db_dependent/Biblio.t
Note the two new cases in the "codes new authorities with the source
thesaurus" subtest: a blank-indicator-2 heading now keeps 008/11 at
the syspref default ('a') instead of being forced to 'z', and an
indicator-2=4 heading respects a customised syspref default instead
of being forced to '|'.
2. Librarian-facing regression check: with AutoLinkBiblios and
AutoCreateAuthorities on, catalogue a 650 with a blank/space
indicator 2 (e.g. `650 __ $a Some topic`, no $2) and save. The new
authority's 008 position 11 should read whatever
MARCAuthorityControlField008 says (LCSH 'a' by default), not 'z'.
3. Confirm the non-LCSH cases from the earlier patches are unaffected:
a FAST heading (ind2=7, $2=fast) still codes 008/11='z' with
040$f=fast, and a MeSH heading (ind2=2) still codes 008/11='c'.
Sponsored-by: Heythrop Library <https://heythroplibrary.co.uk/>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
When AutoCreateAuthorities creates a new authority for a bib subject
heading that didn't match an existing one, it never set 008 or 040 on the
new MARC::Record, so C4::AuthoritiesMarc::AddAuthority()'s own defaulting
always ran and hardcoded 008/11 to 'a' (LCSH), regardless of the source
heading's real thesaurus. A FAST, MeSH, or other non-LCSH heading (MARC
indicator 2 = 7 with subfield $2, or indicator 2 = 1-6) therefore always
got mis-coded as LCSH on creation. Because the authority was permanently
mis-coded, it could never be found again by thesaurus-aware matching
(LinkerConsiderThesaurus), so every later import of the same non-LCSH
heading created yet another duplicate authority instead of linking to the
one already created (see bug 42694 for a real-world report of this).
This calls Koha::Authority->default_marc21_008($thesaurus) to build the
new authority's 008 - correctly coding position 11 for the source
heading's actual thesaurus - and, for headings identified only by a raw
$2 code, adds a 040$f before calling AddAuthority(), whose existing "only
default if the field isn't already there" guards then leave our
correctly-coded fields alone. No change to AddAuthority() itself is
needed for this. This is MARC21-only (matches the existing marcflavour
branch already in this code path) and ships as an unconditional bug fix,
with no new system preference: it only changes how a *new* authority is
coded going forward and does not touch or merge any existing authority
records.
Test plan (librarian-facing):
1. Turn on the AutoLinkBiblios and AutoCreateAuthorities system
preferences (Administration -> System preferences -> Cataloging).
2. Catalog a new bibliographic record, or edit an existing one, and add a
650 field coded for a non-LCSH thesaurus, e.g.:
650 _7 $a Miracles $2 fast
(2nd indicator = 7, subfield $2 = fast; a FAST heading with no
existing matching authority in your catalogue).
3. Save the record. Koha auto-creates a new "Miracles" authority record,
since none existed yet to link to.
4. In the Authorities module, open the newly created "Miracles" record
for editing and switch to MARC view (Edit -> Edit record, or the "..."
menu -> Edit as new).
5. Click into the 008 field to open its editor plugin.
Before this fix: the "11- Subject heading system/thesaurus" dropdown
is set to "a- Library of Congress Subject Headings", and field 040 has
no $f subfield.
After this fix: that dropdown is now set to "z- Other", and field 040
has a $f subfield containing "fast" — correctly recording the real
source vocabulary instead of silently mis-labelling it as LCSH.
6. As a regression check, repeat steps 2-5 with a plain LCSH heading
instead, e.g.:
650 __0 $a Feminism
(2nd indicator = 0, no $2). Confirm the resulting authority's 008 still
shows "a- Library of Congress Subject Headings" as before — this fix
must not change behaviour for ordinary LCSH headings.
7. Optional, Elasticsearch installations only: with LinkerConsiderThesaurus
also turned on, re-import or re-save a bib with the same FAST heading
from step 2 a second time. It should now link to the authority created
in step 3 instead of creating a second duplicate — this is the actual
duplicate-authority problem reported in bug 42694.
Sponsored-by: Heythrop Library <https://heythroplibrary.co.uk/>
Signed-off-by: Clemens Gresser <cgresser@jesuit.org.uk>
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
AutoCreateAuthorities builds a brand-new authority record whenever a bib
subject heading (6XX) has no match, but always lets the record fall back
to C4::AuthoritiesMarc::AddAuthority()'s default 008/040, which hardcodes
008 position 11 ("Subject heading system/thesaurus") to 'a' (LCSH) no
matter what thesaurus the source heading actually uses. This is the first
step towards fixing that.
Adds two things to Koha::Authority:
- $MARC21_THESAURUS_TO_CONTROL_FIELD_008_11: a single-source-of-truth
lookup mapping a thesaurus name (as already derived by C4::Heading from
MARC indicator 2 / subfield $2, e.g. 'lcsh', 'mesh', or a raw $2 code
such as 'fast') to its MARC21 authority 008/11 code, per the LOC ad008
code list. This table previously only existed once, for a different
purpose, in Koha::SearchEngine::Elasticsearch::QueryBuilder (used for
building thesaurus search queries) - it's promoted here so both that
and the authority-creation fix that follows can read from one place
instead of two copies that can silently drift apart.
- default_marc21_008($thesaurus): builds the default 34-character body of
a new MARC21 authority's 008 field, honouring the
MARCAuthorityControlField008 system preference. When $thesaurus is
given, position 11 is set from the table above instead of whatever the
default/syspref value carries, falling back to 'z' (Other) for a
thesaurus with no dedicated code.
AddAuthority() is refactored to build its own (thesaurus-less, LCSH
default) 008 via this new method too, replacing its previous inline copy
of the same defaulting logic - pure refactor, no behaviour change.
Nothing thesaurus-aware calls it yet.
Sponsored-by: Heythrop Library <https://heythroplibrary.co.uk/>
Signed-off-by: Clemens Gresser <cgresser@jesuit.org.uk>
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The mutual-exclusivity check ran before the empty-element cleanup added
for bug 37075, so a bare -c (no value) left an empty string in
@letter_code, which the check saw as truthy and rejected even though
-c had effectively not been used. Move the cleanup above the check.
Also add the same mutual-exclusivity guard directly in
C4::Letters::SendQueuedMessages, since letter_code and
exclude_letter_code build the same search key and passing both would
otherwise let one silently clobber the other in the hashref passed to
->search - only the CLI script enforced this until now, so any other
caller could hit the silent-clobber bug.
Adds regression coverage: unit tests for the new exception in
SendQueuedMessages, and a script-level test reproducing the RM's
reported false positive.
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Some libraries want to send digest messages once per day (e.g., at 20:05)
but send all other messages every hour. Currently, this requires listing
almost all letter codes (~85) in the hourly cron job, which is error-prone
and makes it easy to forget notifications at upgrades.
This patch adds a new --exclude-code (-x) option to process_message_queue.pl
that allows excluding specific letter codes from processing, making it much
easier to configure digest scheduling.
Changes:
- Added --exclude-code option to process_message_queue.pl (repeatable)
- Modified C4::Letters::SendQueuedMessages to handle exclude_letter_code parameter
- Supports both scalar and array reference formats
Example usage:
# Send all messages except digests every hour
00 */1 * * * process_message_queue.pl --exclude-code DUEDGST \
--exclude-code PREDUEDGST --exclude-code HOLDDGST \
--exclude-code AUTO_RENEWALS_DGST
# Send all messages including digests once per day
05 20 * * * process_message_queue.pl
Test plan:
1. Apply both patches
2. Run: ktd --shell --run 'prove t/db_dependent/Letters.t'
3. Verify all tests pass (should show 105 tests passing)
4. Create test messages in message_queue with different letter codes:
- Insert messages with codes: DUEDGST, PREDUEDGST, ACQ_NOTIF, HOLD
5. Test single exclusion:
misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST
- Verify DUEDGST messages remain pending
- Verify other messages are processed
6. Test multiple exclusions:
misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST \
--exclude-code PREDUEDGST
- Verify both digest messages remain pending
- Verify non-digest messages are processed
7. Test combined with --code option (should work together):
misc/cronjobs/process_message_queue.pl --code ACQ_NOTIF \
--exclude-code DUEDGST
- Verify only ACQ_NOTIF messages processed (DUEDGST excluded anyway)
8. Test help output:
misc/cronjobs/process_message_queue.pl --help
- Verify --exclude-code option is documented
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: Marion Durand <marion.durand@biblibre.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Removes the deprecated _GetCircControlBranch function and its export
from C4::Circulation. Removes the corresponding tests from
Circulation.t (now covered by t/db_dependent/Koha/Policy/Circulation.t).
Test plan:
1. Apply patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Circulation.t \
t/db_dependent/Koha/Policy/Circulation.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: Alex Carver [Acerock7] <alex@rcls.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
(cherry picked from commit 8e784622815a2db2ce6f558987d7c0b0951ceec0)
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
# Conflicts:
# C4/Circulation.pm
# t/db_dependent/Circulation.t
- cleanup_database.pl: rewrite DeleteSpecialHolidays to purge the new
library_single_closures and library_closure_exceptions tables via their
Koha::Objects classes (was querying the dropped special_holidays table).
- kohastructure.sql: fix the comment header above library_repeating_closures
that still referred to repeatable_holidays.
- C4::Overdues: drop the dead GetSpecialHolidays / GetRepeatableHolidays /
GetWdayFromItemnumber routines (unreferenced, exported nowhere) that still
queried the dropped special_holidays / repeatable_holidays tables, along
with the now-unused Date::Calc and Date::Manip imports.
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
insert_single_holiday and insert_exception_holiday delegate to
Koha::Library::Calendar::SingleClosure/Exception, but stopped updating
$self->{single_holidays}/{exception_holidays} on the C4::Calendar object
itself, unlike the sibling insert_week_day_holiday/insert_day_month_holiday
methods which still do. isHoliday() reads exactly those hashes, so a date
just inserted via insert_single_holiday/insert_exception_holiday would read
back as "not a holiday" for the rest of that object's lifetime - reachable
from tools/newHolidays.pl and tools/exceptionHolidays.pl, which loop over
several dates on one $calendar object, checking isHoliday() before each
insert to skip duplicates.
While adding a regression test, also found that _init() has the same class
of bug on the DB-load path: it builds the single/exception lookup keys from
`split(/-/, $row->{date})`, which keeps the zero-padding from the DATE
string (e.g. "06"), while isHoliday() numifies its lookup key (e.g. "6").
Any single/exception holiday with a single-digit month or day was therefore
invisible to isHoliday() even on a freshly reloaded C4::Calendar object.
Both call sites now numify year/month/day before building the hash key.
Test plan:
1) prove t/db_dependent/Holidays.t t/db_dependent/Calendar.t
t/db_dependent/Koha/Library/Calendar.t t/db_dependent/HoldsQueue.t
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Koha::Calendar is, by construction, a per-library object — it croaks
without a branchcode argument and every method is branch-scoped. The
top-level namespace hides that. Move the whole tree under
Koha::Library::Calendar to match the actual semantics, the existing
/libraries/{library_id}/calendar REST routes, and the established
Koha::Library::* convention (Group, Hour, FloatLimit, OverDriveInfo).
This commit:
- renames the Koha::Calendar(::*) tree to Koha::Library::Calendar(::*)
- updates package declarations, POD and internal 'use' statements
within the moved tree
- updates the four DBIC Schema Result overrides (koha_object_class /
koha_objects_class) to the new namespace
- updates callers inside C4::* modules (Calendar, Circulation,
Reserves, Overdues, HoldsQueue), Koha::{Patron,Hold,Checkouts,
CurbsidePickup}, circ/returns.pl, the installer and cronjobs
- updates the 42310-owned tests (Calendar tests, HoldsQueue.t,
Holds/WaitingReserves.t, Holidays.t, Koha/CurbsidePickups.t,
Reserves/CancelExpiredReserves.t and the libraries.t CRUD block)
Koha::Library->calendar, the REST controllers and the 42311 tests
are updated in separate follow-up commits under their respective
bugs.
Test plan:
- prove t/db_dependent/Koha/Library/Calendar.t
- prove t/db_dependent/Calendar.t
- prove t/db_dependent/Holidays.t t/db_dependent/HoldsQueue.t
- prove t/db_dependent/Holds/WaitingReserves.t
- prove t/db_dependent/Reserves/CancelExpiredReserves.t
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
When using the new Koha::Calendar::* classes for writing the CRUD API
endpoints I noticed that in my attempt to keep C4::Calendar similar to what it
was, I ended up with cache invalidation not always done in the right place.
The holidays cache (keyed by {branchcode}_holidays) stores dates and their
open/closed status, not titles or descriptions. This means only inserts and
deletes need to invalidate it — metadata updates (title/description) do not.
Following the pattern established by Koha::AuthorisedValue and Koha::ItemType,
I added store() and delete() overrides to Koha::Calendar::SingleClosure and
Koha::Calendar::Exception:
- store() flushes the cache only on insert (!in_storage), not on updates,
since the cache doesn't store titles or descriptions.
- delete() always flushes, since removing a closure changes the cached set.
This allowed removing the now-redundant _clear_cache calls from
Koha::Calendar::add_single_closure, add_exception, delete_single_closure,
delete_exception, and from C4::Calendar::ModSingleholiday and
ModExceptionholiday.
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Add to Koha::Calendar:
add_weekly_closure, add_repeating_closure, add_single_closure,
add_exception, delete_weekly_closure, delete_repeating_closure,
delete_single_closure, delete_exception, copy_to, _clear_cache
C4::Calendar write methods now delegate to Koha::Calendar instead
of using raw DBI. No old table references remain in C4::Calendar.
Tests in t/db_dependent/Koha/Calendar.t cover all CRUD operations:
create, verify via is_holiday, delete, copy_to (including
idempotency).
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
$due_interval and $due_date (and $recall itself) returned by
Koha::Recalls->add_recall were captured but never used in either
branch of the new rank => "recall" handling. Drop the unused
assignment - no behaviour change.
Test plan:
1) prove t/db_dependent/Holds.t t/db_dependent/Reserves.t - all pass
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
When an authorised value is limited to more than one library, the
no_limit query in GetAuthorisedValues joins authorised_values_branches
and returns one row per associated library, so the value was listed
several times in the item edition dropdown.
Group by the authorised value so each one is only returned once, and
aggregate the restricted flag with MIN() so the value stays
unrestricted as soon as one of its libraries matches the current one.
Sponsored-by: BibLibre
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
When we set an authorised value linked to one library,
users who do not belong to this library do not get the
value and can therefore overwrite it.
This patch add current value to the dropdown list even if it comes from an authorized value of another library.
Test plan :
1) Edit an item with one of these fields has an authorized value from
another library.
2) On 'edit' you will see AV but with '(Not an authorised value)' string
and a warn
3) Apply this patch
4) Refresh page and edit item again
5) Now if the authorised value belongs to another lib and is the current
value, you will see it with a tooltip and a warn. But if there are
authorised values restricted to another lib and isn't current value,
normally it will be not present in the option list.
Sponsored-by: BibLibre
Signed-off-by: Mathieu Saby <mathsabypro@gmail.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The code was using the patron's home library instead of passing the pickup-location
of the original hold. This is a simple variable change in 2 places for record level and item
level holds.
A) Set 'UseRecalls' to 'staff' at http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=UseRecalls
B) At http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl set the default rule: 'Recalls allowed' = 5, 'Recalls per record' = 3, 'On shelf recalls allowed' = 'if any unavailable' (not 'all').
C) Use biblionumber '437' (has just one item, barcode '39999000019186'), check that item out to 'Henry Acevedo' (Patron A).
D) Place a record-level hold on it for 'Johnny Bryan' (Patron B), who's from 'Midway' — but set the hold's pickup location to 'Centerville' instead.
E) On http://localhost:8081/cgi-bin/koha/reserve/request.pl?biblionumber=437, tick the hold, click 'Convert selected to recall', confirm.
F) Check the new recall's pickup location on http://localhost:8081/cgi-bin/koha/recalls/request.pl?biblionumber=437
- Expected: 'Centerville'
- Without the patch applied, it will be the patron's home library: Midway
- With the patch applied, the pickup-location of the hold is kept: Centerville.
G) Cancel the recall.
H) Repeat the test plan, but in step D) place an item level hold instead of a record level hold.
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This enhancement adds the ability to place recalls via the staff
interface by changing the UseRecalls system preference to have choices which
describe where recalls can be placed. Recalls can either be placed via
the OPAC, or the staff interface, or both. To be set to OPAC would be
consistent with current 'on' behaviour. UseRecalls is turned off by default.
This enhancement also adds the ability to convert existing holds to
recalls easily via the staff interface. The hold will be cancelled
and a recall placed using the hold's information.
To test:
1) Update database and restart services
2) Set UseRecalls to OPAC and set the relevant recalls circulation rules
OPAC only by default
3) Check out Item A to Patron B.
4) Log into the OPAC as Patron A. Confirm you can place, view and cancel
recalls as normal.
5) In the staff interface, set UseRecalls to Staff interface only.
6) In the OPAC, confirm you can still view and cancel recalls, but can
no longer place recalls.
7) In the staff interface, search for Item A and go to the Holds tab.
Place a hold for Patron C.
8) Check the checkbox for your hold and confirm the counts in the buttons above the Holds table are incremented as expected.
9) Click the button to Convert selected holds to recalls, a modal should pop up. Confirm all of the information is correct.
10) Confirm the conversion and once the page refreshes, your hold should be correctly cancelled using the new RECALLED reason.
11) Go to the Recalls tab. Confirm your hold has been converted to a
recall, and details like the patron, expiration, and pickup location have
carried across to the recall.
12) On this Recalls page, use the patron search to find Patron D. Test
placing a record-level recall by choosing to recall the next available item.
13) When the page refreshes, confirm the recall was successfully placed
with the correct details stored.
14) Repeat steps 12 and 13 with Patron E, this time place an item-level
recall by choosing an item in the 'place a recall on a specific item'
table. When selecting one of these items, the 'recall next available
item' section should become disabled.
15) Set UseRecalls back to OPAC only. Go back to the record and
view the Recalls tab.
16) Confirm you cannot search for a patron to place a recall, but can
still view and cancel recalls. Confirm you also cannot convert holds
to recalls.
17) Set UseRecalls to both staff interface and OPAC. Confirm you
can place recalls on both interfaces.
18) On the staff interface, test placing recalls that your circulation
rules do not allow. For example, if 'recalls per record' is 1, ensure
you're blocked from placing a second recall on a record for a patron. Another example, testing converting a hold to a recall when there are NO checked out items, as recalls can only be placed if something is checked out.
19) Place a hold for one of the patron's that already has a recall.
Once complete, try to convert this hold into a recall. Confirm you
are blocked and shown a message that the hold cannot be converted to a
recall.
20) Confirm patron messaging preferences for recalls notices can still be configured as expected.
21) Ensure tests pass:
* t/db_dependent/Circulation.t
* t/db_dependent/Circulation/transferbook.t
* t/db_dependent/Holds.t
* t/db_dependent/Koha/Biblio.t
* t/db_dependent/Koha/Item.t
* t/db_dependent/SIP/Transaction.t
* t/db_dependent/Search.t
* t/db_dependent/XSLT.t
Sponsored-by: Auckland University of Technology
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Since it is not clear what checkin_library signifies when marking an
issue on a lost item as returned we might as well fallback on setting
it to the issuing library when there is no userenv.
Test plan
* Run the unit test t/db_dependent/Koha/Account/Line.t
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
To be more flexible we could allow the top-level domain instead of
having a predefined list.
However this patch still list the different available options (and adds
Brazilian) for easy selection.
Note that this system preference is not used to retrieve the image but
only to link to from the image. Contrary to what the reporter of the bug
report thought. And actually what I thought as well until I finished to
write the patch and started testing it!
Anyway now the patch is written, there should not be any changes in
behaviour. Most of the time the links are actually broken when not using
.com. So I think we either broke the whole feature at some point, or it
never really worked. On a separate bug we could eventually have a look
at fetching from the locale tld.
Test plan:
Turn on OPACAmazonCoverImages and AmazonCoverImages
http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=5http://localhost:8080/cgi-bin/koha/opac-detail.pl?biblionumber=5
=> You see the PBP cover
click on it, it opens the lightbox, click "(see the original image)" at
the bottom
=> This is where the syspref AmazonLocale/AmazonLocaleTld acts
Modify the syspref's value and confirm that the link is modified and
point to the correct domain (leading to 404 or broken pages most of the time,
at least in my tests).
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
- overdue_notices.pl: fix verbose sprintf - remove spurious \$triggered
argument (CLI 0/1 flag had replaced delay value in output) and use
the \$borr string that was computed but never included in the message
- overdue_notices.pl: remove %already_queued hash which was populated
in _enact_trigger but never read; dead code since the refactor
- C4/Overdues.pm GetBranchcodesWithOverdueRules: guard the global-rule
expansion with @branchcodes check so an empty result (no rules at all)
correctly returns an empty list instead of all branches
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch updates the link from the tools homepage to open the new
circulation triggers administration page.
We then remove the old overduerules controller and template, and finally
we drop the no longer referenced
C4::Overdues::GetOverdueMessageTransportTypes method and associated unit
tests.
Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch updates the Koha::Patron->has_restriction_overdues method to
refer to the migrated overdue rules.
We also correct an oversight in the method prior to this patch that
meant that not all overdues were being correctly checked for a
restriction.. we were incorrectly filtering to only overdues at the
current checkin branch.
Finally, we also take account of the itemtype level addition here too.
NOTE: There weren't any unit tests for this Koha::Patron method.. we
should add some here!
Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch updates the C4::Overdues method
GetBranchecodesWithOverdueRules to refer to the new location of these
rules in the circulation rules table. We also update the unit tests to
move the mocked data into the right location.
We could perhaps migrate this method in Koha::CirculationRules at a
later date?
Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Linked account hold pickup check now applies to all hold
statuses (Reserved, Transferred, Processing), not just Waiting.
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Kellie Stephens <kellie.stephens@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
d43945e7279 moved the issued/patron data-consistency check earlier in
AddReturn (before blocker handling), as its commit message describes,
but only copied the block instead of deleting the original. The
leftover block ran unconditionally afterwards: since $patron is
already guaranteed set whenever $issue is true by that point, its
"else" branch fired on every normal checkin of an already-issued item,
incorrectly setting $messages->{NotIssued} and clearing item.onloan.
This corrupted the not_issued message surfaced to circ/returns.pl, SIP,
and the checkin audit record on every ordinary return.
The same leftover block also buried $iso18626_request's assignment
after a die (making it dead code), so ISO18626 progress-reporting on
checkin was never triggered. Restore the assignment at the point
$issue/$patron are extracted from the availability context, where it
belongs.
Test plan:
1. Return an item that is currently checked out.
2. Confirm the returned $messages hashref no longer contains a
NotIssued key, and that item.onloan is only cleared for items that
were genuinely not checked out.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Restore the withdrawn warning in Koha::Item::Checkin::Availability that
was incorrectly removed. This warning is set when an item is withdrawn
but BlockReturnOfWithdrawnItems is off, making it useful for API
consumers in subsequent work.
Update AddReturn to read the withdrawn state from the availability
result's warnings rather than re-checking $item->withdrawn directly,
avoiding duplication of logic.
Add test for the withdrawn warning (non-blocked) path.
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Move the AllowReturnToBranch and branch transfer limit checks from both
C4::Circulation::CanBookBeReturned and
Koha::Item::Checkin::Availability::_check_return_policy into a single
authoritative method on Koha::Item: can_be_returned_at().
This eliminates the duplicate implementations and follows the Koha
convention of placing item-related logic on the Koha::Item object.
Changes:
- Add Koha::Item->can_be_returned_at() with AllowReturnToBranch policy
and transfer limit checks
- Make CanBookBeReturned a thin wrapper delegating to the new method
- Update Checkin::Availability to call $item->can_be_returned_at()
- Remove _check_return_policy and unused withdrawn warning from
the Availability class
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The refactoring changed the relative order of the Wrongbranch early
return and several status-update operations. In the original AddReturn,
the execution order was:
1. checkout/patron lookup
2. not-issued handling + RecordLocalUseOnReturn
3. withdrawn check (early return)
4. location_update_trigger
5. UpdateNotForLoanStatusOnCheckin
6. CanBookBeReturned / Wrongbranch check (early return)
7. BlockReturnOfLostItems
After the refactoring, the Wrongbranch blocker returned early before
steps 2, 4 and 5, causing three regressions:
- RecordLocalUseOnReturn was not recorded for non-issued items
scanned at the wrong branch
- location_update_trigger was skipped for Wrongbranch returns
- UpdateNotForLoanStatusOnCheckin was skipped for Wrongbranch returns
Fix by splitting blocker handling into two phases:
1. BlockedWithdrawn: earliest early return, before status updates
(matches original withdrawn check position)
2. Wrongbranch and BlockedLost: after RecordLocalUseOnReturn,
location_update_trigger, and UpdateNotForLoanStatusOnCheckin
(matches original CanBookBeReturned position)
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The data inconsistency die (checkout exists but patron does not) was
placed after the blocker handling block. This meant that if an item had
corrupt data AND triggered a blocker (e.g. withdrawn+blocked), the
blocker handler would return early with patron as undef instead of dying
on the inconsistency, silently masking database corruption.
Move the check to immediately after extracting context from the
availability result, before any blocker handling, matching the original
AddReturn behavior where the die fired during checkout lookup.
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
There seems to be no reason to declare $patron near the start of the sub
while $issue is declared where they are both populated.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
- Rename params: branch => library, add to_library for destination
- Move $returnbranch computation before the availability check in
AddReturn so it can be passed as to_library
- Add transfer limit check (bug 7376) to the Availability class
- Rename internal _can_book_be_returned to _check_return_policy
- Remove duplicate $returnbranch block and standalone transfer
limit check from AddReturn
- Update tests for new param names
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch refactors AddReturn to use the new Koha::Item::Checkin::Availability
class for validation logic, eliminating code duplication and centralizing
check-in validation.
Changes:
- Add use statement for Koha::Item::Checkin::Availability
- Look up item first, return BadBarcode if not found
- Call $item->checkin_availability() for validation
- Use Result object methods (available(), blockers, context)
- Replace inline withdrawn/lost/branch validation with blocker handling
- Remove duplicate CanBookBeReturned() call (now in Availability class)
- Preserve all existing behavior and error messages
- Maintain backward compatibility
The refactoring extracts ~40 lines of validation logic while maintaining
identical functionality. All validation results are obtained from a single
availability check that returns a Result object.
Note: The withdrawn message is set in two places depending on context:
- When BlockReturnOfWithdrawnItems is ON: set in blocker handler before early return
- When BlockReturnOfWithdrawnItems is OFF: set by existing check later in the flow
This preserves the original behavior where withdrawn items always get the
message, but are only blocked when the syspref is enabled.
Test plan:
1. Apply patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/Item/Checkin/Availability.t
=> SUCCESS: Tests pass!
k$ prove t/db_dependent/Circul*
=> SUCCESS: Tests pass!
3. Verify check-in operations work correctly:
- Items not found return BadBarcode
- Withdrawn items blocked when syspref enabled
- Lost items blocked when syspref enabled
- Wrong branch returns blocked per AllowReturnToBranch
- Not issued items handled correctly
- Normal check-ins complete successfully
4. Sign off :-D
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This function is not used in the code anymore
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Collecting a booking early (within the bookings lead period) with a
due date override before the booking start syncs end_date to a date
that precedes start_date. Koha::Booking->store has no interval
validation and the clash checks' date windows never see the inverted
period, so the broken row is stored silently - reproduced via AddIssue
on this patchset.
- pull start_date back to the checkout date whenever the synced due
date precedes it, in both AddIssue and AddRenewal, so the booking
window follows the loan instead of inverting
- cover both paths in Circulation.t; the AddRenewal case needs the
Biblio::check_booking exemption from the previous patch, as the
synced window no longer overlaps the stored one
Test plan:
1. prove t/db_dependent/Circulation.t
2. Set bookings_lead_period to e.g. 5 for an itemtype and enable
SpecifyDueDate
3. Book an item of that itemtype for a patron starting in 3 days
4. Check it out to them today with an explicit due date of tomorrow
5. Without this patch the booking is stored with end_date before
start_date; with it both dates follow the checkout period
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
When a checkout satisfying a booking is renewed, the booking previously
kept its original end_date, blocking the booked period from other
patrons for the wrong window and leaving the two records out of sync.
This patch updates C4::Circulation::AddRenewal to pull the linked
booking's end_date along with the renewed due date, mirroring the
behaviour introduced for AddIssue earlier in this patchset. As at
issue time, if the new due date would clash with a subsequent booking
(possible when the renewal was explicitly allowed past a clash), the
booking keeps its original end_date.
Together with the previous patch this makes a renewal the natural way
to extend an issued booking: CanBookBeRenewed refuses renewals that
would run into the next booking, and AddRenewal keeps the booking in
step with the loan.
Test plan:
1) Run t/db_dependent/Circulation.t and confirm the new
'AddRenewal | booking end_date sync' subtest passes
2) Check out a booked item, renew it from the checkouts screen and
confirm the booking end date follows the new due date
Sponsored-by: OpenFifth <https://openfifth.co.uk/\>
Sponsored-by: Westminster City Council <https://www.westminster.gov.uk/\>
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
CanBookBeRenewed checked bookings by asking Koha::Item->find_booking
for the first booking that clashes with the renewal period. When the
renewing patron's own booking satisfies this checkout, that booking is
returned first and renewal is allowed - subsequent bookings for the
item were never considered. A patron could therefore renew a booked
item straight through the start of the next patron's booking.
Additionally, when staff renew with an explicit due date (the renewal
due date input on the checkouts screen, or the hard due date on
circ/renew.pl), CanBookBeRenewed validated a *calculated* due date
whilst AddRenewal then applied the *requested* one, so the requested
date was never checked against bookings at all.
This patch:
* repeats the booking lookup with the patron's own booking excluded,
refusing the renewal ('booked') when it would run into the next
booking for the item
* adds an optional $date_due parameter to CanBookBeRenewed, used in
place of the calculated due date for the bookings check, and threads
the requested date through from svc/renew and circ/renew.pl
Test plan:
1) Run t/db_dependent/Circulation.t - the extended
'CanBookBeRenewed | bookings' subtest covers both the calculated
and requested due date cases
2) Check out a booked item to its patron, add a subsequent booking for
another patron, and attempt to renew past the second booking's
start date from the checkouts screen; confirm the renewal is
refused as 'booked'
Sponsored-by: OpenFifth <https://openfifth.co.uk/\>
Sponsored-by: Westminster City Council <https://www.westminster.gov.uk/\>
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
When checking out a booked item, staff may override the suggested due
date. If that override runs into the next booking for the item, the
previous patch would silently keep the booking's original end date -
staff had no way of knowing their override crossed another booking.
This patch adds an exclude_booking_id option to Koha::Item->find_booking
so a lookup can skip a specific booking (here: the patron's own booking
being collected), and uses it in CanBookBeIssued to detect when an
explicitly requested due date would run into the next booking for the
item. When it would, a new BOOKED_DUE_DATE_CLASH confirmation is raised
and displayed on the checkout screen, telling staff the linked booking
will keep its original end date if they continue.
Only explicitly requested due dates trigger the confirmation -
calculated due dates are clamped to the booking end date later in the
staff checkout flow, so checking them would produce false positives.
SIP is unaffected as it never passes a requested due date.
Test plan:
1) Run t/db_dependent/Circulation.t and t/db_dependent/Koha/Item.t
2) Create two consecutive bookings on the same item for two patrons
3) Check the item out to the first patron specifying a due date beyond
the second booking's start date; confirm the new confirmation
message appears and checkout only proceeds once confirmed
Sponsored-by: OpenFifth <https://openfifth.co.uk/\>
Sponsored-by: Westminster City Council <https://www.westminster.gov.uk/\>
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
When a booked item is checked out, the due date defaults to the booking
end_date. Staff may override the due date at checkout time, which
previously left the booking end_date out of sync with the checkout,
blocking the booked period for longer or shorter than the actual loan.
This patch updates C4::Circulation::AddIssue to sync any due date
override back to booking.end_date the moment the booking transitions to
'issued', keeping the booking and the checkout aligned from the point
of issue. The existing Koha::Booking->store clash detection is used to
guard the update: if the overridden due date would clash with a
subsequent booking for the same item, the checkout proceeds with the
booking's original end_date instead.
Test plan:
1) Run t/db_dependent/Circulation.t and confirm the new
'AddIssue | booking end_date sync with due date override' subtest
passes
2) Create a booking for a patron on a specific item
3) Check the item out to the patron, overriding the suggested due date
4) Confirm the booking's end_date now matches the overridden due date
Sponsored-by: OpenFifth <https://openfifth.co.uk/\>
Sponsored-by: Westminster City Council <https://www.westminster.gov.uk/\>
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Andrew Fuerste Henry
Martin Renvoize
Tomás Cohen Arazi
Baptiste Wojtkowski
Thibaud Guillot
Lisette Scheer
Katrin Fischer
Aleisha Amohia
Laura_Escamilla
Andreas Jonsson
Jonathan Druart
Martin Renvoize
Jacob O'Mara
Kyle M Hall
Paul Derscheid