builttoroam / device_calendar

A cross platform plugin for modifying calendars on the user's device
https://pub.dev/packages/device_calendar
BSD 3-Clause "New" or "Revised" License
267 stars 267 forks source link

Adding new features #435

Open jifisherki opened 2 years ago

jifisherki commented 2 years ago

Database query optimization Added updating recurring event single instance and future instances Added start week field for recurrence rule Added return organizer for android module. Added NonParticipant role Changed return type to create or update event to Event Added canAddAttendees and possibleAttendeeProblems for calendar model Added choice between call in isolate or without it Added the ability to use the plugin in the background on Android Added new method for get events from any calendars

dwenderf commented 2 years ago

This looks like a great PR @jifisherki !

I see that you "Added updating recurring event single instance and future instances", however I don't see how to update a single instance or all future instances of an event. I'd love to test this and some of the other features, so if you can help out by letting me know how to do this, that would be very helpful.

I'm also really hoping that the plugin performs better on Android. The existing plugin is quite slow retrieving events on Android relative to iOS. I see you did some query optimization, so I'm looking forward to see how that affects performance.

jifisherki commented 2 years ago

Hi @dwenderf

I see that you "Added updating recurring event single instance and future instances", however I don't see how to update a single instance or all future instances of an event.

For example:

switch (updateDeleteType) {
  case UpdateDeleteEventType.all:
    result = await _calendarPlugin.createOrUpdateEvent(dcEvent);
    break;
  case UpdateDeleteEventType.following:
    result = await _calendarPlugin.updateEventInstance(dcEvent, oldEvent!.start, oldEvent.end, true);
    break;
  case UpdateDeleteEventType.single:
    result = await _calendarPlugin.updateEventInstance(dcEvent, oldEvent!.start, oldEvent.end, false);
    break;
}

As for android. In my opinion, the performance gain is quite high

dwenderf commented 2 years ago

Thanks @jifisherki!

I have one small suggestion for your code. For method updateEventInstance in device_calendar.dart, the parameter should probably be updateFollowingInstances rather than deleteFollowingInstances:

  Future<Result<UnmodifiableListView<Event>>?> updateEventInstance(
    Event? event,
    DateTime? startDate,
    DateTime? endDate,
    bool updateFollowingInstances, {
    bool useIsolate = false,
  }) 
jifisherki commented 2 years ago

@dwenderf

I agree with you

thomassth commented 2 years ago

Please rebase the commit, which should solve the build test error.

And if something changed, please also update the tests.

thomassth commented 2 years ago

Actually, it would be better if you may split the distinct changes into separate PRs. It would be easier for me and others to inspect what changed, and it would be easier to fix if anything goes wrong.