Check for updates and update public calendars #2917
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2917 +/- ##
============================================
+ Coverage 22.61% 29.43% +6.81%
+ Complexity 395 116 -279
============================================
Files 241 154 -87
Lines 11848 5535 -6313
Branches 2306 817 -1489
============================================
- Hits 2679 1629 -1050
+ Misses 9169 3906 -5263
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
|
I did not read the full code changes yet but is it working in the embedded calendar view as well @azul? |
|
@TheOneWithTheBraid I added the hooks to the main |
azul
force-pushed
the
ux/reloadCalendars
branch
from
cfab71b
to
a1a7333
Compare
Fetch the open calendars every 20 seconds and check if they have changed since they were loaded. If a calendar changed remove all it's data from the store and thereby effectively trigger a reload. This is a fairly brute force approach as it drops all known data rather than fetching only the data that changed via the caldav sync API. However Caldav sync is yet to be implemented. (nextcloud/cdav-library#9) So this serves as a workaround for the time being. See #31. Signed-off-by: Azul <azul@riseup.net>
* Add new calendars to the list as they show up. * Reload non-public calendars if they are updated on the server. Fixes #31. This works but it is not an optimal solution: * If a calendar changes it reloads the current view no matter wether anyting in the view changed or not. * If a calendar changes it drops all cached events even if they did not change. * If an event is added to the calendar or moved this calendar will also be reloaded because the new event changed the syncToken on the server. * If a calendar changed it will be added to the bottom of the list of calendars. I would be fine with most of these but the last one needs to be fixed. Signed-off-by: Azul <azul@riseup.net>
azul
force-pushed
the
ux/reloadCalendars
branch
from
a1a7333
to
48d78d9
Compare
When reloading updated calendars we effectively re-add them. Put them in the list of calendars in the place they belong to according to the `order` given by the dav response. Signed-off-by: Azul <azul@riseup.net>
azul
force-pushed
the
ux/reloadCalendars
branch
10 times, most recently
from
e4a5b6f
to
f1afd6f
Compare
| @@ -120,6 +120,7 @@ private function publicIndex(string $token, | |||
| $defaultSkipPopover = $this->config->getAppValue($this->appName, 'skipPopover', 'yes'); | |||
| $defaultTimezone = $this->config->getAppValue($this->appName, 'timezone', 'automatic'); | |||
| $defaultSlotDuration = $this->config->getAppValue($this->appName, 'slotDuration', '00:30:00'); | |||
| $defaultSyncTimeout = $this->config->getAppValue($this->appName, 'syncTimeout', '00:02:00'); | |||
There was a problem hiding this comment.
This format is kinda ugly and prone to issues. The ISO 8601 duration format is probably better here (we already use if for the refresh rate of subscriptions for instance). On the front-end you can use momentjs to parse it.
There was a problem hiding this comment.
I'm using ISO 8601 in the tests and defaults now and parse the value with moment.duration - so in the end both formats will work.
I'm not entirely sure what more it will take to have a properly working setting. I will open another PR to https://github.com/nextcloud/documentation/blob/master/admin_manual/groupware/calendar.rst to document the setting.
Edit: Removed resolved issue with loading sync_timeout - i was missing a call to loadState.
azul
force-pushed
the
ux/reloadCalendars
branch
3 times, most recently
from
9bf8ae9
to
6734bc3
Compare
The setting is a string that can be formatted as anything that [moment.duration](https://momentjs.com/docs/#/durations/creating/) can parse: * 'hh:mm:ss' * 'PT1M' (ISO 8601) Any invalid setting or setting to less than a second will lead to the calendar updates being disabled. The setting is not exposed in the UI. It's meant to be configured by server admins to be able to adjust the timeout and prevent an overly high load. Signed-off-by: Azul <azul@riseup.net>
azul
force-pushed
the
ux/reloadCalendars
branch
from
6734bc3
to
56fc034
Compare
Description
Fetch calendars every 20 seconds
and check if new ones have been added
or known ones have changed since they were loaded.
If a calendar changed remove all it's data from the store
and thereby effectively trigger a reload.
This is a fairly brute force approach as it drops all known data
rather than fetching only the data that changed via the caldav sync API.
However Caldav sync is yet to be implemented. (nextcloud/cdav-library#9)
So this serves as a workaround for the time being.
Fixes #31.
Type of change
Please delete options that are not relevant.
How to test / use your changes?
Please provide instructions how to test your changes
UI Changes
update-calendar.mp4
Checklist:
git commit -sm "Your commit message")Todolist:
Also reload calendars on the dashboard(The dashboard data structure is quite different from the calendar view. Won't handle it as part of this Pull Request.)