Bug 40405 - systempreferences.value cannot be set to NULL
Summary: systempreferences.value cannot be set to NULL
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 25515
  Show dependency treegraph
 
Reported: 2025-07-16 09:46 UTC by Jonathan Druart
Modified: 2025-07-16 10:03 UTC (History)
0 users

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 40405: Set systempreferences.value to NOT NULL (14.87 KB, patch)
2025-07-16 10:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 40405: Set decreaseLoanHighHolds\* to 0 (1.71 KB, patch)
2025-07-16 10:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 40405: [DO NOT PUSH] Show the problem with C4::Context->set_preference (1.39 KB, patch)
2025-07-16 10:03 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2025-07-16 09:46:37 UTC
Some sysprefs have NULL as their default value, and systempreferences.value allows NULL values.

However C4::Context->set_preference does not unset a syspref if "undef" is passed.

Additionally some of the sysprefs that have NULL as default will generate warnings in the logs (and test suite output).

opac/opac-suggestions.pl:        for my $f ( split( /\s*\,\s*/, C4::Context->preference("OPACSuggestionUnwantedFields") ) ) {



Here is the current list:
+-------------------------------+-------+
| variable                      | value |
+-------------------------------+-------+
| decreaseLoanHighHoldsDuration | NULL  |
| decreaseLoanHighHoldsValue    | NULL  |
| ILLHiddenRequestStatuses      | NULL  |
| ILLOpacbackends               | NULL  |
| ILLSendStaffNotices           | NULL  |
| ListOwnerDesignated           | NULL  |
| MarcFieldDocURL               | NULL  |
| MaxFine                       | NULL  |
| OPACSuggestionUnwantedFields  | NULL  |
| ReportsExportLimit            | NULL  |
| SerialsSearchResultsLimit     | NULL  |
+-------------------------------+-------+
Comment 1 Jonathan Druart 2025-07-16 10:01:33 UTC
Created attachment 184129 [details] [review]
Bug 40405: Set systempreferences.value to NOT NULL

Some sysprefs have NULL as their default value, and systempreferences.value allows NULL values.

However C4::Context->set_preference does not unset a syspref if "undef" is passed.

Additionally some of the sysprefs that have NULL as default will generate warnings in the logs (and test suite output).

opac/opac-suggestions.pl:        for my $f ( split( /\s*\,\s*/, C4::Context->preference("OPACSuggestionUnwantedFields") ) ) {

Here is the current list:
+-------------------------------+-------+
| variable                      | value |
+-------------------------------+-------+
| decreaseLoanHighHoldsDuration | NULL  |
| decreaseLoanHighHoldsValue    | NULL  |
| ILLHiddenRequestStatuses      | NULL  |
| ILLOpacbackends               | NULL  |
| ILLSendStaffNotices           | NULL  |
| ListOwnerDesignated           | NULL  |
| MarcFieldDocURL               | NULL  |
| MaxFine                       | NULL  |
| OPACSuggestionUnwantedFields  | NULL  |
| ReportsExportLimit            | NULL  |
| SerialsSearchResultsLimit     | NULL  |
+-------------------------------+-------+

Test plan:
Code review required here, please confirm the above and that the
different occurrences do not use "undef" as a value.

Note that some tests now do not longer generate warnings
(eg. t/db_dependent/selenium/opac_ill_requests.t)
Comment 2 Jonathan Druart 2025-07-16 10:01:35 UTC
Created attachment 184130 [details] [review]
Bug 40405: Set decreaseLoanHighHolds\* to 0

When reviewing the different sysprefs that had "NULL" as default, I
only found those 2 occurrences that could be problematic. I think we
should default to 0 here.
Comment 3 Jonathan Druart 2025-07-16 10:03:19 UTC
Created attachment 184131 [details] [review]
Bug 40405: [DO NOT PUSH] Show the problem with C4::Context->set_preference

Apply this patch before the others and notice the failure.