Skips the empty results; only reports the 'real failures'.
Includes adding a grand total of hits to the form.
Some preliminary work to remove the hardcoded 20 results per target
in the code.
This is not yet a complete solution, but an improvement. The code in
C4/Breeding needs a larger overhaul. (And there might be an issue on
the Folio server side at LOC.)
Test plan:
Without this patch:
Use the LOC SRU server (since if frequently generates errcode 61).
Try the same search several times in a row (with Go). You probably will
come across the "LOC SRU record x: 0" messages. (Random behavior.)
Apply this patch.
Repeat the above. Notice that you will sometimes see the error 61 (and the
tip to refresh) but not the code 0 messages.
Run t/db../Breeding.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
As with guided reports through the admin interface, this patch switches
EmailReport to use the same function for processing templates. This
gives patron emails sent via cronjobs access to the same variables as
other report locations.
Test plan:
1. Under Tools > Notices and Slips, create a new notice in the Reports
module. Choose a code and a name, and under Email assign a subject
and the following body:
Date: [% today %]
2. Under Reports, create a new report from SQL with a name and the
following query:
SELECT 1 AS borrowernumber, 'to@example.com' AS email
3. Run the following command from the base of Koha, replacing CODE with
the notice code created earlier, and ID with the report ID created
earlier:
misc/cronjobs/patron_emailer.pl -module report -from from@example.com -notice CODE -report ID
4. Verify that the report runs successfully, and the console output
includes "Date:" on a line with nothing following it.
5. Apply patch.
6. Run the following command from the base of Koha, replacing CODE with
the notice code created earlier, and ID with the report ID created
earlier:
misc/cronjobs/patron_emailer.pl -module report -from from@example.com -notice CODE -report ID
7. Verify that the report runs successfully, and the console output
includes "Date:" on a line with the current time following it.
8. Run `prove t/db_dependent/Reports/Guided.t`, and verify it passes.
Sponsored-by: Open Fifth
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The FallbackToSMSIfNoEmail transport selection in ModSuggestion() was
checking $patron->email (the raw borrowers.email column) rather than
$patron->notice_email_address, which respects the EmailFieldPrimary and
EmailFieldPrecedence system preferences. Patrons with only emailpro or
B_email set were incorrectly receiving suggestion status-change notices
via SMS even when a valid email address was available.
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
SIP2 accounts/institutions are stored in the database and the SIP server is supposed to reload its configuration when they change.
In practice, after adding or editing a SIP2 account, most SIP worker processes keep serving the old configuration (e.g. new accounts get "SIP access not authorized") until the SIP server is restarted ( I tested multiple times over 30 minutes ).
There seem to be two issues:
* L1 cache flushed too late. process_request() calls _config_up_to_date() before Koha::Caches->flush_L1_caches(). The SIP parent reads sip2_resource_last_modified into its own L1 cache at startup and never flushes it. Every worker inherits that stale L1, so a worker's first connection compares against the startup-era value instead of the current shared value.
* sip2_config_read_timestamp is stored in the shared (memcached) cache, but the configuration it tracks is per-process. When one worker reloads the config from the database, it bumps the shared timestamp, so every other worker then evaluates as "up to date" and never reloads its own stale in-memory config.
Test Plan:
1) Configure the SIP server with several worker processes (e.g. set
min_servers and max_servers to 5) and confirm an existing SIP2
account works
2) Add a new SIP2 account in the staff interface
3) Without restarting the SIP server, test the new account with misc/sip_cli_emulator.pl
4) Note the login fails
5) Apply this patch
6) Restart all the things!
7) Add another new SIP2 account
8) Without restarting the SIP server test this new account with sip_cli_emulator.pl
9) Note that the new account works!
10) prove t/db_dependent/SIP/SIPServer.t
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
To test:
Test the cronjob purge_suggestions.pl
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>
ConnectDuplicate does not appear to be used anywhere.
No test plan to provide here.
git grep connectDuplicate should return nothing
Since there is no cud-, it would at least have been broken for a year
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>
MARC21 4xx/5xx $w is a control subfield with up to four positions;
only position 0 encodes the hierarchical relationship. The previous
regex required the whole subfield to equal g or h, so values such as
"g " or "gnna" would fall into the middle bucket and lose the
broader/narrower distinction.
This case is hypothetical and likely rare in practice (most catalogs
store a single character in $w), but the correctness improvement still
makes sense and aligns with how BuildSummary reads the same subfield
elsewhere via substr($w, 0, 1).
To test:
- prove -v t/db_dependent/AuthoritiesMarc.t
- Verify BuildSummary/_marc21_sort_hierarchy_alpha subtest passes,
including the new multi-position $w case.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
- koha-qa pod_coverage check flagged the new private helper
- sub as uncovered. Document it briefly to satisfy the check.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Resolve critic warn:
"return" statement followed by "sort" at line 1238, column 5. Behavior is undefined if called in scalar context. (Severity: 5)
Tidied with lastest perltidy version.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Test plan (MARC21):
[1] Create an authority record, say a term. Include two 450s, the first
one starting with Z and second with A. Include five 550s, where two
are narrower ($w==h), two broader ($w==g) and one without $w. Make
sure that the terms will sort differently than the field order (go
back in alphabet when filling subsequent $a's or so).
[2] Perform an authority search that includes the new authority.
[3] Verify that the terms in the Summary column for this authority
are sorted first on hierarchy, second alphabetically. The one
without hierarchy should be in the middle.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
Control fields (generally, just tags below 010) do not have subfields, use data() at /home/koha/Koha/Koha/SimpleMARC.pm line 460.
To test:
1. prove t/db_dependent/MarcModificationTemplates.t
2. Observe success
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Two patterns in the codebase produce spurious codespell hits because
codespell tokenises word fragments out of formatted letter codes:
- B<P>rovider in C4/Auth_with_shibboleth.pm produces 'rovider'
- T(ransit), W(aiting), F(inished), P(rocessing) in C4/Reserves.pm
produce 'ransit', 'aiting', 'inished', 'rocessing'
For the shibboleth POD line the bold-letter markup cannot be simplified
without losing the formatting intent, so a per-line # codespell:ignore
comment is the right suppression.
For the Reserves.pm status table the fix is made at source: the notation
is changed from X(word) to X (Word) so codespell sees the full English
word and no fragment is produced.
Because the per-line comment on the shibboleth line now suppresses
'rovider' directly, the global .codespell-ignore entry for that fragment
is removed as redundant.
Test plan:
1. Apply patch
2. prove xt/author/codespell.t
=> No new codespell failures
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Correct genuine spelling errors identified by the codespell 2.4.x
full scan after the KTD container OS upgrade. Changes are in
comments, POD, strings, and template text across 55 files.
"SAN Ouest Provence" (the French library co-operative and Koha
sponsor) is preserved as a proper noun. Perl copyright lines
receive an inline # codespell:ignore annotation; about.tt files
(contributor attribution pages) are added to the Koha::Devel::Files
codespell exception list so that future uses of "Provence" as a
typo for "Province" are still caught in all other files.
Notable user-visible fixes:
- "submiting" -> "submitting" in authority and bibliographic merge screens
- "deleteable" -> "deletable" in patron exception messages
- "Acquistions" -> "Acquisitions" in reports dictionary
Test plan:
1. Apply patches
2. In the staff interface, go to Cataloguing > Merge records and
confirm the submit button reads "Submitting" (not "Submiting")
3. In the staff interface, go to Authorities > Merge and confirm
the same correction
4. Attempt to delete a patron with outstanding issues and confirm
the error message reads "deletable" (not "deleteable")
5. prove xt/author/codespell.t
=> All tests pass
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
In Koha, the sip2_resource_last_modified key is updated any time the SIP config is edited from the SIP2 editor in the staff interface ( institutions, accounts, system preference overrides ).
If sip2_resource_last_modified is missing from memcached, Koha SIP emits "Couldn't find sip2_resource_last_modified, considering configuration not up to date" in the logs, and fetches the entire configuration from the database, *for each request*.
This causes SIP performance to drop significantly. In my production testing with 83 institutions and 171 accounts the login message response time dropped from 0.283 seconds to 2.899 seconds.
I think the solution is pretty simple. If there is a cache miss on sip2_resource_last_modified, SIP should load the config from database as it does already, but then it should also set sip2_resource_last_modified!
Test Plan:
1) Apply this patch
2) Prove t/db_dependent/Koha/SIP2/SIP2ModuleMigration.t
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Make 'Maximum total holds allowed (count)' (aka max_holds) consider hold groups
Test plan:
Same as the previous test plan, but set 'Maximum total holds allowed (count)' as 2 and instead set 'Holds allowed (total)' as unlimited.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Make 'Holds allowed (total)' (aka reservesallowed) consider each hold group as an individual hold
Test plan:
1) Enable DisplayAddHoldGroups sys pref
2) Set Holds allowed (total) to 2, visit:
http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl
3) Scroll to the right, click the 'Edit' button under 'Actions' column.
4) On the 'Holds allowed (total)' column cell, enter '2' and click 'Save' under the 'Actions' column to the right.
5) Search for 'test':
http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=test
6) Tick the checkbox next the first and second results, click 'Place hold' button at the top toolbar
7) Search for 'koha' user and pick Koha (42)
8) Set a pickup location for each hold and tick the 'Treat as hold group' checkbox. Click 'Place holds'.
9) Try to place another single hold for the same patron. Should succeed, because the 2 grouped holds count as 1. Patron now should have 1 group + 1 ungrouped hold = effective count of 2.
10) Try to place yet another hold, should be blocked with a tooManyHolds error.
11) Prior to this patch, step 9 would've failed because the 2 grouped holds counted as 2.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This will now find the corresponding ISO18626 request and update it accordingly when a check-out happens.
Use-case this is addressing:
1. Place an item-level hold for Henry
2. Check-in that item e.g. 39999000003130 (make it waiting)
3. Create a new ISO18626 request and 'Search to hold', place a bib-level hold on the same bib as above (for patron Walker)
4. Check-out the item from 2. 39999000003130 to Walker
5. Confirm an ISO18626 message input is shown and that the respective ISO18626 request is updated to 'Loaned' once the check-out happens (i.e. override Henry's hold).
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Authority MODIFY action log diffs were emitting bogus entries for every
field with a default ' ' indicator on every save, producing output like:
_marc.fields.1.040.ind1 ' ' ' '
_marc.fields.1.040.ind2 ' ' ' '
_marc.fields.2.150.ind1 ' ' ' '
_marc.fields.3.667.ind1 ' ' ' '
...
even when only a single subfield was edited.
Root cause: Struct::Diff treats a utf8-flagged ' ' as different from a
non-utf8 ' ' SV, even though Perl's eq returns true. ModAuthority was
sourcing the two sides of the diff from different parse paths:
- $oldrecord = GetAuthority($authid)
-> MARC::Record->new_from_xml(...) -> indicators arrive utf8-flagged
- $record (the "after" side) was the in-memory MARC built by the
editor from CGI parameters -> indicators are bare byte strings
The XML round-trip through AddAuthority normalises the stored copy, but
the in-memory $record we passed to logaction had not been re-parsed, so
every default indicator looked like a change to Struct::Diff.
Fix: after AddAuthority commits the new state, re-read it via
$new_authority->record and pass that to _authority_log_data. Both sides
of the diff now traverse the same new_from_xml path, utf8 flags match,
and Struct::Diff falls silent on unchanged indicators.
This is the symmetric counterpart of how biblios already work in
C4::Biblio: ModBiblio sources both $original_marc and $updated_marc_log
through Koha::Biblio::Metadata::record_strip_nonxml (which also calls
new_from_xml), so biblios don't hit this.
The deeper asymmetry between in-memory editor MARC and stored MARC
(default indicators only get normalised on the XML round-trip) is left
for a separate bug; this patch keeps the action log diff clean without
touching the storage path.
Test plan:
1. prove t/db_dependent/AuthoritiesMarc.t
2. Enable AuthoritiesLog. Edit any authority via the staff UI making a
single subfield change. Open Tools -> Log viewer for that authority
and verify the diff column lists only the changed subfield (plus the
005 timestamp), with no rows for ind1/ind2 of untouched fields.
3. Re-run the same edit a second time changing nothing else; the diff
should again be limited to 005 + the touched subfield.
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Mirrors bug 42188's biblio info-column convention. Generalise the
CATALOGUING-only info-column rewrite branch in C4::Log::logaction to
cover AUTHORITIES as well, using a module-aware prefix map so each
module gets its own prefix ("biblio" / "authority"). For AUTHORITIES
this means MODIFY now stores "authority {pre-change JSON}" and DELETE
stores "authority {final-state JSON}", while ADD stays bare
"authority" (the call site passes the string directly and ADD is
guarded anyway). The biblio behaviour is preserved bit-for-bit.
Test plan:
1. prove t/db_dependent/AuthoritiesMarc.t
2. Enable AuthoritiesLog. Create / modify / delete an authority via
the staff UI, then check action_logs:
- ADD row's info column reads just "authority"
- MODIFY row's info column reads "authority { ... pre-change JSON ... }"
- DELETE row's info column reads "authority { ... final-state JSON ... }"
3. prove t/db_dependent/Koha/Biblio.t
4. prove t/db_dependent/Log.t
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Mirrors the bug 42188 main commit for biblios. Replace the custom
_marc_record_to_diffable helper with MARC::Record::MiJ's to_mij_structure
so AUTHORITIES action log diffs use the same MARC-in-JSON shape as
CATALOGUING diffs. Add Koha::Authority::_unblessed_for_log to expose
only the auth_header columns that exist independently of the MARC
record (authtypecode, origincode, linkid), avoiding duplication of
MARC-derived data (heading, authtrees), raw marcxml, and system
timestamps. The explicit whitelist also removes the need for the
post-hoc empty-field stripping added in a previous follow-up.
Test plan:
1. prove t/db_dependent/AuthoritiesMarc.t
2. Enable AuthoritiesLog. Create, edit, and delete an authority via
the staff UI, then inspect action_logs.diff in the DB:
- _marc contains MiJ shape { leader: "...", fields: [ { tag: {...} }, ... ] }
- Changes to heading fields produce diffs on
_marc.fields[N].TAG.subfields[0].a
- The diff no longer carries a redundant top-level heading entry
(it's MARC-derived and now only appears in _marc).
3. prove t/db_dependent/Log.t
4. prove t/db_dependent/Koha/Authority.t
5. prove t/db_dependent/Koha/Biblio.t
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The AUTHORITIES action log entries for authority operations previously
stored minimal data: ADD and DELETE logged just the string "authority",
while MODIFY logged a raw MARC-formatted dump of the pre-change record
as a plain text string ("authority BEFORE=>..."). None of these populated
the diff column.
This patch updates AddAuthority, ModAuthority and DelAuthority in
C4/AuthoritiesMarc.pm to follow the same pattern introduced for biblios
in bug 40135: passing combined hashrefs to logaction so that the diff
column is automatically populated with Struct::Diff JSON.
The combined hashref contains the auth_header table columns (authtypecode,
heading, origincode, etc. — excluding the raw marcxml blob) plus a _marc
key built by the new _marc_record_to_diffable helper, which converts a
MARC::Record into a tag => array-of-formatted-strings structure readable
by the existing diff viewer.
A new _authority_log_data helper packages the authority unblessed data
and MARC record into the combined hashref, keeping the three call sites
concise.
For ADD, the logaction is now emitted after the authority record has been
fully stored (after _after_authority_action_hooks), so the diff captures
the complete saved state. For MODIFY, the before-state is captured from
Koha::Authorities before calling AddAuthority, and the after-state is
read back once AddAuthority has committed the new data. For DELETE, the
MARC is captured before the authority is removed.
The diff column for MODIFY will now show both authority table column
changes (e.g. heading) and MARC field-level changes (e.g. _marc.151.0)
in the existing diff viewer table in the log viewer.
Tests are added to verify that ADD, MODIFY, and DELETE all populate the
diff column and that _marc content is captured correctly.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The info column for CATALOGUING action logs is, by convention, the
record state before modification -- MODIFY writes the pre-change JSON
payload, DELETE writes the state at time of deletion, but ADD has no
pre-change state. Writing the new record state into info on ADD
duplicates the state that the subsequent MODIFY (if any) captures as
its "before" snapshot, and conflicts with that convention.
Skip the JSON payload on ADD so info degrades to the bare "biblio"
prefix. The diff column already captures the full creation (every
field "added" against an empty hashref), so no information is lost.
Test plan:
1. prove t/db_dependent/Koha/Biblio.t
2. Observe that the "CataloguingLog MARC-in-JSON diff tests" subtest
now also asserts:
- ADD info == "biblio"
- MODIFY info matches /^biblio \{/
- DELETE info matches /^biblio \{/
3. Create / modify / delete a record via the staff UI with
CataloguingLog on, then check action_logs: the ADD row's info
column should read just "biblio".
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Replace Dumper with to_json for the biblio info column in logaction,
fixing UTF-8 encoding and extending coverage to ADD and DELETE actions
which previously logged only the literal string "biblio".
All three actions (ADD, MODIFY, DELETE) now log "biblio " followed by
a pretty-printed JSON representation of the relevant state: pre-change
for MODIFY, new state for ADD, and deleted state for DELETE.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Replace the custom _marc_record_to_diffable helper with MARC::Record::MiJ's
to_mij_structure method, which produces standard MARC-in-JSON. This fixes
three problems with the old approach:
- Dollar signs used as subfield delimiters in the formatted string were
ambiguous when subfield content itself contained " $x " sequences.
- Grouping fields by tag into a hash lost inter-field ordering, making
lossless reconstruction of the MARC record impossible.
- Non-ASCII characters were subject to Perl string context encoding
issues; JSON serialisation of the MiJ structure handles UTF-8 cleanly.
_unblessed_for_log in Koha::Biblio is updated to return only the three
biblio columns that are genuinely independent of the MARC record
(frameworkcode, opac_suppressed, serial). The remaining columns
(author, title, subtitle, notes, etc.) are denormalised copies of MARC
data already captured in the _marc key, so including them produced
redundant and noisy diffs.
The fmt() helper in viewlog.js is updated to render JavaScript arrays
as ordered lists rather than objects with numeric keys, improving the
display of MARC subfield arrays in the diff table.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The "Cite" option in the OPAC is missing the subtitle (245) in MARC21.
To recreate:
1. Add a record or find a record with a subtitle (in KTD, biblionumber 58 has a 245 http://localhost:8081/cgi-bin/koha/catalogue/MARCdetail.pl?biblionumber=58
2. In the OPAC, go to the record
http://localhost:8080/cgi-bin/koha/opac-detail.pl?biblionumber=58
3. Click "Cite"
--> The subtitle is not in any of the citations
4. Apply the patch
5. Repeat step 2 and 3
--> The subtitle is now shown
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Test plan:
Read the patch.
Search for both parameters to verify that they are not used.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
UNIMARC instances include the following two authority types
by default (among others): Corporate Body Name (code: 'CO')
and Collective Title (code: 'CO_UNI_TI').
The problem is that when searching for Corporate Body Name
authorities (in the OPAC or in the Staff interface), the
results will include Collective Title authorities as well.
It has to do with Corporate Body Name authorities using
an authtypecode (CO) that happens to be the prefix of the
authtypecode (CO_UNI_TI) used by Collective Titles.
NOTE: Elasticsearch is not affected by this.
The fix is to additionally pass the completeness attribute
to the RPN query sent to Zebra so that it will try to match
on what is literally found in the entire field's phrase
index (i.e. the value of UNIMARC 152$b), instead of performing
a partial match.
More information can be found at the official documentation:
https://software.indexdata.com/zebra/doc/querymodel-rpn.html#querymodel-bib1-completeness
Test plan (tailored for KTD):
0) Launch a UNIMARC Koha instance using Zebra:
ktd --marcflavour unimarc --search-engine zebra up
1) Import the Sample MARC record attached to this Bug,
containing a single Collective Title authority
(adding one manually would take a long time due to
the many mandatory subfields).
2) Perform a generic authority search (without entering
anything in the search box) for all 'Corporate Body
Name' authorities. Note the number of results returned
(83), and also notice that in page 4 of the results you
get a stray 'Collective Title' authority ("Selected
works", the one you imported in Step 1).
3) Apply this patch (and `restart_all`).
4) Repeat the same search as in Step 2. This time, the
number of results returned should be one less than
before (82), and you should only see 'Corporate Body
Name' authorities in the results.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch updates the diff for ADD/CREATE and DELETE logactions to
exclude undefined fields from the diff.
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Update suggestion action logging to populate the diff column using the
correct before/after semantics:
- CREATE: empty before-state ({}), new suggestion data as after-state
- MODIFY: pre-change unblessed data as before, post-store object as after
- DELETE: log before the SQL DELETE so the object can still be fetched;
pass the suggestion as both $infos (info column) and $original (diff
before-state), logaction sets after-state to {}
Also add tests verifying the diff column is populated for all three actions.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch extends the SIP checkout handler to honour the new
three-valued AllowItemsOnHoldCheckoutSIP preference:
* 0 - Don't allow checkout of any held item
* 1 - Allow checkout when the hold is still pending (not attached)
* 2 - Also allow checkout when the hold is waiting, in transit or
being processed for another patron
Previously the preference was a simple YesNo toggle that, when
enabled, allowed the checkout regardless of whether the hold was
pending or already attached to an item on the hold shelf. Libraries
could not block the more disruptive case (checking out a waiting
hold destined for another patron) without also blocking pending
holds.
The existing post-confirmation filter is reworked into a single
branch per confirmation type that consults the preference value.
Test plan:
1. prove t/db_dependent/SIP/Transaction.t
2. With AllowItemsOnHoldCheckoutSIP=0, place a pending hold for
patron A on an item, attempt SIP checkout to patron B: refused.
3. Repeat with the hold marked waiting: refused.
4. Set AllowItemsOnHoldCheckoutSIP=1, pending hold: allowed;
waiting hold: refused.
5. Set AllowItemsOnHoldCheckoutSIP=2, waiting hold: allowed.
Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
With the ability to customise which columns are hidden by default as
part of "Table settings", this syspref is redundant. As part of the
atomicupdate, we incorporate the previous syspref value into
columns_settings.
As there are six possible states the migration could start from (the
syspref can be 0 or 1, and the table setting can be 0, 1, or NULL) and
two final states it must be idempotent from, the test plan is
unfortunately quite cumbersome.
Test plan:
1. Reset the database, to clear the table settings.
2. Apply patch and update the database.
3. Navigate to Administration > System preferences > Cataloging.
4. Verify that "SpineLabelShowPrintOnBibDetails" does not appear.
5. Navigate to a catalogue detail page.
6. Verify that the "Spine label" column is hidden.
7. Update the database again.
8. Navigate to a catalogue detail page.
9. Verify that the "Spine label" column is hidden.
10. Revert the patch.
11. Reset the database, to clear the table settings.
12. Navigate to Administration > System preferences > Cataloging.
13. Set "SpineLabelShowPrintOnBibDetails" to "Show" and save.
14. Apply patch and update the database.
15. Navigate to a catalogue detail page.
16. Verify that the "Spine label" column is visible.
17. Update the database again.
18. Navigate to a catalogue detail page.
19. Verify that the "Spine label" column is visible.
20. Revert the patch.
21. Reset the database, to clear the table settings.
22. Navigate to Administration > Table settings.
23. Under Catalog > holdings_table, check "Is hidden by default" for
"spinelabel" and save.
24. Apply patch and update the database.
25. Navigate to a catalogue detail page.
26. Verify that the "Spine label" column is hidden.
27. Revert the patch.
28. Reset the database, to clear the table settings.
29. Navigate to Administration > System preferences > Cataloging.
30. Set "SpineLabelShowPrintOnBibDetails" to "Show" and save.
31. Navigate to Administration > Table settings.
32. Under Catalog > holdings_table, check "Is hidden by default" for
"spinelabel" and save.
33. Apply patch and update the database.
34. Navigate to a catalogue detail page.
35. Verify that the "Spine label" column is hidden.
36. Revert the patch.
37. Reset the database, to clear the table settings.
38. Navigate to Administration > Table settings.
39. Under Catalog > holdings_table, uncheck "Is hidden by default" for
"spinelabel" and save.
40. Apply patch and update the database.
41. Navigate to a catalogue detail page.
42. Verify that the "Spine label" column is hidden.
43. Revert the patch.
44. Reset the database, to clear the table settings.
45. Navigate to Administration > System preferences > Cataloging.
46. Set "SpineLabelShowPrintOnBibDetails" to "Show" and save.
47. Navigate to Administration > Table settings.
48. Under Catalog > holdings_table, uncheck "Is hidden by default" for
"spinelabel" and save.
49. Apply patch and update the database.
50. Navigate to a catalogue detail page.
51. Verify that the "Spine label" column is visible.
Thanks-to: Katrin Fischer
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Adds minimal POD stubs for all private (underscore-prefixed) subroutines
in C4/Matcher.pm and C4/ImportBatch.pm to satisfy the QA script's
pod_coverage check.
Sponsored-by: Main Library Alliance <https://www.mainlib.org/>
Signed-off-by: Jeanne Mauriello <jeanne.mauriello@mainlib.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Extends C4::Matcher::get_matches() to track each matchpoint's individual
score contribution alongside the aggregate total. A new `composite_scores`
hashref (keyed by search_index) is included in every result entry.
Updates C4::ImportBatch::SetImportRecordMatches() to JSON-encode and
persist the composite scores in the new column, and
GetImportRecordMatches() to decode and return them.
Sponsored-by: Main Library Alliance <https://www.mainlib.org/>
Signed-off-by: Jeanne Mauriello <jeanne.mauriello@mainlib.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit improves upon the original fix by making the intent more
explicit when handling empty string date parameters in AddRenewal and
AddIssue functions.
WHY THIS IS BETTER:
The original fix (changing `defined $datedue` to just `$datedue`)
works correctly but relies on Perl's implicit truthy/falsy semantics:
if ( $datedue && ref $datedue ne 'DateTime' )
This is subtle because:
- Empty string "" is falsy, so parsing is skipped ✓
- But it's not immediately obvious that empty strings are being
handled specially
- The code doesn't explicitly show its intent
THIS APPROACH:
if ( defined $datedue && $datedue ne '' && ref $datedue ne 'DateTime' )
Benefits:
1. EXPLICIT: Clearly states "only parse if defined AND not empty
AND not already a DateTime"
2. SELF-DOCUMENTING: Intent is obvious to future developers
3. NO RELIANCE on truthy/falsy semantics
4. CONSISTENT: Same pattern applied across all date parameter handling
COMPREHENSIVE CHANGES:
1. AddRenewal (C4/Circulation.pm:3449)
- Made datedue empty string check explicit
2. AddIssue (C4/Circulation.pm:1611)
- Made datedue empty string check explicit
- Also updated issuedate handling (line 1616) to explicitly handle
empty strings
3. Test Coverage (t/db_dependent/Circulation/issue.t)
- Added test for AddIssue with empty string datedue
- Added test for AddIssue with empty string issuedate
- Ensures both functions properly calculate dates from circulation
rules instead of using "now"
The existing test for AddRenewal already covered empty string datedue
(added in the original Bug 37966 commit). These new tests ensure the
same correct behavior for AddIssue.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
When renewing a book on hold with the RenewalOnHoldOverride sys pref set
to "Allow", if the due date is not specified then it becomes "now". By
checking the input is set rather than just defined the due date is set as
expected based on circulation rules.
Also change the description of the RenewalOnHoldOverride sys pref as the
due date would no longer be required to be entered with this change.
Test plan:
1. make sure RenewalOnHoldOverride is set to 'Allow'
2. check out a book, then place a title-level hold on it
3. renew the book, see the hold override dialog
4. leave the due date empty and hit the override button
5. observe the new due date is "now"
6. apply patch and restart plack
7. renew the book again, overriding the hold, and see the expected due date
as per circulation rules.
Signed-off-by: Marion Durand <marion.durand@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This is not used!
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Thomas Klausner <domm@plix.at>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This looks trivial at first, but it's not, because of forking and the
comment in C4::Context
I don't think the comment makes sense, we need to retrieve a new dbh,
and so passing the 'new' parameters to C4::Context->dbh should do the
same.
Test plan:
Try rebuild_zebra.pl and confirm there is no error
% perl misc/migration_tools/rebuild_zebra.pl --processes 2 -b
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Thomas Klausner <domm@plix.at>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Some services ( such as CloudFlare ) use alternative heads to X-Forwarded-For. Koha should be able to use a custom header instead of X-Forwarded-For if needed.
This patch updates the feature to allow both the custom header and X-Forwarded-For to work harmoniously
For example, CloudFlare has it's own custom header.
This patch allows a site to be configured for Cloudflare's proxy before the proxy is enabled.
Test Plan:
1) prove t/Koha/Middleware/RealIP.t
Signed-off-by: Lin Wei Li <lin-wei.li@inLibro.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Marcel de Rooy
Olivia Reynolds
Martin Renvoize
Kyle M Hall
Baptiste Wojtkowski
Paul Derscheid
Lari Taskula
Pedro Amorim
Lisette Scheer
Hammat Wele
Andreas Roussos
Olivia Reynolds
Michael Hafen
Jonathan Druart
David Cook
Lucas Gass