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>
This is a patch to bring back a small UI component that was dropped on the previously QA’ed and merged work in bug 16631.
Before applying patch, confirm the filter option for Show reports is not available on the Saved Reports page
1. Pull the latest from Main
2. Enable LimitReportsByLibrary under Reports in System Prefernces
3. Visit the Save reports page
4. There should be NO Show reports with selectbox in the Filter pane on the left side
5. Implement the patch
6. Reload Save reports page
7. In the Filter pane on the left side, there should be a Show reports select box with options for Show my library reports and Show all reports options
Confirm only permissioned users see this option and reports are filtered
8. Create a staff user with the all reports permissions including Manage Report Limits (manage_report_limits). Home library should be Centerville.
9. Create a staff user with all reports permission but NOT Manage Report Limits (manage_report_limits). Home library should be Centerville.
10. Create a report, “No limit” with no limit.
11. Create a report, “Centerville limit” limited to Centerville.
12. Create a report “Fairfield limit” limited to Fairfield.
13. Log in as the permissioned user.
14. Confirm the filter is available and that it filters reports so that only the No limit and Centerville limit reports show when My library reports only is selected.
15.Select All reports and all 3 reports should be in the list along with the Fairfield report .
16. Log in as the unpermissioned user.
17. The filter option for my / all reports should not be in the leftside pane.
18. Only the Centerville limit and No limit reports should be visible
Signed-off-by: Anna Stalter <staltera@hamiltoneastpl.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch adds a check for the ArticleRequests system preference in
opac-request-article.pl so that the page will return a 404 error if the
preference is disabled.
To test, apply the patch and restart services.
- If necessary, enable the ArticleRequests system preference and
configure a circulation rule to allow article requests.
- Locate a bibliographic record in the OPAC which will accept an article
request. Note the biblionumber of the record you select.
- Click "Request article" and complete the process. It should finish
successfully.
- Disable the ArticleRequests system preference.
- Return to the OPAC and navigate directly to the page for requesting an
article:
http://localhost:8080/cgi-bin/koha/opac-request-article.pl?biblionumber=X
...where X is the biblionumber you noted before.
- This should trigger a 404 error.
Sponsored-by: Athens County Public Libraries
Signed-off-by: Bastien CHAUVET <bastien.chauvet@inlibro.com>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch adds some checks to cookieconsent.js to avoid errors in the
console: If we try to access 'document.getElementById("cookieConsentModal")'
when the element doesn't exist there will be an error:
'Uncaught TypeError: can't access property "backdrop", this._config is
undefined'
The error can be seen in the browser console on the Circulation home
page and the Tools home page when the CookieConsent preference is
enabled.
To test apply the patch and clear your browser cache if necessary.
- Enable the CoookieConsent system preference.
- Go to the Circulation home page. There should be no error in the
console.
- Clear your browser's local storage to forget your consent.
- Go to the staff client home page. The cookie consent message should
appear.
Sponsored-by: Athens County Public Libraries
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
- The confirmDelete dialog for removing all reserves from a course
was bound as a click handler on the #rm_items form element, which
is never directly clicked by the user
- The #rm_items_button click handler called .submit() on the form
without checking confirmDelete first, bypassing the confirmation
- Move the confirmDelete check into the #rm_items_button click
handler and remove the dead #rm_items click handler
Test plan:
- Enable UseCourseReserves system preference (should be enabled in ktd)
- Create a course (or use the existing one) and add at least one reserve
to it
- On the course details page, click "Remove all reserves"
- Observe the form submits immediately without confirmation
- Apply patch
- Repeat the steps above
- Verify a confirmation dialog now appears
- Click cancel and verify the reserves are not removed
- Click "Remove all reserves" again, confirm, and verify
the reserves are removed
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
To test:
1. Do a catalog search that brings up multiple results
2. Check the checkbox of in the left column for the first two results
3. Click on the title of the 3rd result to take you to the details page for that item
4. Use the borwser's back button to return to the search results
5. Notice the first 2 titles are still checked.
6. Now check the 3rd result and click 'Add to list'
7. Only the 3rd title is added, even though all 3 are checked.
8. Try steps 1 - 7 again but with 'Add to cart', same behavior.
9. Try setps 1 - 7 again but with 'Place hold', all 3 items are selected.
10. APPLY PATCH, clear browser cache and run through all the steps again. This time your selction should be rememebered.
Bonus points for trying in as many browsers as possible.
Signed-off-by: Bastien CHAUVET <bastien.chauvet@inlibro.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Bug 42310 landed on main after this patch's tests were written. It
renamed Koha::Calendar to Koha::Library::Calendar and normalized the
legacy repeatable_holidays/special_holidays tables into
library_weekly_closures/library_single_closures (plus repeating
closures and exceptions), managed via Koha::Library::Calendar::*
DBIC-backed classes.
The new subtests added for this bug still referenced the old class
name and the dropped DBIC sources (RepeatableHoliday, SpecialHoliday),
so t/db_dependent/Calendar.t and t/db_dependent/Hold.t failed after
rebasing onto current main. Update both to build closures via
Koha::Library::Calendar::WeeklyClosures/SingleClosures and to
instantiate Koha::Library::Calendar, matching the convention already
used elsewhere in these files since the Bug 42310 follow-up.
No production code changes; test-only fix.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
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>
This patch updates several templates in the OPAC so that when a
responsive DataTable is partially collapsed, the column with the "+"
buttons has "Expand" in the column header.
To test, apply the patch and test tables on the affected pages,
narrowing the page enough to confirm that when the table collapses, the
last column has "Expand" in the header.:
- Authorities search results
- The details of a course reserve (with titles on reserve)
- The holdings table on the bibliographic detail page
- Search history
- Most popular
As a logged-in user:
- User summary page (all tabs)
- Curbside pickups (with at least one curbside pickup scheduled)
- Checkout history
- Suggestions
- Tags
In self-checkout:
- The checkouts, holds, and charges tabs.
Sponsored-by: Athens County Public Libraries
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>
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>
Wait before DT has finished the init to display the number of visible
records, otherwise there is a flash (display the count of the full set,
then the number of records displayed (20));
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
So we don't send an empty set to the tool
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
When opening report results in the new DataTables view, batch
operations should only send visible records. Same as the normal view.
Test plan:
1. Create and run a report that will display more resulsts than fit on
one page
2. Click "Open in DataTables".
3. Note "Batch operations with X visible records" shows the default
page size (=20), and open the "Batch operations" dropdown to
send results to e.g. batch item modification or batch record
modification.
4. Confirm only the values on the currently visible page are sent, not
the entire set.
5. Change the DataTable's page, or filter/search within it, and repeat
=> Confirm the count and the sent values update to the number of visible records
6. Confirm the standard (non-DataTables) report view is unaffected.
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
1. add missing <tr> wrapper element
2. amend casing in cypress test
3. change method name so it is codespell-friendly
4. remove deprecated test from TestBuilder.t
(used Overduerules which do not exist anymore.
rely on 'Tests with composite FK in userpermission'
for FK test coverage instead -> no replacement
introduced.)
5. 'Bug 10190: Fix patron_categories permission format'
changed the permission for GET requests to patron_categories
to catalogue = 1, matching the Koha precedent for such
endpoints. However, patron_categories still tested the
endpoint with the old permission assigned. Amend this to
complete the work started in that commit.
6. remove redundant delete statement in permission test:
if the test fails, this means the db update has not
run, and should. It would be incorrect for the db
to still include the permission at the point where the
test runs.
Test plan:
1. run `npm run js:build` (or watch the js:watch output)
2. run t/cypress/integration/Admin/CirculationTriggers_spec.ts
3. run prove xt/author/codespell.t
4. run prove t/db_dependent/TestBuilder.t
5. run prove t/db_dependent/api/v1/patron_categories.t
6. run prove t/Koha/Auth/Permissions.t
All should run with no warnings or errors.
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>
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>
The mutual-exclusivity check ran before the empty-element cleanup added
for bug 37075, so a bare -c (no value) left an empty string in
@letter_code, which the check saw as truthy and rejected even though
-c had effectively not been used. Move the cleanup above the check.
Also add the same mutual-exclusivity guard directly in
C4::Letters::SendQueuedMessages, since letter_code and
exclude_letter_code build the same search key and passing both would
otherwise let one silently clobber the other in the hashref passed to
->search - only the CLI script enforced this until now, so any other
caller could hit the silent-clobber bug.
Adds regression coverage: unit tests for the new exception in
SendQueuedMessages, and a script-level test reproducing the RM's
reported false positive.
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
We already delete the queue at the start of all tests and the tests
here are checking using ID, there is no reason to delete the queue again
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Both options were being accepted but only the last one in the command line
was being applied. Now they are mutually exclusive like --category and
--skip-category in longoverdue.pl.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Some libraries want to send digest messages once per day (e.g., at 20:05)
but send all other messages every hour. Currently, this requires listing
almost all letter codes (~85) in the hourly cron job, which is error-prone
and makes it easy to forget notifications at upgrades.
This patch adds a new --exclude-code (-x) option to process_message_queue.pl
that allows excluding specific letter codes from processing, making it much
easier to configure digest scheduling.
Changes:
- Added --exclude-code option to process_message_queue.pl (repeatable)
- Modified C4::Letters::SendQueuedMessages to handle exclude_letter_code parameter
- Supports both scalar and array reference formats
Example usage:
# Send all messages except digests every hour
00 */1 * * * process_message_queue.pl --exclude-code DUEDGST \
--exclude-code PREDUEDGST --exclude-code HOLDDGST \
--exclude-code AUTO_RENEWALS_DGST
# Send all messages including digests once per day
05 20 * * * process_message_queue.pl
Test plan:
1. Apply both patches
2. Run: ktd --shell --run 'prove t/db_dependent/Letters.t'
3. Verify all tests pass (should show 105 tests passing)
4. Create test messages in message_queue with different letter codes:
- Insert messages with codes: DUEDGST, PREDUEDGST, ACQ_NOTIF, HOLD
5. Test single exclusion:
misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST
- Verify DUEDGST messages remain pending
- Verify other messages are processed
6. Test multiple exclusions:
misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST \
--exclude-code PREDUEDGST
- Verify both digest messages remain pending
- Verify non-digest messages are processed
7. Test combined with --code option (should work together):
misc/cronjobs/process_message_queue.pl --code ACQ_NOTIF \
--exclude-code DUEDGST
- Verify only ACQ_NOTIF messages processed (DUEDGST excluded anyway)
8. Test help output:
misc/cronjobs/process_message_queue.pl --help
- Verify --exclude-code option is documented
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: Marion Durand <marion.durand@biblibre.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch adds unit tests for the new exclude_letter_code functionality
in SendQueuedMessages, which will allow excluding specific letter codes
when processing the message queue.
Test coverage includes:
- Excluding a single letter code (array format)
- Excluding multiple letter codes
- Excluding with scalar parameter format
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: Marion Durand <marion.durand@biblibre.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This follow-up fixes the formatting issue reported by xt/perltidy.t.
Test plan:
1. Run prove xt/perltidy.t
2. Confirm it passes.
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Add two test cases covering reports with no borrowernumber (NULL owner):
a patron without edit_all_reports should be denied, one with the
permission should succeed. The POD documents this case but it was
not previously exercised.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.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>
Test plan:
1) Apply this patch set
2) Run updatedatabase.pl
3) Restart all the things!
4) Create 2 users with create_reports and not edit_all_reports
5) Create reports with each
6) Note that each cannot edit the others reports
7) Create a user with create_reports and edit_all_reports
8) Note the this user can edit reports from the previous two users
Edit button placements to look for:
* Reports table row
* Actions menu
* Toolbar
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>
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>
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>
Something very messy happened during the last rebase before push.
Sorry!
I've restored the original version:
git rebase -i HEAD~258
Edit commit b02ca1270d and remove the next one (Tidy)
perl misc/devel/tidy.pl $(git diff --name-only HEAD~1)
git commit -a
git rebase --continue
Luckily there was no conflict!
I then picked the version of those files and here is the commit.
Use this to confirm:
`git show -w ad694a3411e677fcea96f315758793fd1fe0f203`
You should see changes that are not related to tidiness
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The original _GetCircControlBranch tests covered the case where CircControl
is PickupLibrary but no userenv branch is available, in which case the
resolution falls through to the ItemHomeLibrary behaviour. That coverage was
lost when the tests moved to t/db_dependent/Koha/Policy/Circulation.t, since
the PickupLibrary subtest always mocks a userenv.
This restores coverage for the fall-through path: with CircControl set to
PickupLibrary, no userenv, and no pickup_library_id, the controlling library
is resolved from the item's home library.
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: Martin Renvoize <martin.renvoize@openfifth.co.uk>
(cherry picked from commit 6a13c6e5066384e59e52f0d48f5a430958671ef2)
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Removes the deprecated _GetCircControlBranch function and its export
from C4::Circulation. Removes the corresponding tests from
Circulation.t (now covered by t/db_dependent/Koha/Policy/Circulation.t).
Test plan:
1. Apply patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Circulation.t \
t/db_dependent/Koha/Policy/Circulation.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: Alex Carver [Acerock7] <alex@rcls.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
(cherry picked from commit 8e784622815a2db2ce6f558987d7c0b0951ceec0)
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
# Conflicts:
# C4/Circulation.pm
# t/db_dependent/Circulation.t
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>
_get_data_and_patron() falls back to Mojo::Parameters form-data
parsing whenever $tx->res->json fails to decode the userinfo
response body. That branch was untested: all existing userinfo_url
subtests mock json() to return a hashref, so the fallback path was
never actually exercised.
This adds a subtest that mocks a genuine url-encoded response body
(json() returning undef) and confirms the fallback still maps data
and finds the matching patron correctly.
Test plan:
1. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/Auth/Client/OAuth.t
=> SUCCESS: All 6 subtests pass, including the new fallback-path test
2. Sign off
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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>
This patch adds a regression test demonstrating that the OAuth
userinfo_url response parsing fails when the Content-Type header
contains extra parameters beyond charset (e.g. Microsoft Graph API
returns application/json;odata.metadata=minimal;odata.streaming=true).
The strict regex in _get_data_and_patron only matches Content-Type
values with an optional ;charset=... suffix, causing valid JSON
responses to be silently misinterpreted as URL-encoded form data.
Test plan:
1. Apply this patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/Auth/Client/OAuth.t
=> FAIL: New subtest fails - email is undef, patron not found
3. Sign off :-D
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Martin Renvoize
Tomás Cohen Arazi
Jonathan Druart
Lawrence O'Regan-Lloyd
Nick Clemens
Casey Conlin
Owen Leonard
Paul Derscheid
Lucas Gass
Pedro Amorim
Magnus Enger
Chloé Zermatten
Slava Shishkin
Kyle M Hall