Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FreeBusyStatus GetFreeBusyStatus(IDateTime dt) does not handle all day events. #212

Closed
johnwhoffman opened this issue Dec 29, 2016 · 4 comments

Comments

@johnwhoffman
Copy link

johnwhoffman commented Dec 29, 2016

I found this issue in our code and repro-ed in a unit test you can add to your FreeBusy unit test class.
GetFreeBusyStatus() should return Busy for all the asserts but always returns Free.
I believe the issue is GetFreeBusy relies on Period.Contains() to determine if there is an overlap.
foreach (var fbe in Entries.Where(fbe => fbe.Contains(dt) && status < fbe.Status))
I think there needs to be logic that handles all day events which always have T000000

BEGIN:VCALENDAR
PRODID:-//github.com/rianjs/ical.net//NONSGML ical.net 2.2//EN
VERSION:2.0
BEGIN:VEVENT
DTEND:20161228T000000
DTSTAMP:20161229T151857Z
DTSTART:20161228T000000
RRULE:FREQ=DAILY;BYDAY=MO,TU,WE,TH,FR
SEQUENCE:0
SUMMARY:00:15:00
UID:ca7053f9-7923-4701-8e02-8f677a6c5120
END:VEVENT
END:VCALENDAR
[TestMethod, TestCategory("FreeBusy")]
public void GetFreeBusyStatus2()
{
    var cal = new Calendar();

    var evt = cal.Create<Event>();
    evt.Summary = "Test event";
    evt.Start = new CalDateTime(2010, 10, 1, 0, 0, 0);
    var oneDayBefore = evt.Start.Subtract(new TimeSpan(1, 0, 0, 0));
    var oneDayAfter = evt.Start.Add(new TimeSpan(1, 0, 0, 0));
    evt.End = new CalDateTime(2010, 10, 1, 0, 0, 0);

    var freeBusy = cal.GetFreeBusy(oneDayBefore, oneDayAfter);

    var orderedEntries = freeBusy.Entries.OrderBy(p => p.StartTime).ToList();

    foreach (var entry in orderedEntries)
    {
        System.Diagnostics.Trace.WriteLine($"IFreeBusyEntry.Start: {entry.StartTime}, IFreeBusyEntry.End: {entry.EndTime}, IFreeBusyEntry.Status: {entry.Status}");
    }
    Assert.AreEqual(FreeBusyStatus.Free, freeBusy.GetFreeBusyStatus(new CalDateTime(2010, 10, 1, 7, 59, 59)));
    Assert.AreEqual(FreeBusyStatus.Busy, freeBusy.GetFreeBusyStatus(new CalDateTime(2010, 10, 1, 8, 0, 0)));
    Assert.AreEqual(FreeBusyStatus.Busy, freeBusy.GetFreeBusyStatus(new CalDateTime(2010, 10, 1, 8, 59, 59)));
    Assert.AreEqual(FreeBusyStatus.Free, freeBusy.GetFreeBusyStatus(new CalDateTime(2010, 10, 1, 9, 0, 0)));
}
@rianjs
Copy link
Collaborator

rianjs commented Dec 29, 2016

Does #161 have any relevance here?

@johnwhoffman
Copy link
Author

Does not seem to be related but I am not up to speed on all your unit test patterns.
AFAICT the all day event concept does not seem to be tested in your test suites.
Please correct me if I have missed something.

@rianjs
Copy link
Collaborator

rianjs commented Dec 29, 2016

Oh, I was responding to

I believe the issue is GetFreeBusy relies on Period.Contains() to determine if there is an overlap.

Contains vs Overlaps is talked about in that thread. A closer reading suggests this is different.

@johnwhoffman
Copy link
Author

johnwhoffman commented Dec 29, 2016

Here is the fix I am adding to our copy of the code.
Let me know if you find issues with this.
Thanks for your efforts!

` public virtual FreeBusyStatus GetFreeBusyStatus(IDateTime dt)
{
var status = FreeBusyStatus.Free;
if (dt == null)
{
return status;
}

  foreach (FreeBusyEntry  fbe in Entries)
  {
    if (fbe.Contains(dt))
    {
      if (status < fbe.Status)
      {
        status = fbe.Status;
      }
    }
    else if (fbe.EndTime.CompareTo(fbe.StartTime) == 0 && dt.GreaterThan(fbe.StartTime))
    {
      // all day event
      status = fbe.Status;
    }
  }
  return status;
}`

@axunonb axunonb closed this as completed Oct 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants