Commit Graph

62585 Commits

Author SHA1 Message Date
Chloé Zermatten b38d6d5201
Bug 10190: (QA follow-up): fix: keyboard delay delete behaviour
If the 'Delay' input field value is deleted using backspace or other,
block form submission. The same behaviour apply if the delete button
is used, as both clear out the value.

As triggers with no delay are not processed and would also cause
higher triggers to then also not be processed, we do not allow the
creation of triggers without a set delay.

Triggers when edited individually do not check for impact on any overrides
they might have. It is therefore safest not to allow submission of a trigger
where the user explicitly unset the value.

An explicit 'reset to fallback' feature is possible and would constitute a follow up enhancement.

Test plan (based on comment 872):

A) 'Administration' > 'Circulation triggers' > 'Add new trigger' for any library/category. Set a Delay, a Notice, and a transport type.
B) Select the Delay field's value and clear it via the 'X' clear button.
C) Notice 'Submit' is disabled.
D) Select the Delay field's value and clear it via the keyboard (Backspace/Delete). Click 'Submit'.
E) Notice 'Submit' is disabled.

Assisted-by: Claude Opus 5 (Anthropic)
Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:27 +00:00
Chloé Zermatten e40291cb2c
Bug 10190: (QA follow-up): fix: missing rewrite rule in koha-httpd.conf
Looking at Bug 41167, this ought to fix the 404 error noted in comment 192 for package installs.

We do have a test asserting that RewriteRules must be carried over, so:

Test plan:
1. Apply patch
2. prove xt/check_apache_conf.t
3. Confirm it passes

That said, I don't think it is replicable in ktd - the copied file at /etc/koha/apache-shared-intranet.conf
will remain stale unless cp_debian_files.pl runs. The migration issue itself might be ktd-specific.

Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:26 +00:00
Chloé Zermatten ec22f77b76
Bug 10190: (QA follow-up): fix: permissions
Important note: the following assumes manage_circ_triggers is intended
as a sibling to manage_circ_rules, and that of the two, only
manage_circ_triggers ought to grant access to client-side circulation
trigger configuration actions.

When manage_circ_triggers was introduced, it was not applied client-side,
resulting in the router and menu permission checks incorrectly expecting
manage_circ_rules. Amend that.

Additionally, while the API endpoint blocked a user with only
manage_circ_triggers from editing non-triggers circulation rules,
it did not block a user with only manage_circ_rules from editing
trigger circulation rules. Add a guard clause.

Test plan (based on comment 872):
A) Go to a patron's 'More' > 'Set permissions' tab. Check 'catalogue' (needed for basic staff login), and under circulation-rules sub-permissions, check only 'Manage overdue notice triggers' - leave 'Manage circulation rules' unchecked.
B) Log in as that patron, go to 'Administration' - 'Circulation triggers' link present (admin-home.tt now correctly checks CAN_user_parameters_manage_circ_triggers), 'Circulation and fine rules' is not present.
C) Visit http://localhost:8081/cgi-bin/koha/admin/circulation_triggers directly - you are granted access.
D) Confirm that you can create and edit circulation triggers.
E) Go to a patron's 'More' > 'Set permissions' tab. Check 'catalogue' (needed for basic staff login), and under circulation-rules sub-permissions, check only 'Manage circulation rules' - leave 'Manage overdue notice triggers' unchecked.
F) Log in as that patron, go to 'Administration' - no 'Circulation triggers' link present (as we require manage_circ_triggers), however 'Circulation and fine rules' is present.
G) Visit http://localhost:8081/cgi-bin/koha/admin/circulation_triggers directly - you see "Error: You do not have permission to access this page" and are prompted to log in as a different user (as we require manage_circ_triggers).
H) Go to http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl, and confirm that you can edit circulation rules for your library (no regression).

Assisted-by: Claude Opus 5 (Anthropic)
Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:25 +00:00
Chloé Zermatten a4be8b914e
Bug 10190: (QA follow-up): fix: itiva script
The following issues were identified and resolved:

- items.itemtype does not exist -> swap to items.itype
- leftover WHERE clause still referred to overduerules -> removed
- with the join on overduerules removed there is no need to GROUP BY anymore
- Koha::CirculationRules->all -> use next() instead

Test plan (as per comment 872):
A) In KTD, set 'TalkingTechItivaPhoneNotification' to 'On'.
B) Run perl misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl --type=OVERDUE -o /tmp/out.txt -v
C) No errors / the script does not die mid run.

Quick disclaimer: I am not overly familiar with that script:
while I did remove the causes behind the script dying mid run,
I won't have spotted issues with expected output / outcome.

Assisted-by: Claude Opus 5 (Anthropic)
Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:24 +00:00
Nick Clemens 14e7814c15
Bug 10190: (QA follow-up) Fix spelling for QA tools
Other complaints are invalid:
ContextA spelling - this is a variable
Use of _("") - this is in regex for 'overdue_(' when checking rules
git manipulation - not ideal adding and removing in same set, but this is a long
   work, not asking for a squash here

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:23 +00:00
Nick Clemens a571b00a31
Bug 10190: (QA follow-up) Add new permission to tests
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:22 +00:00
Nick Clemens 8150c532fa
Bug 10190: (QA follow-up) Adjust error message to list both permissions
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:21 +00:00
Chloé Zermatten 2e3f9bd1ce
Bug 10190: (QA follow-up): fixes phase four
- Display new manage_circ_triggers permission (add to template)
- Ensure loading messages are informative to the user

Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:20 +00:00
Chloe Zermatten 763dc46e31
Bug 10190: (follow-up): test: cypress tests update
New tests:
- regression: mtt cells render as .fallback (bold italics)
- bg-success-subtle highlight on completed steps in the add flow
- disabled-Delete states:
  - patron category filter set
  - item type filter set
  - default library, if any library has a trigger higher that default's highest
- redirect to admin home if not permitted to access the page
- in the edit/add stepper form
  - check row actions are enabled at the 'selectOrAdd' step
  - check row actions are disabled once add mode is entered
- check the action column is always available on the main page

Clean up / general updates:
- replace the /api/v1/** catch-all intercept with per-endpoint intercepts so @get-rules waits resolve reliably
- remove now-redundant cy.wait("@get-rules") calls
- drop unused mount / dayjs / dates imports
- ensure expected strings match UI where strings have changed

Assisted-by: Claude Opus 4.7 (Anthropic)

AI used to draft enhancements tests. Changes were reviewed, tweaked, and commited manually and gradually (with a pre-submission commit squash). Tests were run multiple times and adjusted iteratively to match code changes, cover regressions as those were found, and handle vue page load.

Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:19 +00:00
Chloe Zermatten d6f596c514
Bug 10190: (QA follow-up) UI/UX improvements and fixes - phase three
Trigger deletion and rule set reset
- warn the user that resetting or deleting a given set will break sets using it as fallback.
- indicate which sets will be affected.
- display a preview of the post reset state of each affected rule set.
- preparation: allow TriggerTable to take a title parameter.
- refactor to remove 'has_rules' from the store's rule suffixes list so those match the DB.
- DRY: refactor to implement TriggerTable rather than use custom templates.
- select context stage handles narrower viewports
- prevent crash on missing rule set

TriggersTable: refactor: the <a> elements used as buttons are now button elements
- this is so they may be marked as disabled.

Main page - trigger actions (delete button display)
- disable the trigger deletion button.
  - if the context has been narrowed down ('delete' deletes all explicit rule set per trigger per library).
  - if the trigger belongs to default library AND one or more libraries have a trigger of a number bigger than the selected one.
  This is to protect trigger sequentiality from being broken for the relevant libraries.
  We also include an on hover message to explain the disabled button.

Add/ Edit trigger form
- clarify letters 'undo' button behaviour:
- swap the native vue select X for a fa-undo icon.
- display 'undo override and reset to fallback' on hover.
- only display the add/edit button at the selectOrAdd step.
- restore selected trigger yellow highlight (regression fix).
- display any temporarily saved data from prior step in a green 'success' container.
- only messages and data pulled from the DB for informative purposes display in blue containers.

Other
- redirect to correct URL (admin home) if store load fails
- mtt fallbacks display as such consistently (bold italics) - is a bugfix.
- catch errors on store initialisation
- remove whitespace duplicate
- ensure that mtt, if set, always display

Assisted-by: Claude Opus 4.7 (Anthropic)

AI used to help trace back potential bug sources, flag potential regressions, and draft enhancements. Changes were reviewed, tweaked, and commited manually and gradually (with a pre-submission commit squash).

Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:18 +00:00
Victor Grousset/tuxayo 622f0a4742
Bug 10190: (QA follow-up): fix pod of CirculationRules.pm
We have nothing to do with it but since the patches change the file, it
trips the QA script.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:16 +00:00
Chloe Zermatten 37a9684669
Bug 10190: (QA follow-up): UI/UX bug fixes - phase two
Fix: handle exiting modal
 - prevent data reload when the user exits the modal
 - as preparation for the above: refactor to use one handler only
Fix: new trigger saves with delay defined
 - Identified an old refactor omission - we were still passing 'i' despite the from now relying on this.triggerNumber
Fix: return to correct trigger tab after save or reset
Fix: restore delete button and bold italic info
 - restore display, fixes a regression introduced by the previous UI/UX patch set
Fix: order dropdown options by description or name (not code)
Fix: circ rule resets
 - restore reset funtionality
  (mtt was still occasionally passed as a string, and would then get compared to an array. Ensure this is not the case.)
 - do not trigger the data refresh if the current ruleset did not get deleted
 - make sure triggerNumber is defined when navigation is caused by reset
 - add the notice code in brackets to letter names for display (eg. Overdue notice (ODUE))

Assisted-by: Claude Opus 4.6 (Anthropic)

AI used to help trace back potential bug sources and draft fixes. Changes were reviewed, tweaked, and commited manually and gradually (with a pre-submission commit squash)

Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:15 +00:00
Chloe Zermatten 0fa640b387
Bug 10190: (QA follow-up) UI/UX improvements and fixes
- Refactor (prepation): Extract form sub-components for better maintainability
  - Template stucture improvements (eg. consistent fieldset use)
  - Ensure triggerNumber is always cast to integer from query params
  - Add/Edit trigger form UI/UX improvements/fixes:
      - Restore add/edit button functionality on trigger tables in the form
      - Update Context section title to dynamically change depending on whether editing or displaying
      - Display trigger context as informational when not in confirmContext mode
      - Hide required labels and validation on view-only context fields
      - Always display trigger context at the top of the form
      - Add renewal/autorenewal summary (renewals allowed, automatic renewal) to circulation rules summary panel
      - Sort notice letter dropdowns by name rather than code
      - Update Circulation rules summary title to be more descriptive
  - Scroll behaviour improvements:
      - Extract scrollToElementById to store with polling and null guard
      - Block page scroll when modal is open
      - Disambiguate duplicate trigger-table element ids (trigger-table-main
        vs trigger-table-form) to fix unpredictable scroll-to behaviour

Assisted-by: Claude Sonnet 4.6 (Anthropic)

Experimenting a little here - not quite certain where we draw the line for substantial use so I thought I would add a description in case it's useful, especially since the AI code guidelines are quite fresh:

AI was used primarily to help parse longer Vue templates (CirculationTriggersFormAdd.vue) and help isolate elements for extractions as well as highlight template formatting inconsistencies and propose improvements. The refactoring itself was achieved starting from AI-proposed changes which were then thoroughly re-worked manually towards the code changes submitted.
I also trialled using it (currently experimenting with this) for minor coding tasks where there is a clear precedent (for example, to add renewal/autorenewal information and do so in a way that follows exactly what we had written).
In those cases, each code change made by AI was the outcome of multiple iterations with direct feedback (which it did require a fair amount of) from me to ensure the output matched what I would have written myself.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:14 +00:00
Chloe Zermatten 8f6ff0a73d
Bug 10190: (follow-up): feat: indicate mtt is required
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:13 +00:00
Chloe Zermatten 6dcfe9f878
Bug 10190: (follow-up): fix: handle fallbacks in edit/add
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:12 +00:00
Chloe Zermatten b12d701494
Bug 10190: (follow-up): fix: reset rule info message
The information message showing in the reset modal
still referred to an earlier version of the UI.
Update it to correctly name the relevant filter.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:11 +00:00
Chloe Zermatten aa905b0801
Bug 10190: (follow-up): fix: mtt formating: handle unset
This is needed to handle creating new triggers
for libraries for which all earlier triggers are
defaults (do not exist in the DB , are not explicitly set)

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:10 +00:00
Martin Renvoize e4aecaa519
Bug 10190: Add manage_circ_triggers permission for notice trigger access
Replaces the broad manage_circ_rules permission requirement on the
circulation triggers page with a new dedicated manage_circ_triggers
sub-permission (under parameters, flag 3).

- Add manage_circ_triggers to userpermissions.sql
- Update atomicupdate to add the permission, grant it to existing
  edit_notice_status_triggers users, and remove the old permission
- Update admin/circulation_triggers.pl to require manage_circ_triggers
- Add manage_circ_triggers as an OR option on all circulation_rules
  API endpoints (GET/PUT /circulation_rules, /kinds, /config)
- Restrict manage_circ_triggers users in the API controller to
  overdue_* rule names only on write

Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:09 +00:00
Martin Renvoize fabe480e52
Bug 10190: (follow-up) Add missing CSP nonce to inline script tag
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:08 +00:00
Chloe Zermatten f2951ab377
Bug 10190: (follow-up) mtt formatting
The Transport Type options when displaying in 'edit' mode were showing as
a comma separated string rather than as separate options (as they do when being set).
Ammends this so that they display as separate options consistently.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:07 +00:00
Martin Renvoize 4fdc9c76e1
Bug 10190: Fix false conflict error when editing specific trigger rules
When editing a specific rule (e.g. */Kids/Books) alongside an all/all/all
rule, the conflict check incorrectly reported a conflict. The effective=true
API response always returns context as */*/* (fallback), so ruleSetInDb.context
differed from currentRuleSet.context (which was overridden to the real context).

Fix by overriding ruleSetInDb.context before the hasConflict comparison,
consistent with the existing pattern in setCurrentRuleSet.

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:06 +00:00
Martin Renvoize b760367e25
Bug 10190: Fix patron_categories permission format
The x-koha-authorization was using 'parameters: catalogue' which the
Koha auth system interprets as a 'catalogue' subpermission within the
'parameters' module (which doesn't exist), causing a 403 for any user.

The correct format for the top-level 'catalogue' permission is
'catalogue: "1"', consistent with all other read-only API endpoints.

Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:05 +00:00
Martin Renvoize bc461cdde1
Bug 10190: Center tables in add modal
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:03 +00:00
Martin Renvoize 88b16e6445
Bug 10190: Enforce manage_circ_rules_from_any_libraries permission
- Add OR-type x-koha-authorization to /circulation_rules, /circulation_rules/kinds,
  and /circulation_rules/config endpoints so users with either
  circulate:circulate_remaining_permissions OR parameters:manage_circ_rules
  can access them
- Enforce manage_circ_rules_from_any_libraries in the set_rules controller:
  users without this permission may only set rules for their own library and
  cannot set default (*) rules
- Add explicit use C4::Auth qw(haspermission) import to CirculationRules.pm
- Lock the library selector in the Vue UI for users who lack
  manage_circ_rules_from_any_libraries, defaulting them to their own library
- Update set_rules() tests to cover the new permission restrictions

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:02 +00:00
Martin Renvoize 14fe8cb2f5
Bug 10190: (QA follow-up) Fix six issues reported during QA
1. Letter context: setFilteredLetters() was reading library_id from
   currentRuleSet (which has no top-level library_id property) instead
   of this.context.library_id, causing library-specific letter templates
   to never match.

2. False conflict on edit: conflict check fetched rule set from DB with
   effective=false (raw), but the in-memory snapshot was fetched with
   effective=true (with fallbacks). Comparing raw vs effective always
   triggered a conflict when a context inherited any value from a more
   general rule. Fixed by using effective=true for the DB fetch too.

3. Incomplete deletion: updateCircRuleSets() inside deleteRuleSet() was
   called without await, creating a race condition where the page refresh
   could complete before the deletion API call, leaving stale data visible.
   Also added a guard to skip rule sets with no explicit rules for the
   trigger being deleted, and simplified rulesForDeletion construction.

4. No rules displayed on initial load: the route watcher in
   CirculationTriggersList fired immediately before init() had loaded
   patron categories and item types, causing setAllExhaustiveEffectiveRuleSets()
   to produce an empty result. Fixed with a metaInitialized flag that the
   watcher waits for before proceeding.

5. DefaultToLoggedInLibraryOverdueTriggers syspref not respected: the
   default_view template variable was computed but never applied to the
   store's currentLibraryId. Fixed by passing it to init().

6. UI colors: info box background changed from #84c1f3 to #DAECFB (lighter,
   matching the circulation rules page). Error alert messages changed from
   alert-info (blue) to alert-warning (yellow) to match Koha conventions.

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:01 +00:00
Martin Renvoize 8f6cee51ea
Bug 10190: Fix permission mismatch on /circulation_rules/config endpoint
The config endpoint was gated on parameters: manage_circ_rules, while the
actual GET/PUT /circulation_rules endpoints require circulate:
circulate_remaining_permissions. Any user who can read/write circulation
rules should also be able to load the config that drives the UI.

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:25:00 +00:00
Martin Renvoize e41d6b54ca
Bug 10190: Patron categories fetch shouldn't require full manage permission
Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:59 +00:00
Martin Renvoize c53a2ead10
Bug 10190: fix failing Permissions.t and overhaul CirculationTriggers Cypress spec
Permissions.t:
- Remove edit_notice_status_triggers from userpermissions.sql so fresh installs
  no longer seed this deprecated permission (existing installs are handled by
  the atomic update)
- Add a defensive DELETE in the test to cover DBs that pre-date the atomic update

CirculationTriggers_spec.ts:
- Fix getMockCirculationRule: the helper was returning the old {rule_name,
  rule_value, branchcode, ...} format; update to the current API format
  {context: {...}, overdue_N_delay: ..., ...}
- Remove assertions for UI elements that do not exist (success/error flash
  messages, "Existing triggers" heading, "rule set will be removed" text)
- Fix reset confirmation text to match actual component output
- Fix duplicate patron_category_id "ST" in mock patron categories
- Remove aspirational tests that checked non-implemented behaviour
- Add describe block for the no-default-triggers warning (no rules anywhere,
  rules only at specific libraries, library link switches the filter)
- Add regression test for NaN trigger number when adding first rule for a library

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:58 +00:00
Martin Renvoize f631df5afd
Bug 10190: Fix test count in overdues.t
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:57 +00:00
Martin Renvoize b80df78435
Bug 10190: warn when no default triggers exist and show libraries with rules
When viewing the default library context with no triggers defined, show
a warning message explaining defaults apply to all libraries. If any
libraries have specific triggers, list them as clickable links that
switch the filter to that library. If no rules exist anywhere, show a
prompt to add the first trigger.

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:56 +00:00
Martin Renvoize 17c3aab6c4
Bug 10190: fix NaN trigger number when adding first rule for a library
When no circulation rules exist for a library, triggerCounts[library_id]
is undefined, causing undefined + 1 = NaN. Use nullish coalescing to
default to 0 so the first trigger correctly gets number 1.

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:55 +00:00
Martin Renvoize 7413d2724e
Bug 10190: remove has_rules sentinel from circulation_rules table
has_rules was a purely frontend marker stored as a circulation rule to
signal that a context had explicitly-saved overdue trigger rules. The
backend never reads it; the information is fully derivable from whether
any of delay/notice/mtt/restrict are non-null for a given context.

Replace all has_rules DB reads/writes with a hasExplicitRulesForTrigger()
helper that checks the four real rule suffixes directly. has_rules remains
as a computed in-memory concept in the Vue store (derived in findEffectiveRule)
so templates require no changes.

Also adds an atomicupdate to delete existing has_rules rows from the DB,
and removes the has_rules insert from the bug_10190 migration script.

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:54 +00:00
Martin Renvoize 591263d5ef
Bug 10190: three minor issues
- overdue_notices.pl: fix verbose sprintf - remove spurious \$triggered
  argument (CLI 0/1 flag had replaced delay value in output) and use
  the \$borr string that was computed but never included in the message
- overdue_notices.pl: remove %already_queued hash which was populated
  in _enact_trigger but never read; dead code since the refactor
- C4/Overdues.pm GetBranchcodesWithOverdueRules: guard the global-rule
  expansion with @branchcodes check so an empty result (no rules at all)
  correctly returns an empty list instead of all branches

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:53 +00:00
Martin Renvoize 8659f4e016
Bug 10190: correction to x-koha-authorization on /circulation_rules/config
The config endpoint is used exclusively by the Circulation Triggers
Vue app, which is gated on parameters/manage_circ_rules. Using
acquisition/'*' and erm/1 was inconsistent copy-paste from an
unrelated endpoint and would 403 circ-rules admins while granting
access to unrelated module users.

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:52 +00:00
Martin Renvoize 228bf75845
Bug 10190: fix four more issues spotted in the overdues script
- Patron.pm has_restricting_overdues: sort ASC not DESC so the oldest
  (most overdue) item is checked per branch/itemtype; DESC caused the
  deduplication skip to discard the very items most likely to trigger
  a restriction
- overdue_notices.pl: fix --itemtypesout option name in GetOptions
  (was 'itemtypeouts', never matched the documented --itemtypesout flag)
- overdue_notices.pl: fix typo 'catagorycode' -> 'categorycode' in
  --borcatout search, which silently broke category exclusion
- circulation_triggers.pl: add flagsrequired => parameters/manage_circ_rules
  so the admin page enforces the correct permission; also add missing
  use Koha::Notice::Templates

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:51 +00:00
Martin Renvoize 6640e3e58a
Bug 10190: fix three regressions in overdue_notices.pl
- Store patron name/address fields in \$borrower_overdues so
  prepare_letter_for_printing has data for printed/CSV/text notices
- Fix email->print fallback: look up items under original MTT key,
  not the effective (fallback) key
- Set \$admin_email_address from \$library unconditionally in
  _enact_trigger; previously undef in non-patron_homelibrary mode,
  stripping the from_address on all queued notices

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:50 +00:00
Martin Renvoize d539637c5e
Bug 10190: rewrite atomicupdate with correct 1:1 rule migration
The previous migration had two fundamental errors:

1. It manufactured phantom empty rows for all patron categories that
   had no rule, which corrupted the "most frequent" calculation.

2. It created categorycode=NULL (all-categories) default rules for
   every branchcode seen, silently extending overdue notice coverage
   to patron categories that had no configured rule in the old system.
   The backend (overdue_notices.pl) does not consult has_rules, so
   these invented defaults would cause notices to be sent to patrons
   that were deliberately uncovered.

The rewrite migrates only explicit (branchcode, categorycode) pairs
that had a positive delay configured. No default rules are created.
branchcode='' maps to NULL as before. exit is replaced with return
to avoid killing subsequent atomicupdate scripts.

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:49 +00:00
Chloe Zermatten 8c8160761d
Bug 10190: Deprecate permission
user_permissions rows do not need manual cleanup because the FK on
user_permissions(module_bit, code) references permissions(module_bit, code)
ON DELETE CASCADE, so deleting from permissions automatically removes
per-patron rows.

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:48 +00:00
Chloe Zermatten 564f15ca59
Bug 10190: prevent duplicate ruleset load
- prevent the initial API call and subsequent exhaustive applied ruleset list generation logic from running twice

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:46 +00:00
Chloe Zermatten 3bb6f2e69c
Bug 10190: (QA follow-up) swap undef for all in update db output
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:45 +00:00
Chloe Zermatten a45a7aa8e1
Bug 10190: trigger add/edit form UI/UX enhancements
- scroll to the add/edit form on button click
    - prevent redundant full page reload upon the user swapping between adding and editing while the modal is open
    - ensure robust scroll behaviour, handling:
          - the URL being accessed directly
          - the edit button in the table in the main page being used to access the form directly
          - the stepper workflow being used
          - the user swapping between add and edit once already looking at the form
- dynamic display of 'Transport Type' consistency: only show if a 'Notice' has been selected
- if a notice has been selected, prevent form submission if transport type isn't set and display an alert on submission attempt

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:44 +00:00
Chloe Zermatten d3022cb950
Bug 10190: (QA follow-up) add missing filter
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:43 +00:00
Chloe Zermatten d508b600dd
Bug 10190: enhance trigger context display in modal
- improve modal scroll behaviour
- improve modal dimensions
- display contexts as a horizontal list to make better use of the available space
- fully contain dropdowns within the modal body

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:42 +00:00
Chloe Zermatten b8f8175539
Bug 10190: (QA follow-up) typo
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:41 +00:00
Chloe Zermatten 607ddb5a3f
Bug 10190: (QA follow-up) amend copyright statement
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:40 +00:00
Chloe Zermatten 53a2b75851
Bug 10190: (QA follow-up) add new line at end of MessageTransportType.pm
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:39 +00:00
Chloe Zermatten 9c6781302c
Bug 10190: (QA follow-up) fix unix permissions on circulation_triggers.pl
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:38 +00:00
Chloe Zermatten d176ba4d59
Bug 10190: (QA follow-up) fix unix permissions on bug_10190.pl
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:37 +00:00
Chloe Zermatten 298b4b6efc
Bug 10190: refactor to remove redundant try..catch
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:36 +00:00
Chloe Zermatten 7e70013450
Bug 10190: remove circ triggers link from tools menu
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
2026-09-10 14:24:35 +00:00