Skip to content

respect privacy options of event/task in activity for shared calendar - #13331

Closed
fdellwing wants to merge 4 commits into
nextcloud:masterfrom
fdellwing:patch-1
Closed

respect privacy options of event/task in activity for shared calendar#13331
fdellwing wants to merge 4 commits into
nextcloud:masterfrom
fdellwing:patch-1

Conversation

@fdellwing

@fdellwingfdellwing commented Jan 3, 2019

Copy link
Copy Markdown

do not show calendar events in activity log if event is not public

Fabian Dellwing
respect privacy options of event
bdb5201
do not show calendar events in activity log if event is not public
Signed-off-by: Fabian Dellwing <f.dellwing@netfutura.de>
Fabian Dellwing
fix regression
d124d72
make sure to only hide calendar events, not todos
show all events in owner activity log
Signed-off-by: Fabian Dellwing <f.dellwing@netfutura.de>
tcitworld
tcitworld reviewed Jan 3, 2019
View reviewed changes

@tcitworldtcitworld left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me, a test inside https://github.com/nextcloud/server/blob/master/apps/dav/tests/unit/CalDAV/Activity/BackendTest.php would be nice.

Comment threadapps/dav/lib/CalDAV/Activity/Backend.php Outdated
Comment threadapps/dav/lib/CalDAV/Activity/Backend.php Outdated
@fdellwing

fdellwing commented Jan 3, 2019

Copy link
Copy Markdown
Author

I'm really not good with tests in PHP, will look into it.

@tcitworld

tcitworld commented Jan 3, 2019

Copy link
Copy Markdown
Member

I'll be happy to try to provide some if you don't feel like it. :)

@rullzer

rullzer commented Jan 4, 2019

Copy link
Copy Markdown
Member

@fdellwing first of all thanks :)

So this is to hide activities for private events in shared calendars right?

@fdellwing

fdellwing commented Jan 4, 2019

Copy link
Copy Markdown
Author

Yes, that is correct.

Fabian Dellwing
also hide private tasks
7fd4bef
fix indentation
Signed-off-by: Fabian Dellwing <f.dellwing@netfutura.de>
@fdellwingfdellwing changed the title respect privacy options of eventrespect privacy options of event/task in activity for shared calendarJan 4, 2019
@fdellwing

fdellwing commented Jan 4, 2019

Copy link
Copy Markdown
Author

I have no clue why the test is failing, maybe unrelated?

@rullzerrullzer added this to the Nextcloud 16 milestone Jan 4, 2019
@rullzerrullzer added the 3. to review Waiting for reviews label Jan 4, 2019
georgehrke
georgehrke reviewed Jan 4, 2019
View reviewed changes
Comment threadapps/dav/lib/CalDAV/Activity/Backend.php Outdated
Fabian Dellwing
skip loop earlier, use const instead of hardcoded number
24eae49
Signed-off-by: Fabian Dellwing <f.dellwing@netfutura.de>
@fdellwing

fdellwing commented Jan 4, 2019•
edited
Loading

Copy link
Copy Markdown
Author

I did add an use statement for the CalDavBackend to not have the classification hardcoded. I hope this is fine for you. It probably also makes the code more readable, as you do not have to guess what 0 stands for.

@nickvergessen

nickvergessen commented Jan 8, 2019

Copy link
Copy Markdown
Member

I think I would still trigger an activity and just leave out the details/name.

@georgehrke

georgehrke commented Jan 8, 2019

Copy link
Copy Markdown
Member

I think I would still trigger an activity and just leave out the details/name.

Maybe for events that are set to "share as show busy only", but not for events set to "hide when sharing". :)

@nickvergessen

nickvergessen commented Jan 8, 2019

Copy link
Copy Markdown
Member

Maybe for events that are set to "share as show busy only", but not for events set to "hide when sharing". :)

Yes, exactly.

@fdellwing thanks for the PR. Do you want to integrate this change? Or shall I take it over?

@fdellwing

fdellwing commented Jan 8, 2019

Copy link
Copy Markdown
Author

I can change it so, that "show busy" generates an activity without details and "private" does not generate an activity at all.

It will have to wait until tomorrow though, I'll knock of work in 10 minutes. If you wanna change it yourself, feel free to take the PR over.

nickvergessen
nickvergessen reviewed Jan 8, 2019
View reviewed changes
Comment threadapps/dav/lib/CalDAV/Activity/Backend.php
$users[] = $owner;

foreach ($users as $user) {
if ($classification !== CalDavBackend::CLASSIFICATION_PUBLIC && $user !== $owner) {

@nickvergessennickvergessenJan 8, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if ($classification === CalDavBackend::CLASSIFICATION_PRIVATE && $user !== $owner) {

@nickvergessen

nickvergessen commented Jan 8, 2019

Copy link
Copy Markdown
Member

Actually let me take this over and add a step to clean it up.

@nickvergessennickvergessen mentioned this pull request Jan 8, 2019
Merged
@fdellwing
fdellwing deleted the patch-1 branch January 8, 2019 18:22
@nickvergessen

nickvergessen commented Jan 9, 2019

Copy link
Copy Markdown
Member

@fdellwing can you send me an email to <my github name>@nextcloud.com please? Thanks 😎

@fdellwing

fdellwing commented Jan 9, 2019

Copy link
Copy Markdown
Author

Sure thing.

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

Reviewers

@nickvergessennickvergessennickvergessen left review comments
@georgehrkegeorgehrkegeorgehrke left review comments
@tcitworldtcitworldtcitworld left review comments
@MorrisJobkeMorrisJobkeAwaiting requested review from MorrisJobke
@rullzerrullzerAwaiting requested review from rullzer

Assignees

No one assigned

Labels

3. to reviewWaiting for reviews

Projects

None yet

Milestone

Nextcloud 16

Development

Successfully merging this pull request may close these issues.

5 participants

@fdellwing@tcitworld@rullzer@nickvergessen@georgehrke