| Summary: | Obscure notice handling in overdue_notices.pl needs to be improved | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Marcel de Rooy <m.de.rooy> |
| Component: | Architecture, internals, and plumbing | Assignee: | Bugs List <koha-bugs> |
| Status: | NEW --- | QA Contact: | Testopia <testopia> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | jonathan.druart, nick, tomascohen |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29354 | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Comma delimited list of Sponsors: | |
| Crowdfunding goal: | 0 | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
ccing Nick Tomas Jonathan Lowering severity because I just now see that parse_overdues_letter does call GetPreparedLetter after all. This code really needs to be improved. Obscure stuff. This whole stuff with loops, repeat, substitute in old and new style with or without enclosing tag is such a mess ! A librarian changed a few notices, and I just had a 'diagonal glance' over them. They looked good, but didnt work finally ;) But resolving it, almost needs debugging Letters and Overdues code. No Koha advertising.. |
The overdue_notices.pl script contains a lot of code for letter/notice handling that really should not be there. Some regular notice constructs do not work since it is doing its own thing. Look at this example: my $letter = parse_overdues_letter( { letter_code => $overdue_rules->{"letter$i"}, borrowernumber => $borrowernumber, branchcode => $branchcode, items => \@items, substitute => { # this appears to be a hack to overcome incomplete features in this code. bib => $library->branchname, # maybe 'bib' is a typo for 'lib<rary>'? 'items.content' => $titles, 'count' => $itemcount, }, # If there is no template defined for the requested letter # Fallback on the original type message_transport_type => $letter_exists ? $effective_mtt : $mtt, } ); The comment talks about a hack to overcome incomplete features. I did mark this as major, and not enhancement for refactoring.