This patch adds a new template plugin, LinkPref, for displaying system
preference names as links depending on the logged-in user's permission.
Syntax:
[% "SYSTEM_PREFERENCE_NAME" | html | $LinkPref %]
For a user with 'CAN_user_parameters_manage_sysprefs' permission it
outputs:
<a class="link_preference" href="admin/preferences.pl?op=search&ok=Search&searchfield=SYSTEM_PREFERENCE_NAME">SYSTEM_PREFERENCE_NAME</a>
For a user without permission it outputs:
<span class="link_preference">SYSTEM_PREFERENCE_NAME</span>
Sponsored-by: Athens County Public Libraries
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
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>
Koha::SearchEngine::Elasticsearch::QueryBuilder kept its own copy of the
thesaurus-name to 008/11-code table used when building "thesaurus" search
queries against authority records. The previous commit moved that same
table to Koha::Authority as a shared lookup. This commit deletes
QueryBuilder's duplicate and points $thesaurus_to_value at
$Koha::Authority::MARC21_THESAURUS_TO_CONTROL_FIELD_008_11 instead, so
there is only one place to update if the code list ever changes.
Pure refactor: $thesaurus_to_value keeps its existing name and contents,
and build_authorities_query_compat() is unchanged, so query behaviour is
identical before and after.
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>
When a user initiates login from the IdP (e.g. Okta app tile), they
hit the OAuth endpoint without an existing session cookie. The code
crashed calling ->value on an undefined cookie object.
This patch:
- Removes the premature 'No user session found' guard that blocked
IdP-initiated flows entirely
- Creates a session on-the-fly for the initial request when no
cookie exists (IdP-initiated flow)
- On callback, redirects with wrong_csrf_token error instead of
crashing when the cookie is missing
CSRF protection is preserved: the state token is bound to the
session created in the initial request, and the browser returns
the cookie on callback for validation.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Koha::Hold::set_waiting calls Koha::Calendar::days_forward to derive the
holds-shelf expiration date when ExcludeHolidaysFromMaxPickUpDelay is on.
days_forward iterated next_open_days($dt, 1) ReservesMaxPickUpDelay times,
and next_open_days consults get_push_amt for every closed day it
encounters. Under useDaysMode=Dayweek, get_push_amt returns 7 for any
closure that is not a permanently closed weekday -- so an intermediate
Bank Holiday Monday inside the pickup window pushed the cursor a whole
week forward instead of one day, inflating the expiration by seven days.
Dayweek is a loan-due-date concept: it exists to keep a Friday-due loan
due on a Friday across closures. A patron does not pick a weekday for a
hold-shelf collection; staff set the expiration when they trap the item,
and the only thing that should drive it is how many open days the patron
should have to come and collect. days_forward is now decoupled from
useDaysMode and always counts forward one open day at a time, ignoring
closures without consuming them from the count.
next_open_days, prev_open_days, get_push_amt and addDays are unchanged --
loan due-date math keeps the existing Dayweek behaviour.
Test plan:
1. Inside KTD, run the updated unit tests:
prove t/db_dependent/Calendar.t
prove t/db_dependent/Hold.t
Confirm both pass, including the new Bug 42608 sub-tests. These cover
the full matrix of starting weekdays and useDaysMode values; the
manual steps below are a single confirmatory pass through the UI.
2. Manual reproduction (UI only -- no database or CLI access needed):
- Set sysprefs: useDaysMode=Dayweek, ReservesMaxPickUpDelay=6,
ExcludeHolidaysFromMaxPickUpDelay=ON, ExpireReservesOnHolidays=ON.
- Administration > Calendar: on a test branch, add one single
(one-time) holiday -- not weekly/repeating -- on any weekday within
the next 6 days that isn't already closed (e.g. the day after
tomorrow, as long as it isn't a Sunday or other closed day). This
one-off closure inside the pickup window is what triggers the bug.
- Place a hold on an available item, then check the item in at that
branch and confirm/trap it. This stamps waitingdate with today's
date automatically -- no manual date entry required.
- Check the hold's expiration date (Holds awaiting pickup list, or
the patron's holds tab):
Without the fix: expiration lands a full 7 days later than it
should (today + 6 + 7), from the week-jump.
With the fix: expiration is today + 6 open days, only skipping
the one closed day.
3. Verify the existing Calendar mode behaviour is unchanged:
- Switch useDaysMode to Calendar and repeat step 2 -- the same
expected expiration date should apply.
4. Verify loan due-date behaviour is unchanged:
- With useDaysMode=Dayweek, check out an item whose loan period would
place its due date on the same closed day used in step 2. Confirm
the due date still rolls forward by a whole week (the original
Dayweek intent), proving this patch did not regress
addDays/addDuration.
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: Jackie Usher <jackie.usher@westsussex.gov.uk>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The API that this feature was based on was removed by the Swedish
National Library, so we can remove the code and sysprefs from Koha.
Before testing:
- Run this SQL in the database and verify the syspref is empty:
SELECT value FROM systempreferences WHERE variable = 'OPACdidyoumean';
- Go to Administration > Did you mean? and verify there is a box for
LibrisSpellcheck
- Move the boxes around and click on "Save configuration"
- Run the SQL again and verify the contents of the syspref are now
something like this:
[{ "name": "AuthorityFile"}, { "name": "LibrisSpellcheck"}, { "name": "ExplodedTerms"}]
- Go to Administration > System preferences and search for "libris"
- Verify the two sysprefs LibrisKey and LibrisURL show up
To test:
- Apply this patch and make sure the DB update is run. In KTD you can
do:
$ sudo koha-upgrade-schema kohadev
The output from this should be like this:
Removed system preference 'LibrisKey'
Removed system preference 'LibrisURL'
'LibrisSpellcheck' was removed from the OPACdidyoumean syspref
- Run koha-upgrade-schema again. The output should be:
'LibrisKey' syspref already removed
'LibrisURL' syspref already removed
'OPACdidyoumean' syspref does not contain a setting for 'LibrisSpellcheck'
- Run the same SQL as before and verify that LibrisSpellcheck is now
gone:
[{"name":"AuthorityFile"},{"name":"ExplodedTerms"}]
- Go to Administration > Did you mean? and verify there is no box
for LibrisSpellcheck
- Move the boxes around and click on "Save configuration"
- Run the SQL again and verify that there is still no mention of
LibrisSpellcheck
- Go to Administration > System preferences and search for "libris"
- Verify the two sysprefs LibrisKey and LibrisURL do NOT show up
- Empty the OPACdidyoumean syspref by running this SQL:
UPDATE systempreferences SET value = '' WHERE variable = 'OPACdidyoumean';
- Run koha-upgrade-schema again. The output should be:
'LibrisKey' syspref already removed
'LibrisURL' syspref already removed
'OPACdidyoumean' is not set
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
1. apply html filter to letters so notice names are escaped (must
be done in attribute so the JSON format of the letters can
be parsed).
2. fix a correctness issue where the frontend was pinning the
logged in library instead of the logged in user's home library,
which would have led to 403s (correct) when saving rules. Since
the library dropdown is deliberately disabled unless a user has
the manage_circ_rules_from_any_libraries permission, the user
would then have needed to use 'Set library' to match their home
library to fix this. Prevent the issue by setting the library
to the user's home library.
Also rename the variable to user_library_id.
3. list_rules correctly references /api/v1/circulation_rules/kinds
4. make untranslatable strings translatable
5. swap svg for font-awesome icon.
6. avoid css duplication:
- remove dead / unused scoped css (leftover from an early component
extraction)
- import vue.css and only leave the minimal override required.
7. improve set_rules test coverage: add tests for manage_circ_rules
/ manage_circ_triggers separation
Test plan:
1) Administration > Circulation triggers: notice the 'Letter' dropdowns
list circulation notices as "name (code)" and offer 'No letter'.
(no regression)
2) Give a staff user 'catalogue' + 'manage_circ_triggers' only, with
home library A. Log in as them, 'Set library' to B, and open
Administration > Circulation triggers. Notice the view is pinned to
A, and that adding and editing a trigger saves without a 403.
(behaviour is now correct)
3) GET /api/v1/circulation_rules?rules=nosuchrule - notice the error
points at /api/v1/circulation_rules/kinds.
(behaviour is now correct)
4) Run 'gulp po:update --lang fr-FR'. Notice 'Add new trigger
{triggerNumber}', 'No letter' and 'any' now appear in
misc/translator/po/fr-FR-messages-js.po
(behaviour is now correct)
5) Edit a trigger. Notice the delay's reset button is an 'x' inside the
input, left of the chevrons, and clears the delay; the letter's
reset button does the same for the notice.
(no regression)
6) Notice no visual regression across the list, add, edit, reset and
delete screens - select widths, chevrons and modals unchanged.
Also run t/db_dependent/api/v1/circulation_rules.t
Assisted-by: Claude Opus 5 (Anthropic)
Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
- Add null check around Koha::Patrons->find() in guided_reports.pl
before calling ->has_permission, matching the defensive pattern
already used in Koha::Report::check_edit_permission
- Add missing description to Koha::Exceptions::Report base class
for consistency with its sub-exceptions
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Prevents possible template bugs from allowing editing
of reports by others without the proper permissions.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Adds Koha::Policy::Circulation->circ_control_library($item, $patron)
class method that resolves which library controls circulation rules,
based on the CircControl and HomeOrHoldingBranch system preferences.
This follows the same pattern as Koha::Policy::Holds->holds_control_library.
An optional pickup_library_id parameter allows callers in API/daemon
contexts to pass the library explicitly instead of relying on userenv.
Test plan:
1. Apply patch
2. Run:
$ ktd --shell
k$ prove 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>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch simplifies the userinfo response parsing by relying on
Mojo::Message::Response->json which decodes JSON regardless of
Content-Type header parameters. If JSON decoding fails (returns undef),
it falls back to URL-encoded form data parsing.
This fixes integration with APIs that return extra Content-Type
parameters (e.g. Microsoft Graph returns odata.metadata,
odata.streaming, and IEEE754Compatible alongside charset) which
previously caused the strict regex to fail and silently misparse
the response as form data.
Test plan:
1. Apply the regression test (previous patch)
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/Auth/Client/OAuth.t
=> FAIL: Subtest fails - Content-Type with extra params not recognized
3. Apply this patch
4. Repeat step 2
=> SUCCESS: All tests pass
5. Sign off :-D
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The koha-qa pod_coverage check flags 'sub _type has no pod coverage'
in Koha/Checkouts.pm. The POD block above the sub was headed '=head3
type' while the sub is named '_type', so the documentation was not
associated with the sub. Correct the heading to '=head3 _type' to
match, mirroring the adjacent object_class POD.
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
add_weekly_closure/delete_weekly_closure and add_repeating_closure/
delete_repeating_closure returned $self without refreshing
$self->{weekly_closed_days}/{day_month_closed_days}, so a calendar object
would keep reporting the pre-change state to is_holiday (and everything
built on it - days_forward, next_open_days, etc.) until it was
re-instantiated. This is distinct from the _holidays Koha::Caches entry,
which single/exception closures already flush correctly on store/delete.
Call _init to rebuild both structures from the database after each
add/delete, so the same object reflects the change immediately.
Test plan:
1) prove t/db_dependent/Koha/Library/Calendar.t t/db_dependent/Calendar.t
t/db_dependent/Holidays.t t/db_dependent/HoldsQueue.t
t/db_dependent/Holds/WaitingReserves.t t/db_dependent/Koha/CurbsidePickups.t
t/db_dependent/Reserves/CancelExpiredReserves.t
t/db_dependent/api/v1/libraries.t
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
store() only flushes the _holidays cache on insert, on the assumption that
updates only ever touch title/description. That assumption held for every
current caller (ModSingleholiday/ModExceptionholiday only ever change those
two fields), but nothing enforced it - a future caller updating date in
place would silently desync the cache for up to its TTL, with no error.
Make the assumption explicit: reject an update that changes date, rather
than silently allowing it. Callers that need to move a closure/exception to
a different date must delete and re-add.
Test plan:
1) prove t/db_dependent/Koha/Library/Calendar.t t/db_dependent/Calendar.t
t/db_dependent/Holidays.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>
C4::Calendar::insert_week_day_holiday used to croak "Invalid weekday
$weekday" unless $weekday =~ m/^[0-6]$/. That guard was dropped when the
insert delegated to Koha::Library::Calendar::WeeklyClosure, and nothing
replaced it - an out-of-range or undef weekday now persists silently to
library_weekly_closures instead of failing at the API boundary.
Add the check to WeeklyClosure->store() rather than the legacy wrapper, so
every caller (C4::Calendar, Koha::Library::Calendar->add_weekly_closure,
and any future REST/admin caller) gets it, not just the old entry point.
Test plan:
1) prove t/db_dependent/Koha/Library/Calendar.t t/db_dependent/Calendar.t
t/db_dependent/Holidays.t t/db_dependent/HoldsQueue.t
t/db_dependent/Holds/WaitingReserves.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>
Introduces a public Koha::Calendar->closed_dates_in_range($from_dt,
$to_dt) method that returns an arrayref of ISO date strings representing
every closed day in the inclusive range, honouring weekly, annual
repeating, single and exception (open-override) closures.
This replaces callers that were reaching into Koha::Calendar private
state (_holidays, weekly_closed_days, day_month_closed_days) to
reconstruct the same logic. A follow-up under Bug 42206 converts the
REST controller; further callers can be migrated incrementally.
Test plan:
1. prove t/db_dependent/Koha/Calendar.t
2. Verify the new 'closed_dates_in_range' subtest passes
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Wrap the body in schema->txn_do so a failure partway through does not
leave the target library partially populated. Flush the target's
_holidays cache after the copy, and document the behaviour in POD:
weekly and repeating closures are always copied, but single closures
and exceptions are filtered to today-or-later dates by design.
Test plan:
1) prove t/db_dependent/Koha/Calendar.t
2) Confirm existing copy_to tests still pass.
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The four delete_*_closure / delete_exception methods in Koha::Calendar
were calling ResultSet->delete, which issues a single bulk SQL DELETE
and never calls the row-level delete() override. The cache-invalidation
hooks on Koha::Calendar::SingleClosure->delete and
Koha::Calendar::Exception->delete were therefore skipped when callers
went through $library->calendar->delete_single_closure(...) or
delete_exception(...), and the <library>_holidays cache kept reporting
deleted dates as closed until its TTL expired.
The underlying tables enforce UNIQUE constraints on (library_id, weekday),
(library_id, day, month) and (library_id, date), so each search can
match at most one row. Switch the four methods to ->search({...},
{ rows => 1 })->single followed by ->delete on the row, which both
makes the single-row expectation explicit and ensures the per-row
delete override runs.
A regression test is added to t/db_dependent/Koha/Calendar.t asserting
that the _holidays cache is cleared after delete_single_closure and
delete_exception.
Test plan:
1) prove t/db_dependent/Koha/Calendar.t
2) Confirm the new 'delete_*_closure clears _holidays cache' subtest
passes.
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>
Add koha_object_class/koha_objects_class overrides for the four new
Result classes backing the normalized calendar tables, below the
auto-generated marker.
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Regenerated via dbic --force after the library_weekly_closures,
library_repeating_closures, library_single_closures, and
library_closure_exceptions tables replaced repeatable_holidays and
special_holidays.
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
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>
Replace raw DBI queries in _init() and _holidays() with
Koha::Calendar::WeeklyClosures, RepeatingClosures, SingleClosures,
and Exceptions searches.
The internal data structures remain identical (weekly_closed_days
array, day_month_closed_days hash, _holidays hash with YYYYMMDD
keys) so all consumers (is_holiday, addDuration, next_open_days)
continue to work unchanged.
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>
Four Koha::Object + Koha::Objects pairs following the Koha::City(ies)
pattern:
- Koha::Calendar::WeeklyClosure(s) -> library_weekly_closures
- Koha::Calendar::RepeatingClosure(s) -> library_repeating_closures
- Koha::Calendar::SingleClosure(s) -> library_single_closures
- Koha::Calendar::Exception(s) -> library_closure_exceptions
DBIC schema files to be generated via the 'dbic' alias in KTD
after the database migration runs.
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>
UseRecalls was converted from a YesNo syspref to a Choice preference
("off"/"opac"/"staff"/"opac_and_staff") earlier in this bug, and every
call site was updated from a truthy check to `ne "off"` - except this
one. Since even the string "off" is truthy in Perl, the guard in
Koha::REST::V1::Transfers::cancel was silently defeated: cancelling a
transfer via the REST API always attempted to revert an in-transit
recall, even with recalls disabled.
Test plan:
1) prove t/db_dependent/api/v1/transfers.t - new subtest
'cancel() - UseRecalls disabled' fails against the old code
(the in-transit recall gets reverted even though UseRecalls is off)
2) Apply this patch
3) Same test now passes
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
As a continuation of bug 41919, we should limit the total number of simultaneous reports that can be run at a time.
Test Plan:
1) Set total_running_reports_per_instance_limit to 2 in your koha-conf.xml
2) Create 3 long running reports like: SELECT COUNT(*) FROM items i1 JOIN items i2 JOIN items i3 JOIN items i4 JOIN items i5 JOIN items i6
3) Run each report in turn by opening a new tab, browsing to the reports
module, and choosing to run that report
5) Note the first two continue to run, but the last one gives an error
that you have too many reports running simultaneously!
Signed-off-by: Alex Carver [Acerock7] <alex@rcls.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
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>
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>
Large files were failing to enqueue/process because the whole CSV was
embedded in the job's JSON args, hitting DB/broker size limits. This
now uploads the file via Koha::Uploader first (same pattern as MARC
staging) and only passes a small uploaded_file_id through the job,
and fixes the enqueue-failure message to not claim nothing happened
when a job was in fact created.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Introduces a new background job class for patron imports. enqueue() reads
the uploaded file content into the job_args and uses line count as job_size
for progress tracking. process() reconstitutes the file handle from the
stored content, delegates to Koha::Patrons::Import with step_callback wired
to $self->step, handles patron list creation, stores the import results, and
clears file_content from the stored data to free space.
Also registers patron_import in Koha::BackgroundJob::core_types_to_classes
so _derived_class can resolve the job correctly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Adds an optional step_callback coderef parameter to import_patrons(). When
provided it is called after every row — whether the row succeeds, is skipped
as a duplicate, or is rejected as invalid — so callers can track progress.
Also removes the 25-error cap that was there to protect browser response
size; that constraint no longer applies when results are stored in a
background job's data field.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Returns the count of overdue checkouts for a patron as a plain integer.
Computed live from the database (date_due < NOW()) to ensure accuracy,
as overdue status is time-based with no discrete event to trigger updates.
This endpoint is intended for async loading in the patron search table,
allowing the main search results to render immediately while overdues
are fetched separately per visible page.
Test plan:
1. Apply patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/api/v1/patrons_overdues_count.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Where a patron had multiple overdues that ought to cause debarment, it was found that removing one removed the restriction.
This is because has_restricting_overdues() was looking up the ItemType object tied to the overdue issue, then passing it down to Koha::CirculationRules->get_effective_rules(). get_effective_rules(), however, expects an itemtype code.
This resulted in false negatives: the patron was treated as having no restricting overdues left where they in fact did.
Amend this by calling effective_itemtype() instead.
Note on side effects: I think this also will activate the previously potentially dead dedup logic in has_restricting_overdues (eq had been used to compare obj instance to obj instance which had no effect (itemtype() runs a find() on each iteration), and is now used to compare itemtype code to itemtype code).
Test plan (based on Pedro's comment number 949):
0) Pick a test opac patron, if using Henry from above remove the permissions
1) http://localhost:8081/cgi-bin/koha/admin/categories.pl, edit the test patron's category, confirm 'Overdue notice required' is set to 'Yes'.
2) Set syspref AutoRemoveOverduesRestrictions to when_no_overdue_causing_debarment.
3) http://localhost:8081/cgi-bin/koha/admin/circulation_triggers.pl, 'Add new trigger'. Leave 'Library' as 'All libraries' and 'Patron category' as 'All categories', set 'Item type' to 'Books'. For 'Letter' pick ODUE, check 'Email' under 'Transport type(s)', set delay to 5 days, for 'Restricts checkouts' select 'Yes', 'Submit'.
4) Check out barcodes 39999000019186 and 39999000007558 to the test patron (confirm both are itemtype 'Books'). Backdate both due dates 6+ days overdue.
5) perl misc/cronjobs/overdue_notices.pl -n — restricts the patron.
6) Check the patron's 'Restrictions' tab: confirm restricted.
7) Return barcode 39999000019186 only, leave 39999000007558
8) Check the patron's 'Restrictions' tab again. Notice they are still restricted (39999000007558 is still overdue).
9) Return barcode 39999000007558 also.
10) Check the patron's 'Restrictions' tab again. Notice the restriction is now removed (a hard refresh may be needed) as no restricting overdues are left.
Assisted-by: Claude Opus 5 (Anthropic)
Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Important note: the following assumes manage_circ_triggers is intended
as a sibling to manage_circ_rules, and that of the two, only
manage_circ_triggers ought to grant access to client-side circulation
trigger configuration actions.
When manage_circ_triggers was introduced, it was not applied client-side,
resulting in the router and menu permission checks incorrectly expecting
manage_circ_rules. Amend that.
Additionally, while the API endpoint blocked a user with only
manage_circ_triggers from editing non-triggers circulation rules,
it did not block a user with only manage_circ_rules from editing
trigger circulation rules. Add a guard clause.
Test plan (based on comment 872):
A) Go to a patron's 'More' > 'Set permissions' tab. Check 'catalogue' (needed for basic staff login), and under circulation-rules sub-permissions, check only 'Manage overdue notice triggers' - leave 'Manage circulation rules' unchecked.
B) Log in as that patron, go to 'Administration' - 'Circulation triggers' link present (admin-home.tt now correctly checks CAN_user_parameters_manage_circ_triggers), 'Circulation and fine rules' is not present.
C) Visit http://localhost:8081/cgi-bin/koha/admin/circulation_triggers directly - you are granted access.
D) Confirm that you can create and edit circulation triggers.
E) Go to a patron's 'More' > 'Set permissions' tab. Check 'catalogue' (needed for basic staff login), and under circulation-rules sub-permissions, check only 'Manage circulation rules' - leave 'Manage overdue notice triggers' unchecked.
F) Log in as that patron, go to 'Administration' - no 'Circulation triggers' link present (as we require manage_circ_triggers), however 'Circulation and fine rules' is present.
G) Visit http://localhost:8081/cgi-bin/koha/admin/circulation_triggers directly - you see "Error: You do not have permission to access this page" and are prompted to log in as a different user (as we require manage_circ_triggers).
H) Go to http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl, and confirm that you can edit circulation rules for your library (no regression).
Assisted-by: Claude Opus 5 (Anthropic)
Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
We have nothing to do with it but since the patches change the file, it
trips the QA script.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Replaces the broad manage_circ_rules permission requirement on the
circulation triggers page with a new dedicated manage_circ_triggers
sub-permission (under parameters, flag 3).
- Add manage_circ_triggers to userpermissions.sql
- Update atomicupdate to add the permission, grant it to existing
edit_notice_status_triggers users, and remove the old permission
- Update admin/circulation_triggers.pl to require manage_circ_triggers
- Add manage_circ_triggers as an OR option on all circulation_rules
API endpoints (GET/PUT /circulation_rules, /kinds, /config)
- Restrict manage_circ_triggers users in the API controller to
overdue_* rule names only on write
Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
- Add OR-type x-koha-authorization to /circulation_rules, /circulation_rules/kinds,
and /circulation_rules/config endpoints so users with either
circulate:circulate_remaining_permissions OR parameters:manage_circ_rules
can access them
- Enforce manage_circ_rules_from_any_libraries in the set_rules controller:
users without this permission may only set rules for their own library and
cannot set default (*) rules
- Add explicit use C4::Auth qw(haspermission) import to CirculationRules.pm
- Lock the library selector in the Vue UI for users who lack
manage_circ_rules_from_any_libraries, defaulting them to their own library
- Update set_rules() tests to cover the new permission restrictions
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>
Lucas Gass
Jonathan Druart
Owen Leonard
Martin Renvoize
Tomás Cohen Arazi
Magnus Enger
Chloé Zermatten
Kyle M Hall
Pedro Amorim
Thibaud Guillot
Lisette Scheer
Aleisha Amohia
Nick Clemens
Victor Grousset/tuxayo