Bug 32262 - smart-rules: Some rules can be created but are not visible on interface
Summary: smart-rules: Some rules can be created but are not visible on interface
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 27945 30794
Blocks:
  Show dependency treegraph
 
Reported: 2022-11-18 10:05 UTC by Marcel de Rooy
Modified: 2023-12-30 20:44 UTC (History)
3 users (show)

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


Attachments
Bug 32262: Handle non-existing circ rules (5.60 KB, patch)
2022-11-22 08:40 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 Marcel de Rooy 2022-11-18 10:05:58 UTC
See e.g. bug 30794 and bug 27945.
On 30794 you can create a rule with all inputs blank for a patron category under 'Default checkout, hold policy by patron category'. You wont see it.
Same for creating an rule for open_article_request_limit outside the interface though (bad practice generally) with itemtype NULL iirc. You wont see it.
Comment 1 Jonathan Druart 2022-11-22 08:12:05 UTC
Hum, CirculationRules.Search() never returns an undefined value and, more generally, methods from the TT plugins return an empty string when we actually return undef...
Comment 2 Jonathan Druart 2022-11-22 08:14:22 UTC
Ok, perl returns an empty string for undef in a string context.
Comment 3 Jonathan Druart 2022-11-22 08:40:12 UTC
Created attachment 144143 [details] [review]
Bug 32262: Handle non-existing circ rules

Some rules are not displayed on the UI, even if they exist in the DB,
that's confusing.
That's because we are implicitely returning 'undef' from
CirculationRules.Search, and perl returns an empty string for undef in
a string context. Which means "rules.defined" is always true.

This patch is ugly and really hope somebody else will come with a better
solution.

Test plan:
I don't know, search for regressions on the circ rule UI?... :D
Comment 4 Jonathan Druart 2022-11-22 08:41:42 UTC
Note that want_rule flag could have unexpected behaviours