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.

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

allow admins to disable FreeBusy without modifying ShareAPI capabilities #9550

Merged
merged 1 commit into from
Jun 1, 2018

Conversation

georgehrke
Copy link
Member

@georgehrke georgehrke commented May 22, 2018

@MorrisJobke @rullzer As discussed last week

Sorry for not providing a proper testing description in the beginning.
To test this you need an external client like thunderbird, because the Nextcloud Calendar web app does not support Free/Busy yet.

Before setting the property:
before

After executing: php occ config:app:set dav disableFreeBusy --value=yes
after

You can get back to the first behavior by executing: php occ config:app:delete dav disableFreeBusy

@georgehrke georgehrke added this to the Nextcloud 13.0.3 milestone May 22, 2018
@georgehrke georgehrke requested review from rullzer and MorrisJobke May 22, 2018 17:19
@codecov
Copy link

codecov bot commented May 22, 2018

Codecov Report

Merging #9550 into master will decrease coverage by <.01%.
The diff coverage is 52.94%.

@@             Coverage Diff             @@
##             master   #9550      +/-   ##
===========================================
- Coverage      51.7%   51.7%   -0.01%     
- Complexity    25760   25762       +2     
===========================================
  Files          1644    1644              
  Lines         96578   96589      +11     
  Branches       1394    1394              
===========================================
+ Hits          49935   49939       +4     
- Misses        46643   46650       +7
Impacted Files Coverage Δ Complexity Δ
apps/dav/appinfo/v1/carddav.php 0% <0%> (ø) 0 <0> (ø) ⬇️
apps/dav/appinfo/v1/caldav.php 0% <0%> (ø) 0 <0> (ø) ⬇️
apps/dav/lib/Command/CreateCalendar.php 45.16% <0%> (-1.51%) 4 <0> (ø)
apps/files_trashbin/lib/AppInfo/Application.php 64.7% <0%> (-4.05%) 3 <0> (ø)
apps/files_versions/lib/AppInfo/Application.php 50% <0%> (-4.55%) 2 <0> (ø)
apps/dav/lib/Connector/Sabre/Principal.php 90.43% <100%> (+0.43%) 49 <0> (+2) ⬆️
apps/dav/lib/RootCollection.php 97.33% <100%> (+0.03%) 1 <0> (ø) ⬇️
apps/files_trashbin/lib/Expiration.php 90.32% <0%> (-1.62%) 29% <0%> (ø)
lib/private/Files/Cache/Propagator.php 94.93% <0%> (-1.27%) 16% <0%> (ø)

This was referenced May 31, 2018
if (!$this->shareManager->shareApiEnabled()) {
// If sharing is disabled (or FreeBusy was disabled on purpose), return the empty array
$shareAPIEnabled = $this->shareManager->shareApiEnabled();
$disableFreeBusy = $this->config->getAppValue('dav', 'disableFreeBusy', $shareAPIEnabled ? 'no' : 'yes');
Copy link
Member

Choose a reason for hiding this comment

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

mmm I'm not sure this is the right place.

Basically not being able to find principals if sharing is totally disabled makes sense. But if free busy is disabled this will kill all sharing.

Can't we extend this class for calendars? overrwite only this function and then do the extra check? (as for now it is only needed for calendars right?).

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Ok then fine by me. If we ever want to extend this we have to the magic then!

@MorrisJobke
Copy link
Member

Once I set the value to yes via php occ config:app:set dav disableFreeBusy --value=yes it does not change the entries of a shared calendar.

I have two users, A shares a calendar to B. User A adds 3 events: one shown, one shown as busy and one not shown to the share. For user B in both cases (with and without the setting) the entry marked to be shown as "Busy" is shown :/

@MorrisJobke
Copy link
Member

But also putting a debug breakpoint to the two code parts inside the Principal does not get triggered :/

@georgehrke
Copy link
Member Author

@MorrisJobke I updated the description.

@georgehrke
Copy link
Member Author

Backport to stable13: #9707

Backport to stable12 is not necessary, because Free/Busy was only introduced with stable13

@MorrisJobke MorrisJobke merged commit c3fc789 into master Jun 1, 2018
@MorrisJobke MorrisJobke deleted the bugfix/noid/override_freebusy_sharing_rules branch June 1, 2018 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants