fix(rrule): restore before/after compatibility mapping#500
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe changes fix a compatibility issue in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 18 minutes and 45 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/basic.test.js (1)
304-310: Optional: add boundary/inclusive edge-case assertions for stronger contract coverage.This would lock behavior for
inclusive=trueand no-match boundaries (undefined) as part of the same regression scenario.Suggested test additions
const beforeDate = event.rrule.before(new Date('2023-01-03T09:00:00.000Z'), false); const afterDate = event.rrule.after(new Date('2023-01-03T09:00:00.000Z'), false); + const beforeInclusive = event.rrule.before(new Date('2023-01-03T09:00:00.000Z'), true); + const afterInclusive = event.rrule.after(new Date('2023-01-03T09:00:00.000Z'), true); + const beforeBoundary = event.rrule.before(new Date('2023-01-01T09:00:00.000Z'), false); + const afterBoundary = event.rrule.after(new Date('2023-01-05T09:00:00.000Z'), false); assert_.ok(beforeDate instanceof Date, 'rrule.before() should return a Date'); assert_.ok(afterDate instanceof Date, 'rrule.after() should return a Date'); assert_.equal(beforeDate.toISOString(), '2023-01-02T09:00:00.000Z'); assert_.equal(afterDate.toISOString(), '2023-01-04T09:00:00.000Z'); + assert_.equal(beforeInclusive?.toISOString(), '2023-01-03T09:00:00.000Z'); + assert_.equal(afterInclusive?.toISOString(), '2023-01-03T09:00:00.000Z'); + assert_.equal(beforeBoundary, undefined); + assert_.equal(afterBoundary, undefined);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/basic.test.js` around lines 304 - 310, Add assertions around the inclusive boundary and no-match cases for event.rrule.before and event.rrule.after: call event.rrule.before(..., true) and event.rrule.after(..., true) to assert that when inclusive=true and the input equals an occurrence you get that exact Date, and add assertions that calling before/after with a date outside the recurrence range returns undefined (or falsy) to lock the no-match behavior; reference the existing event.rrule.before and event.rrule.after calls and mirror their toISOString checks to validate the inclusive returns and undefined responses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/basic.test.js`:
- Around line 304-310: Add assertions around the inclusive boundary and no-match
cases for event.rrule.before and event.rrule.after: call event.rrule.before(...,
true) and event.rrule.after(..., true) to assert that when inclusive=true and
the input equals an occurrence you get that exact Date, and add assertions that
calling before/after with a date outside the recurrence range returns undefined
(or falsy) to lock the no-match behavior; reference the existing
event.rrule.before and event.rrule.after calls and mirror their toISOString
checks to validate the inclusive returns and undefined responses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 972c8308-0e63-43ff-9bac-6a061fe4b575
📒 Files selected for processing (2)
ical.jstest/basic.test.js
Map wrapper methods to rrule-temporal API: - before() -> previous() - after() -> next() Add regression test to ensure event.rrule.before()/after() work and return the expected Date values without throwing.
KristjanESPERANTO
force-pushed
the
before-after
branch
from
9f48db0 to
91ffd6e
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@jens-maus coderabbit seems satisfied. Did you noticed this PR? 🙂 |
2 participants
Map wrapper methods to rrule-temporal API:
That should have happened during the migration from rrule to rrule-temporal (#422).
I also added a regression test to ensure event.rrule.before()/after() work and return the expected Date values without throwing.
Fixes #499.
Summary by CodeRabbit
Bug Fixes
before()andafter()methods now correctly identify preceding and following occurrences in recurrence patterns.Tests