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

fix(dav): Move DAV app to non deprecated event dispatcher #39190

Merged

Conversation

nickvergessen
Copy link
Member

One step closer for #38546

Checklist

@nickvergessen nickvergessen added bug 3. to review Waiting for reviews feature: caldav Related to CalDAV internals feature: carddav Related to CardDAV internals labels Jul 6, 2023
@nickvergessen nickvergessen added this to the Nextcloud 28 milestone Jul 6, 2023
@nickvergessen nickvergessen self-assigned this Jul 6, 2023
$user = $event->getSubject();
/** @var SyncService $syncService */
$syncService = $container->query(SyncService::class);
$syncService->updateUser($user);
});


$dispatcher->addListener('\OCA\DAV\CalDAV\CalDavBackend::updateShares', function (GenericEvent $event) use ($container) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines -262 to -263
$dispatcher->addListener('\OCP\Calendar\Resource\ForceRefreshEvent', $eventHandler);
$dispatcher->addListener('\OCP\Calendar\Room\ForceRefreshEvent', $eventHandler);
Copy link
Member Author

Choose a reason for hiding this comment

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

I could not find ForceRefreshEvent as a search term in our github org, apart from this occurance here. I assume there were plans that changed on the way, so I think this is dead code and I just remove it.

Copy link
Member

Choose a reason for hiding this comment

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

Also in #35388 :)

IAppContainer $container,
IServerContainer $serverContainer) {
IEventDispatcher $dispatcher,
IAppContainer $container) {

Check notice

Code scanning / Psalm

DeprecatedInterface Note

Interface OCP\AppFramework\IAppContainer is marked as deprecated
@ChristophWurst ChristophWurst changed the title Bugfix/noid/move dav app to non deprecated dispatcher fix(dav): Move DAV app to non deprecated event dispatcher Jul 6, 2023
@@ -227,40 +224,26 @@ public function registerHooks(HookManager $hm,
}
});

$dispatcher->addListener('OC\AccountManager::userUpdated', function (GenericEvent $event) use ($container) {
$dispatcher->addListener('OC\AccountManager::userUpdated', function ($event) use ($container) {
Copy link
Member

Choose a reason for hiding this comment

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

what type is the event now? can it be hinted?

Copy link
Member Author

Choose a reason for hiding this comment

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

Still GenericEvent, but then Psalm complains because the event listener does not consume IEvent. Removing silences psalm and makes it work. The actual fix again is creating and only listening to typed events, but time....

Signed-off-by: Joas Schilling <coding@schilljs.com>
The legacy event was removed in 4bb31c0

Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen force-pushed the bugfix/noid/move-dav-app-to-non-deprecated-dispatcher branch from a4939c0 to e1d4b82 Compare July 7, 2023 08:55
$syncService = $container->query(SyncService::class);
$syncService->updateUser($user);
$dispatcher->addListener('OC\AccountManager::userUpdated', function ($event) use ($container) {
if ($event instanceof GenericEvent) {

Check notice

Code scanning / Psalm

DocblockTypeContradiction Note

Cannot resolve types for $event - docblock-defined type OCP\EventDispatcher\Event does not contain Symfony\Component\EventDispatcher\GenericEvent
if ($event instanceof GenericEvent) {
$user = $event->getSubject();
/** @var SyncService $syncService */
$syncService = $container->query(SyncService::class);

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IContainer::query has been marked as deprecated
@skjnldsv skjnldsv merged commit 2dfd9ee into master Jul 11, 2023
37 checks passed
@skjnldsv skjnldsv deleted the bugfix/noid/move-dav-app-to-non-deprecated-dispatcher branch July 11, 2023 06:27
@nickvergessen nickvergessen added pending documentation This pull request needs an associated documentation update and removed pending documentation This pull request needs an associated documentation update labels Jul 27, 2023
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 bug feature: caldav Related to CalDAV internals feature: carddav Related to CardDAV internals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants