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.

Sign up for GitHub

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

Jump to bottom

send invitations for shared calendars #9609

Merged
merged 1 commit into from
Jun 1, 2018
Merged

send invitations for shared calendars #9609

merged 1 commit into from
Jun 1, 2018

Conversation

georgehrke
Copy link
Member

@georgehrke georgehrke commented May 25, 2018 •
edited
Loading

fixes #3830

@georgehrke georgehrke added the 2. developing Work in progress label May 25, 2018
@georgehrke georgehrke added this to the Nextcloud 13.0.3 milestone May 25, 2018
@georgehrke georgehrke force-pushed the bugfix/3830/invitations_for_shared_calendars branch from 17fdafd to 95165f9 Compare May 25, 2018 17:19
@georgehrke
Copy link
Member Author

note to myself: Can this solution cause issues with infinite invitation loops? Should the address list only be limited to the current user accessing it and not to all users possibly having access to it?

@georgehrke
Copy link
Member Author

Can this solution cause issues with infinite invitation loops?

Possibly

Should the address list only be limited to the current user accessing it and not to all users possibly having access to it?

Yes, just replace $calendarNode->getOwner() with $calendarNode->getPrincipalURI()

@georgehrke
Copy link
Member Author

Saved this patch as a gist because I might need the Principal changes for future work: https://gist.github.com/georgehrke/ce704d4370b8e9d60231f2ea35d15f0c

@georgehrke georgehrke force-pushed the bugfix/3830/invitations_for_shared_calendars branch from 95165f9 to b11a043 Compare May 25, 2018 18:28
@codecov
Copy link

codecov bot commented May 25, 2018 •
edited
Loading

Codecov Report

Merging #9609 into master will decrease coverage by 49.08%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master   #9609       +/-   ##
============================================
- Coverage     51.15%   2.07%   -49.09%     
- Complexity    25692   25697        +5     
============================================
  Files          1568    1568               
  Lines         88039   88052       +13     
============================================
- Hits          45036    1824    -43212     
- Misses        43003   86228    +43225
Impacted Files Coverage Δ Complexity Δ
apps/dav/lib/CalDAV/Schedule/Plugin.php 0% <0%> (-32%) 17 <5> (+5)
lib/private/Cache/CappedMemoryCache.php 0% <0%> (-100%) 15% <0%> (ø)
lib/private/Security/CSRF/CsrfTokenManager.php 0% <0%> (-100%) 8% <0%> (ø)
lib/private/Notification/Action.php 0% <0%> (-100%) 24% <0%> (ø)
lib/private/Remote/Credentials.php 0% <0%> (-100%) 3% <0%> (ø)
apps/user_ldap/lib/Migration/UUIDFixGroup.php 0% <0%> (-100%) 1% <0%> (ø)
apps/dav/lib/CalDAV/PublicCalendarObject.php 0% <0%> (-100%) 1% <0%> (ø)
lib/private/Files/Search/SearchQuery.php 0% <0%> (-100%) 6% <0%> (ø)
...lib/Middleware/Exceptions/NotSubAdminException.php 0% <0%> (-100%) 1% <0%> (ø)
core/Controller/NavigationController.php 0% <0%> (-100%) 14% <0%> (ø)
... and 879 more

@georgehrke
send invitations for shared calendars
a9c313c 
Signed-off-by: Georg Ehrke <developer@georgehrke.com>
@georgehrke georgehrke force-pushed the bugfix/3830/invitations_for_shared_calendars branch from b11a043 to a9c313c Compare May 25, 2018 18:42
@georgehrke georgehrke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 25, 2018
@georgehrke
Copy link
Member Author

In contrast to the previous patch this doesn't check whether the current user is a rw-share, but then a person with a ro-share should not be able to update calendar objects and trigger the calendarObjectChange method anyway.

@georgehrke georgehrke changed the title WIP: send invitations for shared calendars send invitations for shared calendars May 25, 2018
This was referenced May 25, 2018
Merged
Merged
Closed
@MorrisJobke MorrisJobke requested a review from rullzer May 30, 2018 09:54
rullzer
rullzer reviewed May 30, 2018
View reviewed changes
apps/dav/lib/CalDAV/Schedule/Plugin.php
$oldObj = null;
}

$this->processICalendarChange($oldObj, $vCal, $addresses, [], $modified);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if new participant addresses don't have an e-mail set does this do 💥 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is properly handled by processICalendarChange.

As I said in the methods comment, this is basically just a copy of parent:: calendarObjectChange, i just replaced line 133. That's why I also didn't write tests 🙈

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough!

@rullzer
Copy link
Member

rullzer commented May 30, 2018

Tests would be appreciated as this is something we might else easily break in the future

rullzer
rullzer approved these changes May 30, 2018
View reviewed changes
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

MorrisJobke
MorrisJobke approved these changes Jun 1, 2018
View reviewed changes
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works 👍

@MorrisJobke MorrisJobke merged commit f5e16df into master Jun 1, 2018
@MorrisJobke MorrisJobke deleted the bugfix/3830/invitations_for_shared_calendars branch June 1, 2018 10:40
@MorrisJobke
Copy link
Member

Reverted in #15676

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@rullzer rullzer rullzer approved these changes

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

Assignees
No one assigned
Labels
3. to review Waiting for reviews
Projects
None yet
Milestone
Nextcloud 14
Development

Successfully merging this pull request may close these issues.

No calendar invitation send for event in shared calendars
3 participants
@georgehrke @rullzer @MorrisJobke