In Bug 36466 db_revs/251200019.pl was introduced to update wrong date values (0000-00-00) with NULL.
This can fail on MySQL as it is running with sql_mode NO_ZERO_DATE activated which does not allow to select or update date fields with values='0000-00-00'.
This patch changes the update queries in db_revs/251200019.pl to use Installer::sanitize_zero_date on both columns.
Test plan:
As the test shall be done using MysQL and Mariadb start 2 ktds:
MySQL) DB_IMAGE=mysql:8.0 ktd --proxy --name mysql8 up -d
MariaDB) ktd --proxy --name maria up -d
a) checkout a version prior 36466: git checkout v24.11.14-1
b) reset_all
c) Connect to staff interface (mysql8-intra.localhost and maria-intra.localhost)
d) Go to Serials and create a new subscription
e) run the following to enter database client in each ktd shell: koha-mysql kohadev
f-mysql) run the following commands to be allowed to change planneddate and publisheddate to '0000-00-00':
SET SESSION sql_mode = 'ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION';
update serial set planneddate = '0000-00-00', publisheddate = '0000-00-00';
f-maria)
update serial set planneddate = '0000-00-00', publisheddate = '0000-00-00';
g) verify via: select * from serial;
h) checkout main via: git checkout main
i-mysql) run updatedatabase
Upgrade to 25.12.00.019 [09:35:55]: Bug 36466 - Fix the incorrect 0000-00-00 date in planneddate and publisheddate field
ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do Incorrect date error in db-revs when using MySQL
In 36466 db_revs/251200019.pl
for column 'planneddate' at row 1 at /kohadevbox/koha/C4/Installer.pm line 818
i-maria) do not run updatedatabase yet
j) apply patch
k-mysql) run updatedatabase
Upgrade to 25.12.00.019 [09:37:02]: Bug 36466 - Fix the incorrect 0000-00-00 date in planneddate and publisheddate field
Incorrect date planneddate and publisheddate fixed
...
-> is successful now
k-maria) run updatedatabase
l) verify via: select * from serial;
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Miranda Nero <mnero@oslri.net>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Miranda Nero <mnero@oslri.net>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Signed-off-by: Andrew Fuerste Henry <andrew@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>
MySQL 8.4 requires foreign keys to reference PRIMARY KEY or UNIQUE keys.
Since authorised_values.authorised_value is only unique in combination
with category (composite UNIQUE key av_uniq), the FK constraint is not
compatible with MySQL 8.4+.
Data integrity for illrequests.status_alias is maintained at the application layer:
- Koha::ILL::Request->status_alias() validates values against
authorised_values (category='ILL_STATUS_ALIAS') before storing
- All ILL request operations in Koha use this object layer
- CASCADE behaviors (ON DELETE SET NULL, ON UPDATE CASCADE) are already
handled by the application layer when modifying/deleting authorised values.
Test Plan, using KTD:
1. Prepare KTD for MySQL 8.4
- In koha-testing-docker, edit 'compose/db-mysql.yml'
- Comment out the line 5:
`# command: --default-authentication-plugin=mysql_native_password`
Note: This is required because MySQL 8.4 doesn't support the native password plugin.
2. Start a new instance with MySQL 8.4
- `DB_IMAGE=mysql:8.4 ktd up`
- Observe that it fails to start with the errors such as 'DBD::mysql::st execute failed'
3. Apply the patch
- `git bz apply 39107`
4. Start the instance again
- `DB_IMAGE=mysql:8.4 ktd up`
- Observe that it starts successfully
5. Update the database (Optional, only for existing instances)
- `perl installer/data/mysql/updatedatabase.pl`
6. Access Koha Staff and OPAC interfaces and verify that everything works as expected
7. Sign off
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
I don't know if this is appropriate for master as it requires Koha objects,
but this is helpful for upgrading sites to avoid any problems from old mappings
for local-number
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Thomas Klausner <domm@plix.at>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
See comment 4 for a detail explanation of the problem.
Test plan:
hit /cgi-bin/koha/installer/install.pl
login with the DB credentials
upgrade
=> Without this patch you get
```
ISBN_RANGE_MESSAGE is set to [] but that file does not exist!
Trying to use the default locations
```
=> With this patch applied: "Everything went okay. Update done."
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch removes an ISBN_RANGE_MESSAGE env variable that is
accidentally set by Business::ISBN::Data.
Test plan:
- git checkout v25.11.03-2
- reset_all
- git checkout -b 42374 origin/main
- Apply the patch
- restart_all
- Go to staff interface (e.g. http://localhost:8081/)
- Log into web installer
- Click "Update your database"
- Note it works
- Repeat the process but instead of using the web installer
use updatedatabase.pl directly (say through koha-upgrade-schema)
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch splits delete_reports into delete_own_reports and delete_all_reports
While delete_all_reports supercedes delete_own_reports it seems to make more sense than
delete_own_reports and delete_others_reports.
With delete_all_reports we can have a user who can delete all, without specific permission to delete their own
The other way we would have delete others, and you could not delete your own, which seems more odd
To test:
1 - Have 2 users - 1 with delete_reports, 1 without
2 - Apply patches, update database
3 - Confirm user with permission now has 'delete_all_reports' and other user still has no reports deletion permission
4 - Give the second user 'delete_own_reports' permission
5 - Create 4 reports as each user
6 - Confirm view/edit pages show the delete button for each user, and confirm they both work
7 - You shoudl have 2 reports left for each user
8 - Confirm each user can delete their own report from the saved SQL list:
http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?op=list
9 - There should be 1 report from each
10 - Give the second user 'delete_all_reports' permission in one tab and reload the saved reports in another tab
11 - Select both reports in the second tab
12 - Remove 'delete_all_reports' from the second user, leaving 'delete_own_reports' in the first tab
13 - Click 'Delete selected' in the reports tab
14 - Confirm you get 'You do not have permission to continue'
15 - Hit back, deselect other users report, confirm you can delete your own with 'Delete selected'
16 - Sign off
Signed-off-by: Judy Poyer <jpoyer@aacpl.net>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The atomicupdate converts AllowItemsOnHoldCheckoutSCO and
AllowItemsOnHoldCheckoutSIP from YesNo to Choice (0|1|2), where
value 1 now means "Allow pending holds only" and value 2 means
"Allow pending and waiting holds".
For AllowItemsOnHoldCheckoutSCO, pre-upgrade value 1 meant "allow
all on-hold checkouts (pending AND waiting)" at SCO. Post-upgrade,
sco-main.pl reads value 1 as "pending only", silently downgrading
sites that previously allowed waiting-hold checkouts. Remap SCO
value 1 -> 2 at upgrade time to preserve prior behavior.
For AllowItemsOnHoldCheckoutSIP, the SIP checkout code was already
gated on >= 1 for pending and >= 2 for waiting, so value 1 meant
"pending only" in practice even under the old YesNo type. No value
migration is needed -- leaving it at 1 preserves current behavior.
The value remap lives inside the branch that only runs when the
type is not already Choice, so re-running the atomicupdate is safe
and will not double-remap.
Test plan:
1) On a pre-upgrade DB, set both prefs to YesNo/1:
UPDATE systempreferences SET type='YesNo', options=NULL,
value='1'
WHERE variable IN ('AllowItemsOnHoldCheckoutSCO',
'AllowItemsOnHoldCheckoutSIP');
2) Run installer/data/mysql/updatedatabase.pl.
3) Verify:
SELECT variable, type, options, value FROM systempreferences
WHERE variable IN ('AllowItemsOnHoldCheckoutSCO',
'AllowItemsOnHoldCheckoutSIP');
SCO -> type=Choice, options=0|1|2, value=2.
SIP -> type=Choice, options=0|1|2, value=1.
4) Re-run updatedatabase.pl; confirm both prefs report
"already migrated, skipping" and values are unchanged.
5) Spot-check other starting values: SCO=0 stays 0, SCO=2
stays 2, SIP=0 stays 0.
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>
AllowItemsOnHoldCheckoutSCO and AllowItemsOnHoldCheckoutSIP become
Choice (0|1|2) so libraries can distinguish pending from waiting holds:
0 = Don't allow (unchanged)
1 = Allow pending holds only (new semantics; previously "Allow")
2 = Allow pending and waiting holds (preserves old permissive behaviour)
A new AllowHoldCheckoutOverride YesNo preference (default 1) follows
the existing Allow*Override family and lets libraries block the
"check out anyway" override in the staff interface when an item is on
hold for another patron.
Note: installations that previously had either Allow*CheckoutSCO/SIP
set to 1 will migrate to "Allow pending only". Libraries that relied
on the previous behaviour (silently allowing checkout of items already
waiting on the hold shelf) need to manually switch to 2.
Test plan:
- Apply the patch and restart Koha.
- Run the web installer / updatedatabase.pl.
- Confirm no errors and the log mentions the three sysprefs.
- Run it a second time; confirm the "already migrated" / "already
exists" skip messages appear.
- In Administration > System preferences, search for
AllowItemsOnHoldCheckoutSCO and AllowItemsOnHoldCheckoutSIP and
confirm the dropdown now has three options.
- Confirm AllowHoldCheckoutOverride appears in the same section with
the Allow/Don't allow choices and defaults to Allow.
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>
Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
- Replace IFNULL with COALESCE in the atomicupdate to satisfy the
forbidden_patterns check (IFNULL is non-standard SQL).
- Run misc/devel/tidy.pl over the modified catalogue_detail.inc to
satisfy the tidiness check.
Test plan:
1. Run the QA script against the patch series and confirm both
forbidden_patterns and tidiness checks pass.
2. Re-run the parent commit's test plan and confirm spine-label column
visibility behaves identically.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
- Set the executable bit on the atomicupdate so it passes the QA
file_permissions check.
- Drop the unused 'say_info' import; only 'say_warning' and 'say_success'
are now needed.
- Capture the row count from the DELETE so we report 'say_warning' on a
re-run (when the syspref is already gone) instead of falsely claiming
to have deleted it again.
- Add an inline comment explaining the conservative
'(is_hidden OR VALUES(is_hidden))' conflict-resolution rule used when a
table-settings row already exists alongside the old syspref value.
Test plan:
1. Verify the atomicupdate file has the executable bit set.
2. On a database where the syspref is still present, run the
atomicupdate. Confirm output reads
"Deleted 'SpineLabelShowPrintOnBibDetails' syspref".
3. Run the atomicupdate a second time. Confirm output now reads
"'SpineLabelShowPrintOnBibDetails' syspref already removed".
4. Confirm the columns_settings rows are unchanged on re-run.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The catalogue_detail.inc include is shared by both the holdings_table
and otherholdings_table tabs, so the column visibility migration must
seed both. Without this, sites that previously had
'SpineLabelShowPrintOnBibDetails' set to "Don't show" would still see
the spine-label column appear in the other-holdings tab.
Also mark 'spinelabel' as is_hidden by default in admin/columns_settings.yml
for both tables, so fresh installs (where the atomicupdate doesn't run)
match the previous syspref default of 0 (Don't show).
Test plan:
1. Reset the database, do not apply the parent patch yet.
2. With the syspref still present, set it to "Don't show" and save.
3. Apply the parent patch and this follow-up, then update the database.
4. Navigate to a record with items in both 'Holdings' and 'Other holdings'
tabs.
5. Verify the 'Spine label' column is hidden in BOTH tabs.
6. Reset the database again. Set the syspref to "Show", save, then apply
patches and update the database.
7. Verify the 'Spine label' column is visible in BOTH tabs.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
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 a new nullable longtext column `composite_scores` to the
`import_record_matches` table to store a JSON object mapping each
matchpoint search_index to the score it contributed to the total.
This allows the UI to display a breakdown of match scores (e.g.
"title:50, author:50") rather than just the aggregate total.
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>
When a refund is processed through Point of Sale, the system creates:
1. A credit account line (the refund)
2. A debit account line (the payout)
The receipt printing logic currently only accounts for RECEIPTs against
credits.
This patch adds a new 'PAYOUT' template for dealing with the negative
debits that make up a payout so we can construct a proper receipt for
this action.
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>
Introduces a new system preference, autoMemberNumValue, which tracks
the highest auto-generated cardnumber ever issued. It is seeded from
the MAX cardnumber across both borrowers and deletedborrowers so
pre-upgrade deletions do not free numbers for re-issuance.
This preference is managed internally by Koha::Patron::fixup_cardnumber
and has no UI (no YAML entry).
Test plan:
1. Apply the atomicupdate:
perl installer/data/mysql/updatedatabase.pl
2. Verify the preference exists:
SELECT * FROM systempreferences WHERE variable = 'autoMemberNumValue';
3. Confirm the seed value matches:
SELECT GREATEST(
COALESCE((SELECT MAX(CAST(cardnumber AS SIGNED)) FROM borrowers
WHERE cardnumber REGEXP '^-?[0-9]+\$'), 0),
COALESCE((SELECT MAX(CAST(cardnumber AS SIGNED)) FROM deletedborrowers
WHERE cardnumber REGEXP '^-?[0-9]+\$'), 0)
);
4. Re-run the atomicupdate and confirm the already-exists warning.
5. Drop the syspref, re-apply the atomicupdate, confirm it seeds
correctly on fresh installs via mandatory/sysprefs.sql.
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The SIP fields in the database are too small for the templates of some configurations, and are truncated during upgrade resulting in invalid templates. It's likely we need more than 255 characters for some templates. We should convert all sip template fields to TEXT fields.
Test Plan:
1) For each template field on a sip account attempt to save a template like:
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXY
2) Note error preventing you from doing so
3) Apply this patch
4) Run updatedatabase.pl
5) Repeat step 1
6) Note you can now do so without error!
Signed-off-by: Charlie Arthur <carthur@slolibrary.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Also include a reference to the notice template from sysprefs
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>
This adds a new 'open_helper_plugin' keyboard shortcut to the
keyboard_shortcuts table, used by the advanced cataloging editor to
open the value_builder popup for the Leader, 006, 007 and 008 fixed
length fields. The default key combination is Shift-Ctrl-H ("Helper")
and is rebindable from Administration > Keyboard shortcuts. The
behaviour bound to this shortcut is added in a follow-up patch.
Test plan:
1. Apply this patch and run installer/data/mysql/updatedatabase.pl in
KTD.
2. Run:
SELECT * FROM keyboard_shortcuts
WHERE shortcut_name = 'open_helper_plugin';
Confirm a row exists with shortcut_keys = 'Shift-Ctrl-H'.
3. Re-run updatedatabase.pl and confirm the atomic update is idempotent
(no error, no duplicate row).
4. On a fresh install (or by re-loading mandatory/keyboard_shortcuts.sql
on an empty table) confirm the same row is created.
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>
Adding an entry to admin/searchengine/elasticsearch/mappings.yaml is only
picked up by fresh installs or when an administrator explicitly resets the
search mappings (rebuild_elasticsearch.pl -r). Existing installations would
otherwise never see the new 'authid' index in the admin UI nor have it
available as a matching index until they ran the reset.
This atomicupdate inserts:
- a search_field row for 'authid' (type 'stdno', staff_client + opac on)
- search_marc_map rows for authorities/marc21/001 and authorities/unimarc/001
(INSERT IGNORE - the rows are shared with 'local-number' and usually
already exist)
- search_marc_to_field rows linking those MARC maps to the new field with
sort on, matching mappings.yaml
It is a no-op if 'authid' is already present.
Administrators still need to rebuild the Elasticsearch index for the new
field to become queryable - this is noted with say_info at the end.
Test plan:
1 - On an existing instance without the patch: note there is no 'authid'
field under Administration > Search engine configuration (Elasticsearch)
for authorities.
2 - Apply the whole patchset.
3 - perl installer/data/mysql/updatedatabase.pl
4 - Re-check the search engine configuration: 'authid' appears for the
authorities index, mapped to 001 for both MARC flavours.
5 - Run the update again - confirm the "already exists, skipping" warning.
6 - misc/search_tools/rebuild_elasticsearch.pl -r -v -a
7 - Confirm a matching rule using index 'authid' now works.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch clarifies the system level notice css preferences.
We rename NoticeCSS to EmailNoticeStylesheet
We rename SlipCSS to PrintSlipStylesheet
We add AllNoticeStylesheet
We add AllNoticeCSS to contain inline CSS for all html notices
We add EmailNoticeCSS to contain inline CSS for Email notices
We add PrintNoticeCSS to contain inline CSS for Print notices
We add PrintSlipCSS to contain inline CSS for Print slips
Sponsored-by: Open Fifth <https://openfifth.co.uk>
Signed-off-by: Marion Durand <marion.durand@biblibre.com>
Signed-off-by: Thomas Klausner <domm@plix.at>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The restore functionality is a cataloguing operation and should live
under the editcatalogue permission module (bit 9) rather than tools
(bit 13), consistent with where it appears in the UI.
Update all references:
- userpermissions.sql and atomicupdate: module_bit 13 → 9
- Swagger x-koha-authorization: tools → editcatalogue
- CGI flagsrequired: tools → editcatalogue
- Templates: CAN_user_tools_records_restore → CAN_user_editcatalogue_records_restore
- C4::Auth::get_cataloguing_page_permissions: remove from tools
sub-perms list (editcatalogue '*' already covers it)
- Tests: module_bit 13 → 9
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Sponsored-by: ByWater Solutions
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
- Fix permission description to explicitly mention bibliographic records
and items (not just 'records')
- Fix date filter to use correct DB column per table: deletedbiblio uses
'timestamp', deleteditems uses 'deleted_on'
- Remove leftover CAN_user_editcatalogue_edit_catalogue condition from
the Batch editing section in cat-menu.inc; a user with only
edit_catalogue would see an empty section heading
- Wrap biblio and item restore operations in DB transactions so that a
failed store cannot leave a record inserted without being removed from
the deleted table (or vice-versa)
- Catch Koha::Exceptions::Object::DuplicateID in the item restore
controller and return a human-readable 409 instead of a generic 500
- When 'Restore bibliographic record' is clicked in the deleted-bib
warning modal, fetch the biblio and open the full restore modal with
the originally clicked item pre-selected, rather than restoring
immediately without item selection
Signed-off-by: Michaela <michaela.sieber@kit.edu>
Sponsored-by: ByWater Solutions
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
- bug_number was set to 40853 instead of 21820 (probably copy-paste error)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Previously, Koha always added items to the Zebra queue when indexing via Elasticsearch. This adds a new system preference ElasticsearchEnableZebraQueue that allows disabling this behavior.
Unit Test:
* run the unit test t/db_dependent/Koha/SearchEngine/Elasticsearch/Indexer.t
* there should be a new subtest like this:
1..5
ok 1 - We start with an empty zebra queue
ok 2 - new entry in zebraqueue for auth
ok 3 - new entry in zebraqueue for biblio
ok 4 - no new entry in zebraqueue for auth
ok 5 - no new entry in zebraqueue for biblio
ok 5 - syspref ElasticsearchEnableZebraQueue
Manual Test Plan:
* Start ktd with Elasticsearch enabled
* As there is no interface to the zebraqueue, you need to connect to the DB and use SQL directly
* ktd --dbshell
* `select count(*) from zebraqueue;`
* remember that number
* Edit any biblio or auth
* Check the zebraqueue again via SQL
* There should be one more entry
* Now apply the patch
* Then got to Sysprefs and set ElasticsearchEnableZebraQueue to `Disable`
* Edit another biblio or auth
* has to be a different one, because Koha will prevent the same ID to be added again to zebraqueue
* Again check zebraqueue via SQL
* There should be *no* new entry
QA Note:
the qa tool reports
[FAIL] Koha/SearchEngine/Elasticsearch/Indexer.pm
FAIL pod_coverage
sub _get_record has no pod coverage.
But this patch does not touch this code, so I guess this fail is caused by a stricter qa tool
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This has the same goals of bug 24975 but with a different approach.
Bug 24975 tried the "POT/PO files as database tables" approach but
it appeared that is not what best fits our needs.
Instead of a "source text / translated text" mapping, we need
translations to be attached to a specific "object" (or table row), which
has the added benefit of keeping translations unaffected when the
original string changes. It may or may not be what you want but at least
this avoids the problem where fixing an error in the original string
forces users to re-translate everything.
We also need to be able to translate all translatable properties of a
particular entity.
The solution proposed here is to add a DBIC component that will add
several DBIC relations to the `localization` table:
* a relation named `localizations` that will return all translations for
all translatable properties for all languages.
* for each translatable property:
* a relation named `{$property}_localizations` that will return all
translations for a given property, for all languages
* a relation named `{$property}_localization` that will return the
translation for a given property, for the current language
To enable translations for a particular entity, we will need to:
* add two method calls in the "result class" corresponding to the
localizable object type
* in the admin interface, add a link to the the translation popup.
Unlike bug 24975 there is not a page where you can translate
everything. I don't know if it is really useful (probably better to
translate itemtypes when you are on the itemtype page). It can be
added later if needed.
To give an example, this is what needs to be added manually in
Koha/Schema/Result/Itemtype.pm:
__PACKAGE__->load_components('+Koha::DBIx::Class::Localization');
__PACKAGE__->localization_add_relationships(
'itemtype', # the PK column
'description' # a list of translatable columns
);
(see POD in Koha/Schema/Component/Localization.pm for more information)
Final notes:
* I wanted this patch to have as few changes as possible so I kept
methods like `search_with_localization`, `children_with_localization`
even if I believe they can be removed (replaced by search/children)
Localizations do not need to be prefetched/joined unless you want to
sort on them.
For the same reason, $itemtype->unblessed now always return a
`translated_description` key.
This can be fixed in followup patches
* There is cache involved (see
Koha::Schema::Component::Localization::localization).
Localizations in cache are grouped by object type and language (keys
look like 'KOHA:localization:Itemtype:en')
* By keeping a single table for all translations of all entities, we
cannot have data integrity at the RDBMS level, but automatic deletion
of translations is handled by the DBIC `cascade_delete` feature, so
integrity should be fine as long as no raw SQL update/delete operation
is involved.
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch fixes an error when running updatedatabase on an instance using MySQL by defining a key length for an unique key. The unique key in table record_sources was introduced on 35380 but was lacking a key length which works in mariaDB but fails on MySQL.
Test plan:
a) run ktd with mysql f.e.: DB_IMAGE=mysql:8.0 ktd --proxy --name mysql8 up -d
b) enter ktd shell ktd --name mysql8 --shell
c) checkout a Koha version prior to 35380. f.e. "git checkout 67adbce6562fa352b143977747fa290f23c63fde"
d) run 'reset_all'
e) log in and check Koha version in http://mysql8-intra.localhost/cgi-bin/koha/about.pl which should show
25.12.00.036
f) git checkout main
g) restart_all
h) updatedatabase will throw the following error:
Upgrade to 25.12.00.037 [08:29:14]: Bug 35380 - Add new unique name to record sources, add default record sources, add is_system column to default record sources.
ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: BLOB/TEXT column 'name' used in key specification without a key length at /kohadevbox/koha/C4/Installer.pm line 817
i) apply patch
j) 'updatedatabase' should run without errors now
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds predefined values to the record_sources table on installation of a new Koha instance. The values are
- batchmod
- intranet
- batchimport
- z3950
- bulkmarcimport
- import_lexile
and were formerly hardcoded in marc-overlay-rules.tt. This was introduced in 35380 which took care of updated instances via a db_revs script but forgot installation scripts.
Testplan
a) start up a fresh ktd
b) login and check go to Administration -> Record sources
c) check that no source exists
d) apply patch
e) go to mysql shell 'koha-mysql kohadev'
f) empty the database to trigger Kohas installer: 'drop database koha_kohadev;' followed by a 'create database koha_kohadev;'
g) restart_all
h) open Koha in browser again, the web installer should appear now. Credentials are 'koha_kohadev' with pasword 'password'.
i) run through the installation and create an admin user,
j) login with that user and repeat b) and c)
k) check that record sources are now set with the values from the descriptioni above.
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>
The categorycode column comment in the categories table had a typo:
'idenfity' instead of 'identify'. This fixes it in kohastructure.sql
and adds an atomic update to correct existing installations.
Test plan:
1. Apply this patch
2. Run:
$ updatedatabase
=> SUCCESS: The atomic update is applied
3. Verify:
echo "SHOW FULL COLUMNS FROM categories WHERE Field='categorycode';" | koha-mysql kohadev
=> SUCCESS: The comment reads 'unique primary key used to identify the patron category'
4. Sign off :-D
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Bug 37893 only creates the sip2 userflag if the instance has a
SIPConfig.xml set up. This userflag needs to be created reguardless
or the instance will not be able to use SIP in the future!
Test Plan:
1) Ensure you have no SIPConfig.xml
2) Run the bug 37893 update
3) Note the missing userflag
4) Apply this patch
5) Run updatedatabase.pl again
6) Note the userflag exists!
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
This also includes a 'manager' accessor which returns the Koha::Patron
object
Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
- Add order_by => ['name'] to record_sources fetch in marc-overlay-rules.pl
so filter dropdown options are consistently sorted
- Add COMMENT to record_sources.is_system column in kohastructure.sql
to match convention of other columns in that table
- Remove commented-out Data::Dumper debug lines from biblios.t
- Remove redundant explicit object deletes inside subtests that are
already wrapped in txn_rollback; the rollback undoes them anyway
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
INSERT IGNORE silently skips rows whose name already exists, leaving
their is_system column at 0. Systems that already had 'intranet',
'z3950' etc. as record sources would not have them protected from
deletion. Added an explicit UPDATE after the INSERT to ensure all
known system record source names have is_system=1 regardless of
whether the row was just inserted or already existed.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Make atomicupdate script executable, add html filters to tt, annotate is_system as boolean (SQL12 coding guidelines)
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
This changes the structure of record source with an is_system column which controls whether this value can be deleted.
Also predefined values are inserted to reflect the default hard coded values from 'Record overlay rules' sources
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
This adds a new system preference to control the amount of boost. The default of 0 will act as before this patch.
You can add any positive number, or a number between 0 and 1 for negative boost
To test:
1 - Add a record with
245 $a novel
2 - Add a record with
245 $a A novel : $b about things / $c by me
3 - Search for: novel
4 - Search for: a novel
5 - Apply patch, restart all
6 - Repeat searches, note exact titles are boosted
7 - Test with more titles as you are able
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Thomas Klausner <domm@plix.at>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Replace the O(n²) accumulate-and-re-insert pattern with a simple
per-row execute inside the while loop.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
This patch removes Baker & Taylor integration following the shutdown of
the company.
To test, apply the patch, run the database update, and restart all.
- In the staff interface go to Administration -> System preferences and
search for 'Baker.' There should be no results.
- If necessary enable at least one cover image service, e.g.
AmazonCoverImages, OpacCoce, GoogleJackets, etc.
- Log in to the OPAC and confirm that cover images display correctly on
these pages:
- Search results
- Bibliographic detail page
- Shelf browser
- Your summary -> Checked out
- Checkout history
- Recent comments
- Lists
Sponsored-by: Athens County Public Libraries
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Sponsored-by: Cheshire West and Chester Council
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
This patch adds a new PatronAgeRestriction, which acts in the same way
PatronSelfRegistrationAgeRestriction (a restriction on the maximum age
of a borrower), except for the intranet. It will also act as the
maximum age restrictor on the OPAC, in the event that the
PatronSelfRegistrationAgeRestriction is not set.
To test:
== APPLY PATCH ==
a) updatedatabase
b) Set the PatronAgeRestriction syspref to 20, set the
PatronSelfRegistrationAgeRestriction to 30
c) On the OPAC, try to create or modify a patron whose age is 35 years
*) Note the error message presented, suggesting the age exceeds the
maximum
d) On the OPAC, try to create or modify a patron whose age is 25 years
*) Note the submission succeeds
e) On the staff client, try to create or modify a patron whose age is
25 years *) Note the error message presented, suggesting the age
exceeds the maximum
f) Set the PatronSelfRegistrationAgeRestriction to empty
g) On the OPAC, try to create or modify a patron whose age is 25 years
*) Note the error message presented, suggesting the age exceeds
the maximum
h) On the OPAC, try to create or modify a patron whose age is 19 years
*) Note the submission succeeds
i) Set the PatronAgeRestriction to empty
j) On the staff client, try to create or modify a patron whose age is
75 years
*) Note the submission succeeds
k) On the OPAC, try to create or modify a patron whose age is 75 years
*) Note the submission succeeds
== SIGN OFF ==
Sponsored-by: Cheshire West and Chester Council
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
This patch corrects a typo in the auto renewal error code and related
notice templates:
- Adds database update to fix existing notice templates in letter table
- Fixes sample notices for new installations
- Fixes renew_strings.inc template include
The error code changes from 'auto_too_much_oweing' to 'auto_too_much_owing'
throughout.
This follow-up patch addresses feedback from the QA process:
1. System preference naming consistency
Renamed the following preferences to remove "OPAC" prefix and align
with the FineNoRenewals naming convention:
- OPACFineNoRenewalsIncludeCredits → FineNoRenewalsIncludeCredits
- OPACFineNoRenewalsBlockAutoRenew → FineNoRenewalsBlockAutoRenew
2. Self-checkout backward compatibility
Added new system preference FineNoRenewalsBlockSelfCheckRenew to
control whether fine limits block renewals via web-based self-checkout.
Previously, OPACFineNoRenewals did not block self-checkout renewals.
With the rename to FineNoRenewals, this behavior changed. The new
preference defaults to "0" (don't block) to maintain backward
compatibility, allowing sites to opt-in to blocking self-checkout
renewals when patrons exceed the fine threshold.
Test plan:
1. Run prove t/db_dependent/Circulation.t - all tests should pass
2. Run prove t/db_dependent/api/v1/checkouts.t - all tests should pass
3. Verify system preferences are correctly renamed in the admin interface
4. Test self-checkout renewals with FineNoRenewalsBlockSelfCheckRenew
set to 0 (default) - renewals should work even when fines exceed limit
5. Test self-checkout renewals with FineNoRenewalsBlockSelfCheckRenew
set to 1 - renewals should be blocked when fines exceed limit
Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
This patch adds new syspref AllowFineOverrideRenewing. It allows
staff to renew items for patrons whose fines are over limit set
in syspref FineNoRenewals. Otherwise renewing is prevented. If
renewing items is allowed staff is still required to confirm
if they really want to renew items for the patron.
To test:
1. Apply this patch, update database and restart all services.
2. Find patron with fines over limit set in FineNoRenewals and
renewable items.
=> Note that renewing is prevented with message "Cannot renew,
the patron has a debt of..."
3. Set syspref AllowFineOverrideRenewing as "Allow".
=> Note that items can now be renewed and renewing them displays
confirmation pop-up.
Sponsored-by: Koha-Suomi Oy
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-of-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
OPACFineNoRenewals no longer applies just to OPAC.
To avoid confusion rename it as FineNoRenewals and
also rename all occurences in code.
To test, confirm everything works as described in
previous test plans.
Also prove t/db_dependent/Circulation.t
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch fixes the BZ 33363 database update, so that it
selects uses with the "suggestions" permission rather than the "catalogue"
permission.
Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
In the previous tests we had:
$b =~ s/_/ZZZ/g; # mysql sorts underscore last, if you modify this qa-test-tools will need adjustments
But this is wrong, try:
select variable from systempreferences;
and confirm that hide_marc is before hidelostitems and that
intranet_includes is before IntranetAddMastheadLibraryPulldown
We will deal with qa-test-tools once this is pushed.
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
In vim `:%s/^\(('[^']*', \?'[^']*'\).*/\1),/`
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
We currently have to set options, explanation and type in sysprefs.sql and the atomic/dbrev files.
However we (almost) never use them, we rely on the values in the yml files.
We could remove them and don't require them when when add new sysprefs.
It will avoid the mess we currently have (mismatch between new and updated installations, see bug 41682 and bug 41800).
Instead of fixing the discrepancies (again, see bug 41800) we should
NULL the 3 columns in sysprefs.sql and so at the DB level to rely only
on the yaml files.
This patch does several things:
1. Remove the 3 columns from sysprefs.sql to use the default NULL
2. Atomic update:
* Set to NULL for existing installations as well
* Rename UseICUStyleQUotes => UseICUStyleQuotes
* Delete OpacMoreSearches and OPACMySummaryNote, old prefs that have been moved
to additional contents
3. Fix some errors in yaml files (mostly empty entries)
4. Move parsing of yaml files to a dedicated Koha::Devel::Syspref module
5. Retrieve all yaml content using new methods added to Koha::Config::SysPrefs
(that will be reused in follow-up bugs)
Test plan:
1. Run updatedatabase and confirm that now all systempreferences rows in DB
have options, explanation and type set to NULL (except local-use prefs)
2. `prove t/db_dependent/check_sysprefs.t` should return green
3. Confirm that the UI is still working correctly
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
To test:
1 - Checkout 25.11.x - reset_all
2 - Enable StoreLastBorrower
3 - Issue an item
4 - Ensure item has a row in items_last_borrower
SELECT * FROM items_last_borrower
5 - Checkout main
6 - updatedatabase
7 - Try to delete any item, doesn't have to be the one you issued
DELETE FROM items WHERE itemnumber=93;
8 - ERROR 1451 (23000): Cannot delete or update a parent row: a foreign key constraint fails (`koha_kohadev`.`items_last_borrower`, CONSTRAINT `items_last_borrower_ibfk_1` FOREIGN KEY (`itemnumber`) REFERENCES `items` (`itemnumber`) ON DELETE CASCADE ON UPDATE CASCADE)
9 - repeat 1-5
10 - APPLY PATCH
11 - updatedatabase
12 - Try to delete any item
13 - Success!
14 - Delete the item you issued previously (in the staff interface on via mysql)
15 - Confirm no rows in items_last_borrower
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Peter Moore <peter@mrcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch alters the items_last_borrowers table to allow for NULL borrowers
It also adds two new system preferences to allow controlling the anonymization of items_last_borrower data
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Peter Moore <peter@mrcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds the 'OPACTableColExpandedByDefault' system preference,
disabled by default, which controls whether responsive datatable rows
are expanded or not. This work has been performed after recommendations
from a recent accessibility audit.
TO TEST:
a) set 'OpacLocationBranchToDisplay' to both
b) search the OPAC for history, select first result
c) zoom your browser in to maxiumum, scroll down to holdings table
- notice how the rows are collapsed by default, with '+' visible
**apply patch**
e) refresh details page, notice how the behaviour is currently unchanged
f) change new 'OPACTableColExpandedByDefault' syspref to 'Expand'
g) return to OPAC details page, refresh, zoom in
- notice how the rows are now expanded by default, with '-' visible
**sign off**
Sponsored-by: British Museum
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
We say that you cannot limit transfers internally within a branch, but
it seems we do allow you to, and don't show you when we do
To test:
1 - Go to Admin->Library transfer limits
2 - On a branches page, uncheck the box for that branch
e.g. don't allow FIC to transfer to CPL from CPL
3 - Save
4 - Note the box is still checked
5 - Check the DB
SELECT * FROM branch_transfer_limits;
6 - There is a line to/from CPL!
7 - Apply patches
8 - updatedatabase
9 - Repeat 5 - the line is deleted!
10 - Try to create the rule again, not the box is now disabled
11 - Be sneaky, enable the box, uncheck it and save, no rule created
12 - Be sneaky, edit the HMTL for another button to make it limiting CPL to CPL, save, no rule created
13 - Sign off
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
remaning ocurrences in test files seem okay, this covers the others
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
- Add comprehensive POD documentation for lowest_ratio_library() method
including parameter descriptions, return values, and algorithm explanation
- Add database index on itemtype column in library_float_limits table
to improve query performance when searching by itemtype
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The TitleHoldFeeStrategy system preference was added in the database
update script (251200023.pl) but was missing from sysprefs.sql,
causing issues for fresh installations that don't run the update
script.
This preference likely disappeared during rebases. Adding it in the
correct alphabetical position between 'timeout' and
'todaysIssuesDefaultSortOrder'.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit completely modernizes the hold fee system by migrating from
the deprecated categories.reservefee column to the circulation rules framework
and implementing modern Koha::Objects patterns.
WHAT CHANGED:
1. MIGRATION TO CIRCULATION RULES:
- Moved hold fees from categories.reservefee to circulation_rules table
- Added database migration script with atomic update
- Maintains existing fee amounts during migration
- Provides rule hierarchy: category+itemtype+branch specific rules
2. NEW SYSTEM PREFERENCE: TitleHoldFeeStrategy
- Options: highest, lowest, most_common
- Addresses inconsistent title-level hold fees when items have different fees
- Default: 'highest' (safest approach for library revenue)
3. MODERN Koha::Objects API:
- Koha::Item->holds_fee($patron): Calculate fee for specific item
- Koha::Hold->calculate_hold_fee(): Enhanced item/title-level logic
- Koha::Hold->should_charge($stage): Centralized charging logic
- Koha::Hold->charge_hold_fee(): Modern fee charging with proper accounting
- Removed C4::Reserves::GetReserveFee and ChargeReserveFee
4. ENHANCED FUNCTIONALITY:
- Proper ReservesControlBranch logic (patron home vs item home library)
- Complete HoldFeeMode implementation (placement/collection/not_always)
- Clear OPAC messaging with "You will be charged..." commitment
- Comprehensive test coverage for all fee scenarios
5. TECHNICAL IMPROVEMENTS:
- Object-oriented design following Koha patterns
- Proper error handling and edge case management
- Modern test structure with comprehensive coverage
- Clean separation between fee calculation and charging
TEST PLAN:
A) MIGRATION AND SETUP:
1. Apply all patches and run database update
2. Verify existing hold fees migrated to circulation rules:
- Administration > Circulation and fine rules
- Previous categories.reservefee values now in "Hold fee" column
3. Confirm old categories.reservefee column no longer used
B) BASIC FEE CALCULATION:
1. Set up patron categories with different hold fees in circulation rules
2. Create items with different item types, each with different hold fees
3. Test item-level holds charge correct fees based on specific item
4. Test title-level holds use TitleHoldFeeStrategy preference
C) TITLE-LEVEL HOLD STRATEGIES:
1. Set TitleHoldFeeStrategy to "highest":
- Create title with items having fees $1.00, $2.00, $3.00
- Verify title-level holds charge $3.00
2. Set TitleHoldFeeStrategy to "lowest":
- Verify same title charges $1.00
3. Set TitleHoldFeeStrategy to "most_common":
- Add multiple items with $2.00 fee
- Verify title-level holds charge $2.00 (most common)
D) HOLDFEEMODE PREFERENCES:
1. Set HoldFeeMode to "any_time_is_placed":
- Place holds and verify immediate fee charging
- Check patron account for correct hold fee debits
2. Set HoldFeeMode to "any_time_is_collected":
- Place holds, verify no immediate charges
- Check out items to patron, verify fee charged on collection
3. Set HoldFeeMode to "not_always":
- With available items: verify no fee charged
- With all items checked out but no other holds: verify no fee
- With all items checked out and other holds exist: verify fee charged
E) RULE HIERARCHY TESTING:
1. Set different fees for different category/itemtype/branch combinations
2. Verify most specific rule applies:
- Branch + Category + ItemType (most specific)
- Category + ItemType
- Category only (most general)
3. Test with different patron categories and item types
F) USER INTERFACE TESTING:
1. OPAC interface:
- Verify "You will be charged..." messages display correct amounts
- Test both item-level and title-level hold fee messages
- Confirm fees match actual charges in patron account
2. Staff interface:
- Place holds for patrons and verify fee calculations
- Check Administration > Circulation rules interface works correctly
G) EDGE CASES AND ERROR HANDLING:
1. Test $0.00 fees (no charge scenarios)
2. Test patrons with no applicable fee rules (should default to $0.00)
3. Test items with no holdable copies (should return $0.00)
4. Test invalid/missing itemtypes or categories
5. Test holds on biblios with no items
H) REGRESSION TESTING:
1. Verify existing hold placement workflows unchanged
2. Check patron account charging remains accurate
3. Confirm OPAC and staff interfaces display correctly
4. Test that existing holds continue to work properly
I) UNIT TESTS:
Run the following test suites to verify functionality:
- prove t/db_dependent/Koha/Hold.t
- prove t/db_dependent/Koha/Item.t
- prove t/db_dependent/Reserves.t
- prove t/db_dependent/Circulation.t
BACKWARDS COMPATIBILITY:
- All existing functionality preserved
- Hold fee amounts maintained during migration
- Existing workflows continue unchanged
- Legacy code updated to use new API seamlessly
Sponsored-by: OpenFifth <https://openfifth.co.uk>
Signed-off-by: Jackie Usher <jackie.usher@westsussex.gov.uk>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit adds the database migration to move hold fee configuration
from patron categories to circulation rules, providing more granular
control over hold fees based on patron category, item type, and branch.
The migration:
- Creates circulation rules for existing categories.reservefee values
- Removes the deprecated reservefee column from categories table
Sponsored-by: OpenFifth <https://openfifth.co.uk>
Signed-off-by: Jackie Usher <jackie.usher@westsussex.gov.uk>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Also fix QA failures:
[FAIL] installer/data/mysql/atomicupdate/bug_41624.pl
FAIL file_permissions
File must have the exec flag
FAIL forbidden_patterns
forbidden pattern: trailing space char (line 13)
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Bug 41624: Revert "Bug 35211: (QA follow-up) Fix counting"
This reverts commit 97a022054d.
Bug 41624: Revert "Bug 35211: (QA follow-up) fix display of other results tab and move template toolkit logic outside of script array"
This reverts commit c4c2a4b235.
Bug 41624: Revert "Bug 35211: Removed local items from group tabs and removed group items from other holdings tab"
This reverts commit 124c6c8670.
Bug 41624: Revert "Bug 35211: Fixed group tab item building logic"
This reverts commit 23eb2dbd39.
Bug 41624: Revert "Bug 35211: Fix tab building"
This reverts commit f438535b5c.
Bug 41624: Revert "Bug 35211: Set SeparateHoldingsByGroup to separate based on SeparateHoldingsBranch syspref"
This reverts commit d2a0527d86.
Bug 41624: Revert "Bug 35211: Fixed count on logged in holdings"
This reverts commit 96c7eb3459.
Bug 41624: Revert "Bug 35211: Reordered tabs in Holdings Table and fixed group tabnames"
This reverts commit 5c2b0fd496.
Bug 41624: Revert "Bug 35211: Separate group holdings into individual tabs"
This reverts commit 08045c8009.
Bug 41624: DB update to remove the pref
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Pedro Amorim
Lucas Gass
Jan Kissig
Brendan Lawlor
Emily Lamancusa
Saiful Amin
Nick Clemens
Jonathan Druart
David Cook
Martin Renvoize
Matt Blenkinsop
Olivia Reynolds
Andrew Fuerste Henry
Kyle M Hall
Thomas Klausner
Martin Renvoize
Jacob O'Mara
Paul Derscheid
Julian Maurice
Tomás Cohen Arazi
Janusz Kaczmarek
Michael Hafen
Owen Leonard
Katrin Fischer
Jake Deery
Emmi Takkinen