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

Extract caldav sharing stuff from publishing stuff #34873

Closed
wants to merge 3 commits into from

Conversation

tcitworld
Copy link
Member

Based on and replaces #34372 by @pboguslawski

The publishing plugin handled both doing the actual publishing/unpublishing as well as exposing the supported sharing features.

Now we have one for each use and publishing can be properly disabled. The CalDAVSharingPlugin will probably be useful for most of the issues in #20096 anyway.

* @var IConfig
*/
protected $config;
protected Server $server;

Check notice

Code scanning / Psalm

PropertyNotSetInConstructor

Property OCA\DAV\CalDAV\Publishing\PublishPlugin::$server is not defined in constructor of OCA\DAV\CalDAV\Publishing\PublishPlugin or in any methods called in the constructor
class SharingPlugin extends ServerPlugin {
public const NS_CALENDARSERVER = 'http://calendarserver.org/ns/';

protected Server $server;

Check notice

Code scanning / Psalm

PropertyNotSetInConstructor

Property OCA\DAV\CalDAV\SharingPlugin::$server is not defined in constructor of OCA\DAV\CalDAV\SharingPlugin or in any methods called in the constructor
*
* @param Server $server
*/
public function initialize(Server $server) {

Check notice

Code scanning / Psalm

MissingReturnType

Method OCA\DAV\CalDAV\SharingPlugin::initialize does not have a return type, expecting void
$this->server->on('propFind', [$this, 'propFind']);
}

public function propFind(PropFind $propFind, INode $node) {

Check notice

Code scanning / Psalm

MissingReturnType

Method OCA\DAV\CalDAV\SharingPlugin::propFind does not have a return type, expecting void
@pboguslawski
Copy link
Contributor

Didn't notice any problems after applying this patch on 24.0.5. Thank you!

Copy link
Contributor

@miaulalala miaulalala left a comment

Choose a reason for hiding this comment

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

  • no value set - public share links work and is an option in calendar
  • value set to yes - public share link works and is an option in calendar
  • value set to no - public share link does not work and there is not an option to create a public share link in calendar but sharing with users and groups still works

apps/dav/composer/autoload.php Outdated Show resolved Hide resolved
pboguslawski and others added 3 commits January 20, 2023 15:10
This mod disallows sharing calendars via link when `shareapi_allow_links`
is disabled.

Related: nextcloud/calendar#525
Related: nextcloud/calendar#4399
Author-Change-Id: IB#1126264
Signed-off-by: Pawel Boguslawski <pawel.boguslawski@ib.pl>
Fixes: 45eefc2
Related: #34372
Author-Change-Id: IB#1126264
Signed-off-by: Pawel Boguslawski <pawel.boguslawski@ib.pl>
So that it's still there when we disable the PublishPlugin

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
@tcitworld tcitworld force-pushed the extract-caldav-sharing-plugin branch from 4fd3cba to 7f87ac8 Compare January 20, 2023 14:12
@ChristophWurst
Copy link
Member

Now we have one for each use and publishing can be properly disabled. The CalDAVSharingPlugin will probably be useful for most of the issues in #20096 anyway.

🙌 🙌

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Should we add tests?

@tcitworld
Copy link
Member Author

tcitworld commented Jan 20, 2023

Due to the callbacks, Sabre patterns are hard to test, there would be low value for added tests here. Not sure if it's impossible, but it's never been done before in all other plugins.

@pboguslawski
Copy link
Contributor

pboguslawski commented Jan 30, 2023

After upgrading to server 25.0.3 with this mod applied + calendar 4.2.1, when shareapi_allow_links is disabled...

# /usr/bin/php /var/www/nextcloud/occ config:app:set core shareapi_allow_links --value='no'
Config value shareapi_allow_links for app core set to no

# /usr/bin/php /var/www/nextcloud/occ config:app:get core shareapi_allow_links
no

...then Edit calendar dialog still contains Share link with + but clicking + displays popup with An error occurred, unable to publish calendar. (POST with 501 Not Implemented in browser dev tools).

After enabling...

# /usr/bin/php /var/www/nextcloud/occ config:app:set core shareapi_allow_links --value='yes'
Config value shareapi_allow_links for app core set to yes

# /usr/bin/php /var/www/nextcloud/occ config:app:get core shareapi_allow_links
yes

...clicking + works ok.

Seems that server blocks sharing via link correctly but calendar app after upgrade should have Share link removed in Edit calendar when shareapi_allow_links is disabled.

@blizzz blizzz mentioned this pull request Feb 1, 2023
@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
@blizzz blizzz mentioned this pull request Mar 7, 2023
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
@szaimen
Copy link
Contributor

szaimen commented Apr 17, 2023

@tcitworld can you please fix the conflicts and the tests? Thanks in advance! :)

@szaimen szaimen added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Apr 17, 2023
This was referenced May 3, 2023
@blizzz blizzz mentioned this pull request May 17, 2023
@blizzz blizzz modified the milestones: Nextcloud 27, Nextcloud 28 May 23, 2023
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
pboguslawski added a commit to ibpl/calendar that referenced this pull request Jan 10, 2024
Related: nextcloud/server#34873
Author-Change-Id: IB#1126264
Signed-off-by: Pawel Boguslawski <pawel.boguslawski@ib.pl>
@pboguslawski
Copy link
Contributor

nextcloud/calendar#5679 fixes Share link option problem described above for us (tested with this PR merged also).

This was referenced Mar 12, 2024
This was referenced Mar 20, 2024
@skjnldsv skjnldsv mentioned this pull request Mar 28, 2024
81 tasks
@skjnldsv skjnldsv modified the milestones: Nextcloud 29, Nextcloud 30 Mar 28, 2024
This was referenced Jul 30, 2024
This was referenced Aug 5, 2024
@skjnldsv skjnldsv mentioned this pull request Aug 13, 2024
@skjnldsv skjnldsv closed this Aug 14, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 30 milestone Aug 14, 2024
@skjnldsv skjnldsv deleted the extract-caldav-sharing-plugin branch August 30, 2024 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress feature: caldav Related to CalDAV internals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants