Enable/disable CalDAV calendar synchronization #1091

bertrandda · 2021-03-07T18:39:26Z

Pull Request check-list

To ensure your Pull Request can be accepted as fast as possible, make sure to review and check all of these items:

If your changes affects code, did your write the tests?
Are tests passing? (npm test on both front/server)
Is the linter passing? (npm run eslint on both front/server)
Did you run prettier? (npm run prettier on both front/server)
If you are adding a new features/services, did you run integration comparator? (npm run compare-translations on front)
If your changes modify the API (REST or Node.js), did you modify the API documentation? (Documentation is based on comments in code)
If you are adding a new features/services which needs explanation, did you modify the user documentation? See the GitHub repo and the website.
Did you add fake requests data for the demo mode (front/src/config/demo.json) so that the demo website is working without a backend? (if needed) See https://demo.gladysassistant.com.

NOTE: these things are not required to open a PR and can be done afterwards / while the PR is open.

Description of change

Add the ability to choose which calendars to synchronize in CalDAV integration.

codecov · 2021-03-07T19:36:36Z

Codecov Report

Merging #1091 (c421ef3) into master (8de2652) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1091      +/-   ##
==========================================
+ Coverage   96.41%   96.42%   +0.01%     
==========================================
  Files         640      643       +3     
  Lines        8163     8193      +30     
==========================================
+ Hits         7870     7900      +30     
  Misses        293      293

Impacted Files	Coverage Δ
server/api/controllers/calendar.controller.js	`100.00% <100.00%> (ø)`
server/lib/calendar/calendar.destroyEvents.js	`100.00% <100.00%> (ø)`
server/lib/calendar/calendar.get.js	`100.00% <100.00%> (ø)`
server/lib/calendar/index.js	`100.00% <100.00%> (ø)`
server/lib/index.js	`94.44% <100.00%> (ø)`
server/services/caldav/api/caldav.controller.js	`100.00% <100.00%> (ø)`
...es/caldav/lib/calendar/calendar.disableCalendar.js	`100.00% <100.00%> (ø)`
...ces/caldav/lib/calendar/calendar.enableCalendar.js	`100.00% <100.00%> (ø)`
.../caldav/lib/calendar/calendar.syncUserCalendars.js	`100.00% <100.00%> (ø)`
server/services/caldav/lib/index.js	`100.00% <100.00%> (ø)`
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8de2652...c421ef3. Read the comment docs.

atrovato

Looks good to my eyes, but didn't try with real calendars. Please find any testers to confirm it works well.

server/test/lib/calendar/calendar.event.test.js

atrovato

Good for me, but didn't test in real life.

VonOx · 2021-03-13T11:39:17Z

Hi @bertrandda

I've tested this PR with Google calendar

No error in logs, but no event in Gladys Calendar ( Waited from 2H to be sure )

2021-03-13T11:07:15+0100 <info> calendar.cleanUp.js:12 (CalDAVHandler.cleanUp) Start cleaning CalDAV data for user: 8da2b78a-bbbb-4804-b39c-a0a39ed0ca66
2021-03-13T11:07:15+0100 <info> calendar.cleanUp.js:14 (CalDAVHandler.cleanUp) 0 calendars to clean
2021-03-13T11:07:30+0100 <info> calendar.requests.js:52 () CalDAV : Found calendar XXXXXXX@gmail.com
2021-03-13T11:07:30+0100 <info> calendar.requests.js:52 () CalDAV : Found calendar 4CAD
2021-03-13T11:07:30+0100 <info> calendar.requests.js:52 () CalDAV : Found calendar Anniversaires
2021-03-13T11:07:30+0100 <info> calendar.requests.js:52 () CalDAV : Found calendar Jours fériés en France
2021-03-13T11:07:30+0100 <info> calendar.syncUserCalendars.js:38 (CalDAVHandler.syncUserCalendars) CalDAV : Found 4 calendars.
2021-03-13T11:07:34+0100 <info> calendar.syncUserCalendars.js:132 (Promise.map.concurrency) CalDAV : 167 events updated for calendar XXXXXXX@gmail.com.
2021-03-13T11:07:35+0100 <info> calendar.syncUserCalendars.js:132 (Promise.map.concurrency) CalDAV : 57 events updated for calendar Jours fériés en France.
2021-03-13T11:07:37+0100 <info> calendar.syncUserCalendars.js:132 (Promise.map.concurrency) CalDAV : 45 events updated for calendar Anniversaires.
2021-03-13T11:07:42+0100 <info> calendar.syncUserCalendars.js:132 (Promise.map.concurrency) CalDAV : 250 events updated for calendar 4CAD.
2021-03-13T11:07:48+0100 <debug> bluetooth.scanPresence.js:24 (BluetoothManager.scanPresence) Bluetooth: 0 presence sensor features
2021-03-13T11:08:03+0100 <info> calendar.enableCalendar.js:19 (CalDAVHandler.enableCalendar) Calendar jours-feries-en-france disabled & emptied
2021-03-13T11:08:03+0100 <info> calendar.enableCalendar.js:19 (CalDAVHandler.enableCalendar) Calendar anniversaires disabled & emptied
2021-03-13T11:08:26+0100 <info> calendar.requests.js:52 () CalDAV : Found calendar XXXXXXX@gmail.com
2021-03-13T11:08:26+0100 <info> calendar.requests.js:52 () CalDAV : Found calendar 4CAD
2021-03-13T11:08:26+0100 <info> calendar.requests.js:52 () CalDAV : Found calendar Anniversaires
2021-03-13T11:08:26+0100 <info> calendar.requests.js:52 () CalDAV : Found calendar Jours fériés en France
2021-03-13T11:08:26+0100 <info> calendar.syncUserCalendars.js:38 (CalDAVHandler.syncUserCalendars) CalDAV : Found 4 calendars.

bertrandda · 2021-03-13T19:41:35Z

@VonOx There is no error in your logs & I just tried with Google calendar, it seems to work on my side. Can you check your t_calendar_event table in the database ?
To avoid waiting you can force synchronisation in Integrations -> Account -> Sync

VonOx · 2021-03-13T19:57:33Z

Seem's to be a problem with my instance/google calendar.

PR is ok !

Pierre-Gilles · 2021-03-22T08:23:36Z

server/services/caldav/api/caldav.controller.js

+   * @apiName Enable
+   * @apiGroup CalDAV
+   */
+  async function enable(req, res) {


It's weird to have an "enable" function that either enable or disable

Either you have a PATCH calendar API call (simple), or either you have a "enable" + "disable" route.

Pick one :)

I chose to split in 2 routes because we need actions after disabling

Pierre-Gilles

See my comments :)

Pierre-Gilles

Looks good to me!

Just one change, I just merged this PR: #1133

And we can no longer use this.context.intl.dictionary in the front, but instead we use a HoC component to inject the dictionnary as props :)

Could you make the change in this PR? (see the PR to see how to do it, it's pretty easy)

Thanks for your PR!

atrovato

Code is great enough for me.

Pierre-Gilles

Tested locally, works fine!

) Co-authored-by: Pierre-Gilles Leymarie <pierregilles.leymarie@gmail.com>

atrovato previously approved these changes Mar 10, 2021

View reviewed changes

server/test/lib/calendar/calendar.event.test.js Outdated Show resolved Hide resolved

bertrandda dismissed atrovato’s stale review via 2f12ed8 March 10, 2021 20:02

atrovato previously approved these changes Mar 11, 2021

View reviewed changes

VonOx previously approved these changes Mar 13, 2021

View reviewed changes

Pierre-Gilles reviewed Mar 22, 2021

View reviewed changes

Pierre-Gilles requested changes Mar 22, 2021

View reviewed changes

bertrandda dismissed stale reviews from VonOx and atrovato via eaaf5cb April 16, 2021 09:48

bertrandda requested a review from Pierre-Gilles April 16, 2021 11:42

Pierre-Gilles requested changes Apr 19, 2021

View reviewed changes

bertrandda added 9 commits April 19, 2021 22:10

Enable/disable caldav calendar synchronization

ad5365b

Change translation strings

56cff5e

Improve code coverage

719750f

Remove useless calendar request

ffa9d3b

Fix assertion to destroyEvents test

04ae31e

Fix prettier

81eb9ac

Split enable/disable controllers

67e7e55

Add test on controller

5faa6a6

Use intl as props

f4fa360

bertrandda force-pushed the feature/caldav-disable-sync branch from ef7b5d7 to f4fa360 Compare April 19, 2021 20:47

bertrandda requested a review from Pierre-Gilles April 19, 2021 22:20

atrovato reviewed Apr 20, 2021

View reviewed changes

Merge branch 'master' into feature/caldav-disable-sync

58bfb4d

Pierre-Gilles approved these changes Apr 22, 2021

View reviewed changes

Merge branch 'master' into feature/caldav-disable-sync

c421ef3

Pierre-Gilles merged commit 0141aeb into GladysAssistant:master May 3, 2021

bertrandda deleted the feature/caldav-disable-sync branch May 3, 2021 10:21

Jean-PhilippeD pushed a commit to Jean-PhilippeD/Gladys that referenced this pull request Oct 13, 2021

Add Enable/disable CalDAV calendar synchronization (GladysAssistant#1091

a24b13b

) Co-authored-by: Pierre-Gilles Leymarie <pierregilles.leymarie@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable/disable CalDAV calendar synchronization #1091

Enable/disable CalDAV calendar synchronization #1091

bertrandda commented Mar 7, 2021 •

edited

Loading

codecov bot commented Mar 7, 2021 •

edited

Loading

atrovato left a comment

atrovato left a comment

VonOx commented Mar 13, 2021

bertrandda commented Mar 13, 2021

VonOx commented Mar 13, 2021

Pierre-Gilles Mar 22, 2021

bertrandda Apr 16, 2021

Pierre-Gilles left a comment

Pierre-Gilles left a comment

atrovato left a comment

Pierre-Gilles left a comment

Enable/disable CalDAV calendar synchronization #1091

Enable/disable CalDAV calendar synchronization #1091

Conversation

bertrandda commented Mar 7, 2021 • edited Loading

Pull Request check-list

Description of change

codecov bot commented Mar 7, 2021 • edited Loading

Codecov Report

atrovato left a comment

Choose a reason for hiding this comment

atrovato left a comment

Choose a reason for hiding this comment

VonOx commented Mar 13, 2021

bertrandda commented Mar 13, 2021

VonOx commented Mar 13, 2021

Pierre-Gilles Mar 22, 2021

Choose a reason for hiding this comment

bertrandda Apr 16, 2021

Choose a reason for hiding this comment

Pierre-Gilles left a comment

Choose a reason for hiding this comment

Pierre-Gilles left a comment

Choose a reason for hiding this comment

atrovato left a comment

Choose a reason for hiding this comment

Pierre-Gilles left a comment

Choose a reason for hiding this comment

bertrandda commented Mar 7, 2021 •

edited

Loading

codecov bot commented Mar 7, 2021 •

edited

Loading