Fix Multi-day Ongoing

[muness]

• Added regression coverage in tests/icalUtils.test.ts:84 ensuring multi-day events only repeat across their span when showOngoing is enabled, and stay isolated to the start day when it’s off.
• Reworked shouldIncludeOngoing in src/icalUtils.ts:171 to compare normalized day boundaries, skip the already-added start day, include interior days, and include the end day only when the event actually continues into it. The new test regression now passes.

Summary by CodeRabbit

• Bug Fixes
  • Improved multi-day event handling to correctly display events across all spanned dates when ongoing events are enabled.

[coderabbitai]

Walkthrough

The shouldIncludeOngoing function in src/icalUtils.ts is refactored to implement explicit day-based inclusion logic for ongoing events, replacing a simple between-check with day normalization, boundary comparisons, and a guard excluding the initial occurrence. A corresponding test validates multi-day event filtering behavior with and without the showOngoing flag.

Changes

Cohort / File(s)	Summary
Ongoing event filtering logic src/icalUtils.ts	Refactored shouldIncludeOngoing function with normalized day-based comparisons, explicit start/end day handling, and guard logic to exclude the first occurrence of ongoing events.
Test coverage and formatting tests/icalUtils.test.ts	Added new test for multi-day event handling with showOngoing flag enabled and disabled. Minor whitespace adjustments to existing test block.

Sequence Diagram

sequenceDiagram
    participant Filter as Event Filter
    participant shouldInclude as shouldIncludeOngoing()
    participant Logic as Day Logic

    Filter->>shouldInclude: dayToMatch, event
    
    rect rgb(200, 220, 255)
    Note over Logic: Day Normalization
    shouldInclude->>Logic: normalize dayToMatch to start of day
    end
    
    rect rgb(220, 240, 220)
    Note over Logic: Boundary Checks
    Logic->>Logic: is dayToMatch >= event start day?
    Logic->>Logic: is dayToMatch <= event end day?
    end
    
    rect rgb(255, 240, 200)
    Note over Logic: Inclusion Rules
    alt Between start and end
        Logic->>Logic: Include day strictly between
    else On end day
        Logic->>Logic: Include if event continues past start
    else On start day
        Logic->>Logic: Exclude (guard: skip initial)
    end
    end
    
    shouldInclude-->>Filter: boolean result

Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

The changes modify core filtering logic with day-based boundary handling and introduce a guard condition that requires careful verification of edge cases (start day, end day, multi-day events). The new test adds confidence but the logic density and impact on event inclusion filtering warrant focused review.

Poem

🐰 A hop through the calendar days,
Where ongoing events find their ways,
No duplicates of the start so bright,
Between and end now handled right,
Filter logic blooms in morning light! 🌅

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "Fix Multi-day Ongoing" directly and clearly describes the primary objective of the changeset. The PR updates the shouldIncludeOngoing function to correct how multi-day events are handled when the ongoing feature is enabled, and adds a regression test to verify this behavior. The title is concise, specific, and avoids vague terminology or noise, accurately reflecting the main change from the developer's perspective. A teammate reviewing the PR history would immediately understand that this addresses multi-day event handling in ongoing event scenarios.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch muness/issue198

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 887fdf1 and 5af3d31.

📒 Files selected for processing (2)

• src/icalUtils.ts (1 hunks)
• tests/icalUtils.test.ts (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

src/icalUtils.ts (1)

tests/__mocks__/obsidian.ts (1)

• moment (2-2)

tests/icalUtils.test.ts (1)

src/icalUtils.ts (1)

• filterMatchingEvents (194-248)

🔇 Additional comments (2)

tests/icalUtils.test.ts (1)

84-110: Great regression test for the multi-day ongoing fix!

The test comprehensively validates the core behavior: showOngoing OFF restricts the event to its start date, while showOngoing ON includes all spanned dates.

Note: Consider adding test coverage for recurring multi-day events with showOngoing enabled, as this combination isn't currently tested and could have different behavior (see comment on src/icalUtils.ts line 242).

src/icalUtils.ts (1)

171-192: Excellent refactor of the ongoing event logic!

The day-based inclusion logic is now explicit and correct:

• Normalizes to start-of-day for consistent comparisons
• Skips the start day to avoid duplicating the initial occurrence (already added via line 229)
• Includes all days strictly between start and end
• Correctly includes the end day only when the event continues past midnight (diff > 0)

This properly handles both regular multi-day events and all-day events (which end at the next day's midnight).

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}