https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37966

--- Comment #17 from Martin Renvoize (ashimema) 
<[email protected]> ---
Created attachment 191258
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191258&action=edit
Bug 37966: (follow-up) Make empty string handling more explicit in
Circulation.pm

This commit improves upon the original fix by making the intent more
explicit when handling empty string date parameters in AddRenewal and
AddIssue functions.

WHY THIS IS BETTER:

The original fix (changing `defined $datedue` to just `$datedue`)
works correctly but relies on Perl's implicit truthy/falsy semantics:

  if ( $datedue && ref $datedue ne 'DateTime' )

This is subtle because:
- Empty string "" is falsy, so parsing is skipped ✓
- But it's not immediately obvious that empty strings are being
  handled specially
- The code doesn't explicitly show its intent

THIS APPROACH:

  if ( defined $datedue && $datedue ne '' && ref $datedue ne 'DateTime' )

Benefits:
1. EXPLICIT: Clearly states "only parse if defined AND not empty
   AND not already a DateTime"
2. SELF-DOCUMENTING: Intent is obvious to future developers
3. NO RELIANCE on truthy/falsy semantics
4. CONSISTENT: Same pattern applied across all date parameter handling

COMPREHENSIVE CHANGES:

1. AddRenewal (C4/Circulation.pm:3449)
   - Made datedue empty string check explicit

2. AddIssue (C4/Circulation.pm:1611)
   - Made datedue empty string check explicit
   - Also updated issuedate handling (line 1616) to explicitly handle
     empty strings

3. Test Coverage (t/db_dependent/Circulation/issue.t)
   - Added test for AddIssue with empty string datedue
   - Added test for AddIssue with empty string issuedate
   - Ensures both functions properly calculate dates from circulation
     rules instead of using "now"

The existing test for AddRenewal already covered empty string datedue
(added in the original Bug 37966 commit). These new tests ensure the
same correct behavior for AddIssue.

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to