Fix midnight-crossing events showing as 'all day'

[chrisballinger]

Summary

• Fixed 255 events with incorrect time display where events crossing midnight were showing as "all day"
• Events like "Hot Dogs & Hard Techno Beats" (Thu 10PM-2AM) now display correctly as 4-hour events
• Replaced flawed date swap logic with smart correction that trusts start date/time

Problem

Users reported events incorrectly displaying as "all day" when they crossed midnight. Investigation revealed 255 events in the 2025 data with negative durations (end before start):

• 75 same-day midnight crossers (10PM-2AM type events)
• 180 data entry errors (wrong end date selected)

The previous fix was swapping dates, which created long-duration events that got marked as "all day".

Solution

New approach: Trust the start date/time as correct, apply end time to start date

• If end time is still before start (same day), add 1 day for midnight crossing
• This fixes ALL 255 events with a simple, consistent approach

Test Plan

• Build succeeds
• All 255 events now have valid positive durations
• Verify events display correct time ranges in app
• Test with specific examples like "Hot Dogs & Hard Techno Beats"

Documentation

Created comprehensive documentation in Docs/2025-08-22-fix-midnight-crossing-events.md

🤖 Generated with Claude Code

[claude]

Claude finished @chrisballinger's task —— View job

---

PR Review Complete

This PR effectively fixes a critical bug where midnight-crossing events were incorrectly displayed as "all day" events. The solution is well-thought-out and properly documented.

Todo List:

• Read documentation file to understand full context
• Analyze the modified BRCRecurringEventObject.m file
• Review logic changes for correctness and edge cases
• Assess code quality and best practices
• Evaluate security and performance considerations
• Check test coverage implications
• Provide comprehensive review feedback

Review Feedback

✅ Strengths

Logic and Correctness (iBurn/BRCRecurringEventObject.m:52-75)

• The new approach is much more intuitive: "trust the start date/time as correct"
• Properly handles both midnight crossers and data entry errors
• The two-step correction (apply end time to start date, then add day if needed) is elegant
• Edge case handling is solid with the <= 0 check for same-day midnight crossing

Code Quality

• Clear variable naming (endTimeComponents, startDateComponents, correctedEndDate)
• Good separation of concerns - extracts time components separately from date components
• Appropriate logging for debugging with before/after values
• Maintains existing code style and patterns

Documentation

• Excellent comprehensive documentation explaining the problem, solution, and results
• Clear examples with specific event names help understand the impact
• Good traceability to previous related work

⚠️ Areas for Improvement

Performance Considerations

• Minor: Creating NSCalendar *calendar = [NSCalendar currentCalendar] inside the loop (line 54) could be optimized by moving it outside the enumerateObjectsUsingBlock since it's the same for all iterations
• Impact: Low - only affects events with negative durations (255 out of total events)

Code Robustness

• Edge Case: No validation that [calendar dateFromComponents:startDateComponents] succeeds - could return nil in edge cases
• Edge Case: The logging statement (line 72-73) could potentially cause issues if correctedEndDate is nil
• Suggestion: Add nil checks after dateFromComponents calls

Memory Management

• The code follows proper ARC patterns, no issues detected

🔒 Security Assessment

• No security concerns identified
• No user input processing or external data validation involved
• Date manipulation uses system APIs appropriately

🧪 Test Coverage Recommendations

The PR mentions testing in the description but could benefit from:

1. Unit tests for the date correction logic - specifically test cases for:

   • Midnight crossers (10PM-2AM scenarios)
   • Data entry errors (wrong end date selected)
   • Edge cases (exactly midnight, 11:59PM-12:01AM)
   • Nil date handling

2. Integration tests to verify:

   • Events no longer show as "all day" when they shouldn't
   • 12-hour threshold still works correctly
   • No regression in normal event handling

🚀 Minor Optimization Suggestion

// Move calendar creation outside the loop for better performance
NSCalendar *calendar = [NSCalendar currentCalendar];

[self.eventTimes enumerateObjectsUsingBlock:^(BRCEventTime *eventTime, NSUInteger idx, BOOL *stop) {
    // ... existing code ...
    
    if ([endDate timeIntervalSinceDate:startDate] < 0) {
        // Use the pre-created calendar instance
        NSDateComponents *endTimeComponents = [calendar components:(NSCalendarUnitHour | NSCalendarUnitMinute | NSCalendarUnitSecond) fromDate:endDate];
        // ... rest of logic unchanged ...
    }
}];

Overall Assessment

✅ Recommend approval - This is a solid fix that addresses a real user-facing issue with a clean, understandable solution. The performance impact is minimal, and the logic is sound. The comprehensive documentation makes this an exemplary PR.

The suggested optimizations are minor and don't block approval. The fix successfully resolves 255 problematic events and should significantly improve user experience.

[claude]

Claude finished @chrisballinger's task —— View job

---

PR Review Complete

This PR effectively fixes a critical bug where midnight-crossing events were incorrectly displayed as "all day" events. The solution is well-thought-out and properly documented.

Todo List:

• Read documentation file to understand full context
• Analyze the modified BRCRecurringEventObject.m file
• Review logic changes for correctness and edge cases
• Assess code quality and best practices
• Evaluate security and performance considerations
• Check test coverage implications
• Provide comprehensive review feedback

Review Feedback

✅ Strengths

Logic and Correctness (iBurn/BRCRecurringEventObject.m:52-75)

• The new approach is much more intuitive: "trust the start date/time as correct"
• Properly handles both midnight crossers and data entry errors
• The two-step correction (apply end time to start date, then add day if needed) is elegant
• Edge case handling is solid with the <= 0 check for same-day midnight crossing

Code Quality

• Clear variable naming (endTimeComponents, startDateComponents, correctedEndDate)
• Good separation of concerns - extracts time components separately from date components
• Appropriate logging for debugging with before/after values
• Maintains existing code style and patterns

Documentation

• Excellent comprehensive documentation explaining the problem, solution, and results
• Clear examples with specific event names help understand the impact
• Good traceability to previous related work

⚠️ Areas for Improvement

Performance Considerations

• Minor: Creating NSCalendar *calendar = [NSCalendar currentCalendar] inside the loop (line 54) could be optimized by moving it outside the enumerateObjectsUsingBlock since it's the same for all iterations
• Impact: Low - only affects events with negative durations (255 out of total events)

Code Robustness

• Edge Case: No validation that [calendar dateFromComponents:startDateComponents] succeeds - could return nil in edge cases
• Edge Case: The logging statement (line 72-73) could potentially cause issues if correctedEndDate is nil
• Suggestion: Add nil checks after dateFromComponents calls

Memory Management

• The code follows proper ARC patterns, no issues detected

🔒 Security Assessment

• No security concerns identified
• No user input processing or external data validation involved
• Date manipulation uses system APIs appropriately

🧪 Test Coverage Recommendations

The PR mentions testing in the description but could benefit from:

1. Unit tests for the date correction logic - specifically test cases for:

   • Midnight crossers (10PM-2AM scenarios)
   • Data entry errors (wrong end date selected)
   • Edge cases (exactly midnight, 11:59PM-12:01AM)
   • Nil date handling

2. Integration tests to verify:

   • Events no longer show as "all day" when they shouldn't
   • 12-hour threshold still works correctly
   • No regression in normal event handling

🚀 Minor Optimization Suggestion

// Move calendar creation outside the loop for better performance
NSCalendar *calendar = [NSCalendar currentCalendar];

[self.eventTimes enumerateObjectsUsingBlock:^(BRCEventTime *eventTime, NSUInteger idx, BOOL *stop) {
    // ... existing code ...
    
    if ([endDate timeIntervalSinceDate:startDate] < 0) {
        // Use the pre-created calendar instance
        NSDateComponents *endTimeComponents = [calendar components:(NSCalendarUnitHour | NSCalendarUnitMinute | NSCalendarUnitSecond) fromDate:endDate];
        // ... rest of logic unchanged ...
    }
}];

Overall Assessment

✅ Recommend approval - This is a solid fix that addresses a real user-facing issue with a clean, understandable solution. The performance impact is minimal, and the logic is sound. The comprehensive documentation makes this an exemplary PR.

The suggested optimizations are minor and don't block approval. The fix successfully resolves 255 problematic events and should significantly improve user experience.

---