feat: Ability to invite circles #5420

onny · 2023-08-18T16:09:56Z

This commit adds support for adding circles as attendees to a calendar event.
The relationship between the imported group and members will be compliant with
the iCal specification.

A circle with the title "testcircle" will be added as an attendee with iCal
attributes "CUTYPE=GROUP` and uri "mailto:circle+CIRCLEID@CIRCLE_INSTANCE".

Members of the circle will be imported as standard attendees. Each member gets
assigned to the circle group entry by assigning them to the group uri using the
iCal member property: "MEMBER='mailto:circle+CIRCLEID@CIRCLE_INSTANCE'".

Searching for circles is only enabled if the circles app is activated.

Circles added to the list of attendees get imported only once and are not
synced yet. While adding a circle, a notice about this is shown to the user.
Only members of local circles which are local users get imported.

Rendering groups in the frontend is done in a separate PR
https://github.com/nextcloud/calendar/pull/5396

Closes #2954

Work inspired by @tcitworld PR #4742

st3iny · 2023-09-05T07:54:18Z

Hey, I just saw this PR and wanted to thank you for tackling this. Could you please add more context to the PR so that we can proceed, e.g what needs to be done and a short description?

It certainly is a nice feature :)

codecov · 2023-09-30T13:19:03Z

Codecov Report

Attention: 71 lines in your changes are missing coverage. Please review.

Comparison is base (83bb7be) 24.67% compared to head (df2d51b) 24.53%.
Report is 1 commits behind head on main.

Files	Patch %	Lines
.../components/Editor/Invitees/InviteesListSearch.vue	0.00%	36 Missing ⚠️
src/services/circleService.js	0.00%	22 Missing ⚠️
src/components/Editor/Invitees/InviteesList.vue	0.00%	9 Missing ⚠️
src/store/calendarObjectInstance.js	0.00%	3 Missing ⚠️
src/services/isCirclesEnabled.js	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #5420      +/-   ##
============================================
- Coverage     24.67%   24.53%   -0.15%     
- Complexity      406      418      +12     
============================================
  Files           241      243       +2     
  Lines         10693    10785      +92     
  Branches       1739     1755      +16     
============================================
+ Hits           2639     2646       +7     
- Misses         8054     8139      +85

Flag	Coverage Δ
javascript	`15.57% <0.00%> (-0.10%)`	⬇️
php	`61.60% <ø> (-0.77%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

onny · 2023-10-02T15:10:17Z

Hey, I just saw this PR and wanted to thank you for tackling this. Could you please add more context to the PR so that we can proceed, e.g what needs to be done and a short description?

It certainly is a nice feature :)

Tank you for your interest. Currently thinking on how to query circle information and the circle members with their email addresses all at once with one request. Propably I'll have to add an additional backend controller / api?

onny · 2023-10-09T13:17:21Z

This is the current state of both PRs combined (rendering groups & invite circles) 🥳

onny · 2023-10-16T12:52:50Z

@miaulalala @jancborchardt @nimishavijay @tcitworld @st3iny I tested and simplified the PR a bit more, would be nice to know if there's anything missing :)

jancborchardt

Looks very nice! Only a small wording notice: In the search suggestions it says "Contains 3 contacts" in the subline, but when the Circle is added it says "3 members".
The latter is nicer and I would recommend to also shorten & simplify the subline of the search suggestion to " 3 members". Then it's consistent. :)

onny · 2023-10-18T16:13:49Z

Thank you for reviewing :) Fixed it!

miaulalala

~~Testing this currently.~~

~~First thing - the circle membership is unclear with the list of attendees like this:~~

~~I don't know if it would be possible to have the members displayed as a sub- group. Quick'n'dirty mockup:~~

It also sends an invitation email to the one creating the event if they are also a member of the circle. In principle, the ORGANIZER can also be an ATTENDEE, so it's not technically wrong, but it creates awkward visuals as well because the calendar app will show me the accept/decline buttons on an event that I made:

Plus I'm a bit concerned about side effects for singificant changes and other stuff if I'm both ORGANIZER and ATTENDEE.

I think it's neccessary to filter out the circle member that is the organizer if that is the case and not set them as an attendee.

Other than that it works flawlessly and I'm super grateful @onny!

miaulalala · 2023-10-23T14:50:21Z

Ah, does #5396 address the comments from above about the display?

onny · 2023-10-27T11:36:37Z

@miaulalala I adjusted the code so that the organizer of the event doesn't get imported from the circle :)

miaulalala · 2023-10-27T12:11:32Z

@miaulalala I adjusted the code so that the organizer of the event doesn't get imported from the circle :)

I will test it today, thanks a lot!

miaulalala

Hi onny, I fixed the tests locally, so if you implement the suggestions it should run through. You will also need an additional entry in psalm.xml.

Thanks for your hard work - @ChristophWurst and I have agreed since Circles has no public API, it's fine to leave the backend code as is. If you want you can add a comment above the call to the private API in that regard as well that this has been approved but us.

Looking forward to see this in action!!!! 😍

tests/php/unit/Controller/ViewControllerTest.php

psalm.xml

lib/Controller/ViewController.php

tests/php/unit/Controller/ViewControllerTest.php

onny · 2023-11-07T18:16:27Z

Thank you @miaulalala and @ChristophWurst for the review. Added the requested changes to the PR :)

onny · 2023-11-17T19:36:16Z

@miaulalala I refactored the code which computes groups and memberships a bit to be more robust and simple. Works quite good for me now :)
Still have a look at the PHP linting errors etc 👀

phpunit --configuration phpunit.unit.xml --fail-on-warning
PHPUnit 9.6.13 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.0-dev with Xdebug 3.3.0alpha4-dev
Configuration: phpunit.unit.xml

...............................................................  63 / 173 ( 36%)
..............................EEEEEEE.......................... 126 / 173 ( 72%)
...............................................                 173 / 173 (100%)

Time: 00:01.075, Memory: 46.00 MB

There were 7 errors:

1) OCA\Calendar\Controller\ViewControllerTest::testIndex
TypeError: Mock_CompareVersion_cf83b8dc::isCompatible(): Argument #2 ($required) must be of type string, int given, called in /home/runner/work/calendar/calendar/nextcloud/apps/calendar/lib/Controller/ViewController.php on line 131

/home/runner/work/calendar/calendar/nextcloud/apps/calendar/lib/Controller/ViewController.php:131
/home/runner/work/calendar/calendar/nextcloud/apps/calendar/tests/php/unit/Controller/ViewControllerTest.php:175

2) OCA\Calendar\Controller\ViewControllerTest::testIndexViewFix with data set #0 ('agendaDay', 'timeGridDay')
TypeError: Mock_CompareVersion_cf83b8dc::isCompatible(): Argument #2 ($required) must be of type string, int given, called in /home/runner/work/calendar/calendar/nextcloud/apps/calendar/lib/Controller/ViewController.php on line 131

/home/runner/work/calendar/calendar/nextcloud/apps/calendar/lib/Controller/ViewController.php:131
/home/runner/work/calendar/calendar/nextcloud/apps/calendar/tests/php/unit/Controller/ViewControllerTest.php:268

3) OCA\Calendar\Controller\ViewControllerTest::testIndexViewFix with data set #1 ('timeGridDay', 'timeGridDay')
TypeError: Mock_CompareVersion_cf83b8dc::isCompatible(): Argument #2 ($required) must be of type string, int given, called in /home/runner/work/calendar/calendar/nextcloud/apps/calendar/lib/Controller/ViewController.php on line 131

/home/runner/work/calendar/calendar/nextcloud/apps/calendar/lib/Controller/ViewController.php:131
/home/runner/work/calendar/calendar/nextcloud/apps/calendar/tests/php/unit/Controller/ViewControllerTest.php:268

4) OCA\Calendar\Controller\ViewControllerTest::testIndexViewFix with data set #2 ('agendaWeek', 'timeGridWeek')
TypeError: Mock_CompareVersion_cf83b8dc::isCompatible(): Argument #2 ($required) must be of type string, int given, called in /home/runner/work/calendar/calendar/nextcloud/apps/calendar/lib/Controller/ViewController.php on line 131

/home/runner/work/calendar/calendar/nextcloud/apps/calendar/lib/Controller/ViewController.php:131
/home/runner/work/calendar/calendar/nextcloud/apps/calendar/tests/php/unit/Controller/ViewControllerTest.php:268

5) OCA\Calendar\Controller\ViewControllerTest::testIndexViewFix with data set #3 ('timeGridWeek', 'timeGridWeek')
TypeError: Mock_CompareVersion_cf83b8dc::isCompatible(): Argument #2 ($required) must be of type string, int given, called in /home/runner/work/calendar/calendar/nextcloud/apps/calendar/lib/Controller/ViewController.php on line 131

/home/runner/work/calendar/calendar/nextcloud/apps/calendar/lib/Controller/ViewController.php:131
/home/runner/work/calendar/calendar/nextcloud/apps/calendar/tests/php/unit/Controller/ViewControllerTest.php:268

6) OCA\Calendar\Controller\ViewControllerTest::testIndexViewFix with data set #4 ('month', 'dayGridMonth')
TypeError: Mock_CompareVersion_cf83b8dc::isCompatible(): Argument #2 ($required) must be of type string, int given, called in /home/runner/work/calendar/calendar/nextcloud/apps/calendar/lib/Controller/ViewController.php on line 131

/home/runner/work/calendar/calendar/nextcloud/apps/calendar/lib/Controller/ViewController.php:131
/home/runner/work/calendar/calendar/nextcloud/apps/calendar/tests/php/unit/Controller/ViewControllerTest.php:268

7) OCA\Calendar\Controller\ViewControllerTest::testIndexViewFix with data set #5 ('dayGridMonth', 'dayGridMonth')
TypeError: Mock_CompareVersion_cf83b8dc::isCompatible(): Argument #2 ($required) must be of type string, int given, called in /home/runner/work/calendar/calendar/nextcloud/apps/calendar/lib/Controller/ViewController.php on line 131

/home/runner/work/calendar/calendar/nextcloud/apps/calendar/lib/Controller/ViewController.php:131
/home/runner/work/calendar/calendar/nextcloud/apps/calendar/tests/php/unit/Controller/ViewControllerTest.php:268

ERRORS!
Tests: 173, Assertions: 564, Errors: 7.

Generating code coverage report in Clover XML format ... done [00:00.082]
Script phpunit --configuration phpunit.unit.xml --fail-on-warning handling the test event returned with error code 2
Error: Process completed with exit code 2.

jancborchardt · 2023-12-04T11:35:21Z

@onny did anything change design-wise? Cause otherwise I would say it is still fine. :)

miaulalala · 2023-12-04T15:03:18Z

@onny can you rebase on the current main? Tests will pass again, then I will take a look at the linter issues.

This commit adds support for adding circles as attendees to a calendar event. The relationship between the imported group and members will be compliant with the iCal specification. A circle with the title "testcircle" will be added as an attendee with iCal attributes "CUTYPE=GROUP` and uri "mailto:circle+CIRCLEID@CIRCLE_INSTANCE". Members of the circle will be imported as standard attendees. Each member gets assigned to the circle group entry by assigning them to the group uri using the iCal member property: "MEMBER='mailto:circle+CIRCLEID@CIRCLE_INSTANCE'". Searching for circles is only enabled if the circles app is activated. Circles added to the list of attendees get imported only once and are not synced yet. While adding a circle, a notice about this is shown to the user. Only members of local circles which are local users get imported. Rendering groups in the frontend is done in a separate PR nextcloud#5396 Signed-off-by: Jonas Heinrich <[email protected]>

onny · 2023-12-04T17:36:33Z

@onny did anything change design-wise? Cause otherwise I would say it is still fine. :)

I refactored the code in src/components/Editor/Invitees/InviteesList.vue which computes groups and memberships a bit to be more robust and simple

@miaulalala Rebased the PR

miaulalala · 2023-12-05T09:28:48Z

@ChristophWurst codecov coverage is complaining, am I ok to ignore? I would like to get this merged today 😊

ChristophWurst · 2023-12-05T09:31:26Z

The circles code is probably pain to create test for due to the lack of a stable API so I'll wave this through

miaulalala · 2023-12-05T09:35:24Z

The circles code is probably pain to create test for due to the lack of a stable API so I'll wave this through

I'll test this one last time during lunch time and then I'll force merge.

Thanks so much for your work and patience @onny !

jancborchardt

Still looks good design-wise :)

miaulalala

Perfect!

github-actions · 2024-05-01T02:08:16Z

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

onny mentioned this pull request Sep 25, 2023

feat: Ability to render groups as attendees #5396

Merged

2 tasks

onny force-pushed the invite-circles branch 7 times, most recently from 7dda494 to 201aa4d Compare September 30, 2023 09:00

onny force-pushed the invite-circles branch 4 times, most recently from 6abd2d6 to 2801294 Compare October 2, 2023 14:43

onny marked this pull request as ready for review October 8, 2023 11:51

onny force-pushed the invite-circles branch 2 times, most recently from a14695b to 7da0c49 Compare October 8, 2023 14:31

jancborchardt requested changes Oct 16, 2023

View reviewed changes

miaulalala requested changes Oct 23, 2023

View reviewed changes

onny force-pushed the invite-circles branch from cea0b14 to 4bea3c5 Compare October 27, 2023 11:37

onny requested review from jancborchardt and miaulalala October 27, 2023 11:37

This comment was marked as outdated.

Sign in to view

tcitworld mentioned this pull request Nov 6, 2023

Possibility to add a Nextcloud circle as attendees for an event [$250] #2954

Closed

miaulalala requested changes Nov 7, 2023

View reviewed changes

tests/php/unit/Controller/ViewControllerTest.php Show resolved Hide resolved

tests/php/unit/Controller/ViewControllerTest.php Show resolved Hide resolved

psalm.xml Show resolved Hide resolved

lib/Controller/ViewController.php Show resolved Hide resolved

miaulalala requested changes Nov 7, 2023

View reviewed changes

tests/php/unit/Controller/ViewControllerTest.php Outdated Show resolved Hide resolved

onny force-pushed the invite-circles branch 3 times, most recently from df5f44f to 04eeeaa Compare November 7, 2023 18:15

onny requested a review from miaulalala November 7, 2023 18:15

onny force-pushed the invite-circles branch from a72c94a to 8a5e60a Compare November 17, 2023 19:32

onny force-pushed the invite-circles branch from 8a5e60a to a7b5378 Compare November 18, 2023 12:15

onny force-pushed the invite-circles branch from a7b5378 to 52b8b08 Compare November 27, 2023 17:06

onny requested a review from jancborchardt November 27, 2023 17:11

jancborchardt requested review from tcitworld and st3iny December 4, 2023 11:34

miaulalala added this to the v4.7.0 milestone Dec 4, 2023

onny force-pushed the invite-circles branch from 52b8b08 to df2d51b Compare December 4, 2023 17:33

jancborchardt approved these changes Dec 5, 2023

View reviewed changes

miaulalala approved these changes Dec 5, 2023

View reviewed changes

miaulalala merged commit 4449e39 into nextcloud:main Dec 5, 2023
36 of 38 checks passed

github-actions bot added the feedback-requested label May 1, 2024

kesselb mentioned this pull request Sep 6, 2024

fix(teams): resolve undefined variable error and add logging #6290

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Ability to invite circles #5420

feat: Ability to invite circles #5420

onny commented Aug 18, 2023 •

edited by tcitworld

Loading

st3iny commented Sep 5, 2023

codecov bot commented Sep 30, 2023 •

edited

Loading

onny commented Oct 2, 2023

onny commented Oct 9, 2023

onny commented Oct 16, 2023

jancborchardt left a comment

onny commented Oct 18, 2023

miaulalala left a comment •

edited

Loading

miaulalala commented Oct 23, 2023

onny commented Oct 27, 2023

miaulalala commented Oct 27, 2023

This comment was marked as outdated.

miaulalala left a comment •

edited

Loading

onny commented Nov 7, 2023

onny commented Nov 17, 2023 •

edited

Loading

jancborchardt commented Dec 4, 2023

miaulalala commented Dec 4, 2023

onny commented Dec 4, 2023

miaulalala commented Dec 5, 2023

ChristophWurst commented Dec 5, 2023

miaulalala commented Dec 5, 2023

jancborchardt left a comment

miaulalala left a comment

github-actions bot commented May 1, 2024

feat: Ability to invite circles #5420

feat: Ability to invite circles #5420

Conversation

onny commented Aug 18, 2023 • edited by tcitworld Loading

st3iny commented Sep 5, 2023

codecov bot commented Sep 30, 2023 • edited Loading

Codecov Report

onny commented Oct 2, 2023

onny commented Oct 9, 2023

onny commented Oct 16, 2023

jancborchardt left a comment

Choose a reason for hiding this comment

onny commented Oct 18, 2023

miaulalala left a comment • edited Loading

Choose a reason for hiding this comment

miaulalala commented Oct 23, 2023

onny commented Oct 27, 2023

miaulalala commented Oct 27, 2023

This comment was marked as outdated.

miaulalala left a comment • edited Loading

Choose a reason for hiding this comment

onny commented Nov 7, 2023

onny commented Nov 17, 2023 • edited Loading

jancborchardt commented Dec 4, 2023

miaulalala commented Dec 4, 2023

onny commented Dec 4, 2023

miaulalala commented Dec 5, 2023

ChristophWurst commented Dec 5, 2023

miaulalala commented Dec 5, 2023

jancborchardt left a comment

Choose a reason for hiding this comment

miaulalala left a comment

Choose a reason for hiding this comment

github-actions bot commented May 1, 2024

onny commented Aug 18, 2023 •

edited by tcitworld

Loading

codecov bot commented Sep 30, 2023 •

edited

Loading

miaulalala left a comment •

edited

Loading

miaulalala left a comment •

edited

Loading

onny commented Nov 17, 2023 •

edited

Loading