Remove the use of sip.enabled for systemd, keep for sysv
No other service using a .enabled file is controlled via systemd or sysv
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Documentation is wrong, -c doesn't work for z3950_responder.pl
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The koha-sip@.service unit gates startup on
/var/lib/koha/<inst>/sip.enabled, the per-instance "I want SIP on"
flag file. `koha-sip --enable` creates that flag in the same call
that runs `systemctl enable`; running a bare `systemctl enable
koha-sip@<inst>.service` only flips the systemd half, so the unit
ends up enabled-but-refusing-to-start.
With ConditionPathExists the failure mode is silent and the unit
ends up "enabled" at the systemd level but "Active: inactive"
(dead) with "Condition: start condition failed" and the journal
logs aren't a big help figuring out the problem.
This patch replaces the silent skip with a loud failure with a helpful message:
ERROR: SIP not enabled for instance <inst>. Use: koha-sip --enable <inst>
SIPconfig.xml is gated by AssertPathExists if it's missing the unit fails.
Either failure path lands the unit in failed state, surfaced by 'systemctl --failed'
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Adds PartOf=koha.target to koha@.target so that stopping or
restarting the global target propagates to all instance targets
and their services.
Ships a koha-common.service oneshot unit that delegates to
koha.target, so 'systemctl restart koha-common' works as the
drop-in replacement for the old 'service koha-common restart'.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The systemd units were missing most of the parameters that the SysV
init functions read from koha-conf.xml. This caused wrong worker
counts, missing log files, ignored batch sizes, and other config
being silently dropped.
This patch adds koha-generate-env, a helper script that reads
koha-conf.xml and writes an EnvironmentFile for each service.
Each unit runs it as ExecStartPre and then uses the variables.
Changes per unit:
- koha-plack@: workers, max-requests, access/error logs,
environment (development/deployment), per-instance psgi, -M FindBin
- koha-zebra@: loglevels, max_record_size, timestamp format
- koha-es-indexer@: batch_size
- koha-indexer@: INDEXER_TIMEOUT from /etc/default/koha-common
- koha-z3950@: config dir fallback to global, additional options
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
When upgrading from the old koha-common (pre-split) to the new
koha-core + koha-systemd packages, the SysV init script
/etc/init.d/koha-common is left behind as an orphan. The
systemd-sysv-generator auto-creates koha-common.service from it,
which runs in parallel with the native systemd units, causing
double-starts of all Koha services.
Changes:
- koha-systemd.postinst now stops, disables, and removes the
orphaned SysV init script when koha-sysv is not installed
- koha-sysv gets Replaces/Breaks on koha-common (<< 25.11) so
dpkg can cleanly take over the init script file
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
On first install of koha-systemd (migration from SysV), enable
units for all existing Koha instances. Core services (plack, zebra,
workers) are always enabled. SIP, Z39.50, and indexers are enabled
conditionally based on instance configuration.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
koha-core takes over files previously owned by koha-common
(e.g. /etc/default/koha-common). Without Replaces/Breaks, dpkg
refuses to overwrite them during upgrade.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Run debian/update-control to include koha-systemd and koha-sysv
package stanzas and refresh Perl autodependencies.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Replace per-service ExecStartPre=+koha-create-dirs (running as root
on every start of every service) with a single oneshot unit per
instance. Services declare Requires= on it, and the oneshot's Before=
ensures directories exist before any service starts.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
- koha-z3950-responder: add systemd enable/disable to enable_z3950/disable_z3950
- koha-disable: add plack and es-indexer stop/disable
- koha-remove: add worker --disable after --stop
- koha-systemd-ctl: fix service name worker-long to worker-long_tasks
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
- koha-sip: disable_sip() was calling 'enable' instead of 'disable'
- koha-indexer: enable/disable dispatch was calling start/stop functions
- koha-systemd.postinst: upgrade try-restart was outside the case block
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
In order for apt upgrades to trigger a service restart for all koha-systemd subservices and all instances, this adds a wildcard try-restart for those in addition to the daemon-reload. Only services which are already present and running will be restarted at postinst time
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch adds an elevated koha-create-dirs to the systemd units, so
that when the units (re)start, the directories are definitely present.
We found that PID files were missing and couldn't be created without
this patch.
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch adds more enable / disable flags to the koha daemon helper
scripts. This is essential as systemd does not enable units by default,
especially when using instantiation variables.
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
We found a bug when running koha-create. The systemd unit name didn't
match expected, so we've moved the unit to match the koha functions
computed value.
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch moves the update-rc.d lines from koha-core to koha-sysv, so
it only fires if systemd is not in use
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch removes or renames some of the debian files from koha-common
to koha-core, to avoid file clash during install
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch restructures the Koha packaging to support both systemd and
SysV init systems through a clean separation of concerns.
Package hierarchy:
- koha-core: Core application files (no init system)
- koha-systemd: Native systemd units (Provides: koha-init)
- koha-sysv: SysV init scripts (Provides: koha-init, Conflicts: koha-systemd)
- koha-common: Metapackage (Depends: koha-core, koha-systemd | koha-sysv)
Changes:
- koha-core: Removed 'daemon' dependency, updated description to clarify
it provides core files without init system
- koha-systemd: Now depends on koha-core instead of koha-common, provides
koha-init virtual package
- koha-sysv: Now depends on koha-core instead of koha-common, provides
koha-init virtual package
- koha-common: Converted to metapackage that depends on koha-core and
prefers koha-systemd on systemd-based systems
- Removed koha-common.install (files now installed by koha-core)
- Simplified koha-common.postinst (metapackage has minimal logic)
Benefits:
- Clean separation between core application and init system
- Users can explicitly choose their init system
- KTD can use koha-sysv without systemd
- Modern systems get native systemd by default
- Conflicts prevent both init systems being active simultaneously
Test plan:
1. Build packages
2. Install koha-common on a systemd system, verify koha-systemd is pulled in
3. Try installing both koha-systemd and koha-sysv, verify conflict
4. Install koha-core + koha-sysv explicitly, verify it works
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch introduces the koha-sysv package, which provides traditional
SysV init.d scripts for managing Koha instances. All services (Plack,
Zebra, SIP, Z3950, workers, indexers) are managed through a single init
script using the daemon wrapper.
The package includes:
- /etc/init.d/koha-common (SysV init script)
- /lib/systemd/system/koha-common.service (systemd wrapper for init.d)
This package is intended for legacy systems, containers without systemd
(like KTD), or environments where SysV init is preferred.
The package provides 'koha-init' and conflicts with koha-systemd to
ensure only one init system is active at a time.
Test plan:
1. Build and install the package
2. Start services with /etc/init.d/koha-common start
3. Verify all services start correctly
4. Check that koha-systemd cannot be installed alongside
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch introduces the koha-systemd package, which provides native
systemd service units for managing Koha instances. Each service type
(Plack, Zebra, SIP, Z3950, workers, indexers) is managed as a separate
systemd unit using instance templates.
The package includes:
- 8 service templates (plack, zebra, sip, z3950, worker, worker-long,
indexer, es-indexer)
- 2 target units (koha@.target for per-instance, koha.target for global)
- Helper script (koha-systemd-ctl) for managing services
All service units include:
- KOHA_HOME environment variable pointing to /usr/share/koha
- WorkingDirectory set to /var/lib/koha/%i
- Proper user/group permissions (%i-koha)
This allows granular control over individual services per instance and
leverages systemd features like dependency management, resource limits,
and journald logging.
Test plan:
1. Build and install the package
2. Enable services for an instance
3. Verify all services start correctly
4. Check journalctl logs show no errors
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
- 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>
Prove t/db_dependent/HoldsQueue.t
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>
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>
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>
The previous follow-up passed the checked-in itemnumber to the slip, but
the slip was still rendered before ModReserveAffect() confirmed the
hold. Fields updated during confirmation, such as waitingdate and
expirationdate, were therefore missing or stale on the first print.
Open a blank named popup during the user gesture, then submit the
confirmation form with print_slip set. The response page reuses that
popup after the hold has been updated.
This preserves the popup blocker fix and allows ReserveSlip() to use
the persisted hold and item data without passing individual fields as
workarounds.
To test:
1. Set HoldsAutoFill to "Don't".
2. Add the following to the HOLD_SLIP notice:
Waiting since: [% hold.waitingdate | $KohaDates %]
Barcode: <<items.barcode>>
3. Place a title-level "Next available" hold with pickup at the
check-in library.
4. Check in a matching item.
5. Click "Print slip and confirm".
6. Confirm the popup is not blocked.
7. Confirm the first slip contains the item's barcode and today's
waiting date.
8. Reload the slip and confirm the values are unchanged.
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
When returning an item required to fulfil a hold at another branch with
HoldsAutoFill set to "Don't", the librarian is shown a "Hold found"
modal and chooses "Print slip, transfer, and confirm". The previous
implementation set a hidden 'print_slip' input to 1, submitted the
form, and relied on $(document).ready on the response page to call
Dopop() / window.open() to open the slip. Because that window.open()
is no longer associated with a user gesture (the gesture chain is
broken by the form submission and page navigation), modern browsers
block it as an unsolicited popup. The hold is confirmed but the slip
never appears, and the librarian may not realise it was attempted.
This patch opens the slip synchronously inside the .print click
handler instead, while the user gesture is still active, so the
browser allows the window.open() through. The form is then submitted
as before to confirm the hold. The reserve_id is taken from the
hidden input already present in the modal form; hold-transfer-slip.pl
needs only the reserve_id and does not require the hold to have been
moved to "Waiting" state first, so the slip content is correct.
To test:
1. Set syspref HoldsAutoFill = "Don't".
2. Place a hold for a patron whose pickup branch is different from
your logged-in branch.
3. Check in the item at your logged-in branch.
4. The "Hold found" modal appears.
5. Click "Print slip, transfer, and confirm".
6. Confirm the hold transfer slip opens in a new window and is not
blocked by the browser's popup blocker.
7. Confirm the hold is still correctly confirmed and the item placed
in transit.
8. Repeat with HoldsAutoFill = "Don't" and a same-branch hold,
clicking "Print slip and confirm" instead, and confirm the slip
opens for that path too.
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: Jackie Usher <jackie.usher@westsussex.gov.uk>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
In case it got backported, we don't want to grant permissions again,
only when the db rev is run for the first time.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Adds a new permission to control access to the Item Modification by Age tool in the Cataloging Module.
Previously access to this tool was controlled by the items_batchmod permission. This can be an issue for consortiums
where staff members should be able to batch change items that belong to their library, but who shouldn't be able
to define item aging rules. Staff with automatic item aging tool access can use authorized values to change items
that belong to other libraries.
This patch introduces a dedicated permission (items_modification_by_age) to restrict access to the tool to users
with this permission.
- new permission definition via atomicupdate
- removal of dependency on items_batchmod for tool access
- updated permission UI display
- updated cataloguing-home button visibility
- updated flagsrequired check in tool script
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
QA (comment 58/60) suggested LinkerConsiderThesaurus only works with
Elasticsearch, having observed duplicate authorities being created when
testing with Zebra. Reproduced this directly: with AutoCreateAuthorities
and LinkerConsiderThesaurus on, saving two bibs with the same new FAST
heading back-to-back (no pause) creates a duplicate authority under
Zebra, but waiting for Zebra's indexing daemon to catch up (a few
seconds) between saves lets the second heading link to the first-created
authority correctly, as designed.
The same duplicate-on-rapid-succession behaviour also reproduces with
LinkerConsiderThesaurus off and a plain LCSH heading - it's a pre-existing
race between AutoCreateAuthorities and Zebra's asynchronous indexing
daemon, not something specific to thesaurus-aware matching or this bug.
Elasticsearch indexes near-synchronously within the same request, so the
same race window is far less likely to be hit there, which is almost
certainly why this only showed up in manual Zebra testing.
This documents that dependency on the LinkerConsiderThesaurus system
preference description, rather than gating the feature to Elasticsearch
only, since the underlying logic is search-engine agnostic and works
correctly on Zebra given normal indexing latency.
Sponsored-by: Heythrop Library <https://heythroplibrary.co.uk/>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
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>
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>
Add tests for the OAuth/OIDC login endpoint covering:
- IdP-initiated flow (no CGISESSID cookie on initial request)
- Callback without CGISESSID cookie (session lost)
- Normal SP-initiated flow (cookie present)
Tests create a real identity provider in the database and assert:
- Initial request without cookie redirects to the IdP authorize URL
- A CGISESSID cookie is set for the new session
- Callback without cookie returns wrong_csrf_token error (not 500)
- Normal flow with cookie redirects to IdP as expected
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>
If a user search for a local use syspref, the name will appear in the
autocomplete but when choosen "not found" will be return. This patch
suggests to redirect to the "local use" syspref view if we found a local
use syspref that match the search term.
Test plan:
- in admin, type "OPACdid" into the system preference search
- see "OPACdidyoumean" is offered as a search term
- click OPACdidyoumean
=> Without this patch you get an error that no such syspref exists
=> With this patch you are redirected to the local use syspref view
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: John Vinke <jvinke@austinseminary.edu>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
pay.tt, paycollect.tt and boraccount.tt all open printfeercpt.pl with
a change_given= query parameter after a payment, but printfeercpt.pl
only ever reads a 'change' param into its GetPreparedLetter substitute
hash - matching the ACCOUNT_CREDIT/CREDIT_* default notice templates,
which use [% change | $Price %], not [% change_given | $Price %].
As a result the change amount is silently blank on every printed fee
receipt that shows it, regardless of FinePaymentAutoPopup.
Rather than touch printfeercpt.pl or the notice content (both already
agree on 'change', matching a similar migration Koha did once before
in installer/data/mysql/db_revs/220600072.pl going the same direction),
fix the three query strings that build the printfeercpt.pl URL to
send change= instead of change_given=. The change_given variable name
is kept everywhere else (form fields, JS locals, Perl params) since
this is purely about the query key at the printfeercpt.pl boundary.
Test plan:
1) Set FinePaymentAutoPopup to 'show'
2) Take a payment for a patron via members/pay.pl or members/paycollect.pl
that generates change (fine amount not evenly covered by cash tendered)
3) Note the auto-popped-up receipt shows the change amount
(before this patch, the Change line was always blank)
4) Repeat via the boraccount.pl > printfeercpt.pl print receipt route
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>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This reverts commit 4fcff9db9e.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This fixes information displayed in the holds table for a patron after a single hold is suspended (in the "Check out" and "Details" sections for a patron record in the staff interface).
After suspending a hold, no holds are selected in the holds table, the select_hold_all checkbox is unchecked, and the holds select badge counter is 0.
Test plan:
0. Apply patch
1. Go to a patron with 2 or more active holds
2. Navigate to Details -> Holds tab
3. Click Suspend on a single hold and confirm
4. Verify the select-all checkbox remains unchecked
5. Verify the selected holds count is set to 0
6. Repeat steps 3-5 from the Check out -> Holds tab
Assisted-by: Opus 5 (Anthropic)
Sponsored-by: CLAMS
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch copies the implementation from the general background
worked into the es_indexer daemon
Note: There is a 'sleep(10)' in the general indexer and I copied it here,
but I don't know why we do it?
To test:
1 - Apply the patch
2 - sudo koha-es-indexer --restart kohadev
3 - perl misc/maintenance/touch_all_biblios.pl
4 - Wait a bit
5 - Check the background jobs in the admin interface
6 - Confirm the jobs finish and records are searchable
7 - Test with and without rabbitmq enabled:
. Stop rabbitmq service: sudo -s service rabbitmq-server stop
. Repeat steps 2 to 5
8 - Sign off
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Aleisha Amohia <aleisha@catalyst.net.nz>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch adds two entries mapping both addresses I commit from to a single
identity, so the generated files pick up the right name the next time
they are regenerated.
Test plan:
1. Apply patch
2. Run: git check-mailmap "lawrenceol-clams <loreganlloyd@clamsnet.org>"
=> SUCCESS: returns Lawrence O'Regan-Lloyd <loreganlloyd@clamsnet.org>
3. Run: git check-mailmap "lawrenceol <hello@lawrenceol.dev>"
=> SUCCESS: returns Lawrence O'Regan-Lloyd <loreganlloyd@clamsnet.org>
4. Run: git shortlog -sne | grep -i "regan-lloyd"
=> SUCCESS: a single entry, not two
Assisted-By: Claude Opus 5 (Anthropic)
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>
Kyle M Hall
Tomás Cohen Arazi
Jake Bateman
Jake Deery
Pedro Amorim
Andrew Fuerste Henry
Martin Renvoize
Slava Shishkin
Alex Carver [Acerock7]
Jonathan Druart
Lawrence O'Regan-Lloyd
Nick Clemens