From 0af497dde024531a148dd348e68b258f281d9507 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 29 Jul 2024 15:12:34 +0200 Subject: [PATCH 1/5] test: Add integration tests for call notifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [skip-ci] Signed-off-by: Daniel Calviño Sánchez --- .../features/federation/call.feature | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/tests/integration/features/federation/call.feature b/tests/integration/features/federation/call.feature index 620a76c334b..8adf8650519 100644 --- a/tests/integration/features/federation/call.feature +++ b/tests/integration/features/federation/call.feature @@ -139,3 +139,117 @@ Feature: federation/call | actorType | actorId | inCall | | federated_users | participant1@{$LOCAL_URL} | 0 | | users | participant2 | 0 | + + + + Scenario: normal call notification for federated user + Given user "participant1" creates room "room" (v4) + | roomType | 2 | + | roomName | room | + And user "participant1" adds federated_user "participant2@REMOTE" to room "room" with 200 (v4) + And using server "REMOTE" + And user "participant2" has the following invitations (v1) + | remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName | + | LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname | + And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1) + | id | name | type | remoteServer | remoteToken | + | LOCAL::room | room | 2 | LOCAL | room | + And user "participant2" joins room "LOCAL::room" with 200 (v4) + And using server "LOCAL" + And user "participant1" joins room "room" with 200 (v4) + When user "participant1" joins call "room" with 200 (v4) + Then using server "REMOTE" + And user "participant2" has the following notifications + | app | object_type | object_id | subject | + | spreed | call | room | A group call has started in room | + + Scenario: normal call notification for federated user is cleared when joining + Given user "participant1" creates room "room" (v4) + | roomType | 2 | + | roomName | room | + And user "participant1" adds federated_user "participant2@REMOTE" to room "room" with 200 (v4) + And using server "REMOTE" + And user "participant2" has the following invitations (v1) + | remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName | + | LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname | + And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1) + | id | name | type | remoteServer | remoteToken | + | LOCAL::room | room | 2 | LOCAL | room | + And user "participant2" joins room "LOCAL::room" with 200 (v4) + And using server "LOCAL" + And user "participant1" joins room "room" with 200 (v4) + When user "participant1" joins call "room" with 200 (v4) + And using server "REMOTE" + And user "participant2" has the following notifications + | app | object_type | object_id | subject | + | spreed | call | room | A group call has started in room | + When user "participant2" joins call "room" with 200 (v4) + Then user "participant2" has the following notifications + | app | object_type | object_id | subject | + + Scenario: missed call notification for federated user + Given user "participant1" creates room "room" (v4) + | roomType | 2 | + | roomName | room | + And user "participant1" adds federated_user "participant2@REMOTE" to room "room" with 200 (v4) + And using server "REMOTE" + And user "participant2" has the following invitations (v1) + | remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName | + | LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname | + And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1) + | id | name | type | remoteServer | remoteToken | + | LOCAL::room | room | 2 | LOCAL | room | + And user "participant2" joins room "LOCAL::room" with 200 (v4) + And using server "LOCAL" + And user "participant1" joins room "room" with 200 (v4) + And user "participant1" joins call "room" with 200 (v4) + When user "participant1" leaves call "room" with 200 (v4) + Then using server "REMOTE" + And user "participant2" has the following notifications + | app | object_type | object_id | subject | + | spreed | call | room | You missed a group call in room | + + Scenario: silent call by federated user does not trigger call notification + Given user "participant1" creates room "room" (v4) + | roomType | 2 | + | roomName | room | + And user "participant1" adds federated_user "participant2@REMOTE" to room "room" with 200 (v4) + And using server "REMOTE" + And user "participant2" has the following invitations (v1) + | remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName | + | LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname | + And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1) + | id | name | type | remoteServer | remoteToken | + | LOCAL::room | room | 2 | LOCAL | room | + And using server "LOCAL" + And user "participant2" joins room "LOCAL::room" with 200 (v4) + And using server "REMOTE" + And user "participant2" joins room "LOCAL::room" with 200 (v4) + When user "participant2" joins call "LOCAL::room" with 200 (v4) + | silent | true | + Then using server "LOCAL" + And user "participant1" has the following notifications + | app | object_type | object_id | subject | + + Scenario: silent call by federated user does not trigger call notification + Given user "participant1" creates room "room" (v4) + | roomType | 2 | + | roomName | room | + And user "participant1" adds federated_user "participant2@REMOTE" to room "room" with 200 (v4) + And using server "REMOTE" + And user "participant2" has the following invitations (v1) + | remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName | + | LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname | + And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1) + | id | name | type | remoteServer | remoteToken | + | LOCAL::room | room | 2 | LOCAL | room | + And using server "LOCAL" + And user "participant2" joins room "LOCAL::room" with 200 (v4) + And using server "REMOTE" + And user "participant2" joins room "LOCAL::room" with 200 (v4) + And user "participant2" joins call "LOCAL::room" with 200 (v4) + | silent | true | + When user "participant2" leaves call "LOCAL::room" with 200 (v4) + Then using server "LOCAL" + And user "participant1" has the following notifications + | app | object_type | object_id | subject | From 432cf58d8d41f6c310b5ac2705d0aea9cd7f7661 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Jul 2024 16:58:22 +0200 Subject: [PATCH 2/5] fix(event): Make info available whether this request changed the active-since Signed-off-by: Joas Schilling --- lib/Events/ActiveSinceModifiedEvent.php | 22 ++++++++++++++++++++++ lib/Service/RoomService.php | 8 ++++---- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/lib/Events/ActiveSinceModifiedEvent.php b/lib/Events/ActiveSinceModifiedEvent.php index e250899dee5..f69f89a71cd 100644 --- a/lib/Events/ActiveSinceModifiedEvent.php +++ b/lib/Events/ActiveSinceModifiedEvent.php @@ -8,5 +8,27 @@ namespace OCA\Talk\Events; +use OCA\Talk\Room; + class ActiveSinceModifiedEvent extends AActiveSinceModifiedEvent { + public function __construct( + Room $room, + ?\DateTime $newValue, + ?\DateTime $oldValue, + int $callFlag, + int $oldCallFlag, + protected bool $updatedActiveSince, + ) { + parent::__construct( + $room, + $newValue, + $oldValue, + $callFlag, + $oldCallFlag, + ); + } + + public function hasUpdatedActiveSince(): bool { + return $this->updatedActiveSince; + } } diff --git a/lib/Service/RoomService.php b/lib/Service/RoomService.php index eec1459826f..33a83320b73 100644 --- a/lib/Service/RoomService.php +++ b/lib/Service/RoomService.php @@ -856,7 +856,7 @@ public function resetActiveSince(Room $room): bool { $result = (bool) $update->executeStatement(); - $event = new ActiveSinceModifiedEvent($room, null, $oldActiveSince, Participant::FLAG_DISCONNECTED, $oldCallFlag); + $event = new ActiveSinceModifiedEvent($room, null, $oldActiveSince, Participant::FLAG_DISCONNECTED, $oldCallFlag, $result); $this->dispatcher->dispatchTyped($event); return $result; @@ -901,14 +901,14 @@ public function setActiveSince(Room $room, \DateTime $since, int $callFlag): boo ->set('active_since', $update->createNamedParameter($since, IQueryBuilder::PARAM_DATE)) ->where($update->expr()->eq('id', $update->createNamedParameter($room->getId(), IQueryBuilder::PARAM_INT))) ->andWhere($update->expr()->isNull('active_since')); - $update->executeStatement(); + $result = (bool) $update->executeStatement(); $room->setActiveSince($since, $callFlag); - $event = new ActiveSinceModifiedEvent($room, $since, $oldActiveSince, $callFlag, $oldCallFlag); + $event = new ActiveSinceModifiedEvent($room, $since, $oldActiveSince, $callFlag, $oldCallFlag, $result); $this->dispatcher->dispatchTyped($event); - return true; + return $result; } public function setLastMessage(Room $room, IComment $message): void { From 099db18c8e6954ec593950d707602cf2d454e969 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Jul 2024 16:59:38 +0200 Subject: [PATCH 3/5] fix(federation): Add federated call notifications Signed-off-by: Joas Schilling --- lib/AppInfo/Application.php | 1 + lib/Notification/Listener.php | 14 ++++++++++++++ lib/Notification/Notifier.php | 5 ++++- 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 444b064080a..2746aba0a5a 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -246,6 +246,7 @@ public function register(IRegistrationContext $context): void { // Notification listeners $context->registerEventListener(AttendeesAddedEvent::class, NotificationListener::class); + $context->registerEventListener(ActiveSinceModifiedEvent::class, NotificationListener::class); $context->registerEventListener(BeforeParticipantModifiedEvent::class, NotificationListener::class); $context->registerEventListener(CallNotificationSendEvent::class, NotificationListener::class); $context->registerEventListener(ParticipantModifiedEvent::class, NotificationListener::class); diff --git a/lib/Notification/Listener.php b/lib/Notification/Listener.php index 674e4cafae0..ebf75cd24cd 100644 --- a/lib/Notification/Listener.php +++ b/lib/Notification/Listener.php @@ -10,6 +10,7 @@ use OCA\Talk\AppInfo\Application; use OCA\Talk\Controller\ChatController; +use OCA\Talk\Events\ActiveSinceModifiedEvent; use OCA\Talk\Events\AParticipantModifiedEvent; use OCA\Talk\Events\AttendeesAddedEvent; use OCA\Talk\Events\BeforeParticipantModifiedEvent; @@ -61,6 +62,7 @@ public function handle(Event $event): void { UserJoinedRoomEvent::class => $this->handleUserJoinedRoomEvent($event), BeforeParticipantModifiedEvent::class => $this->checkCallNotifications($event), ParticipantModifiedEvent::class => $this->afterParticipantJoinedCall($event), + ActiveSinceModifiedEvent::class => $this->afterActiveSinceModified($event), }; } @@ -222,6 +224,18 @@ protected function afterParticipantJoinedCall(ParticipantModifiedEvent $event): } } + protected function afterActiveSinceModified(ActiveSinceModifiedEvent $event): void { + if (!$event->hasUpdatedActiveSince()) { + return; + } + + if (!$event->getRoom()->isFederatedConversation()) { + return; + } + + $this->sendCallNotifications($event->getRoom()); + } + /** * Call notification: "{user} wants to talk with you" * diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index a6f5a25f355..7ffd454d81e 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -1032,7 +1032,10 @@ protected function parseCall(INotification $notification, Room $room, IL10N $l): throw new AlreadyProcessedException(); } } elseif (\in_array($room->getType(), [Room::TYPE_GROUP, Room::TYPE_PUBLIC], true)) { - if ($this->notificationManager->isPreparingPushNotification() || $this->participantService->hasActiveSessionsInCall($room)) { + if ($this->notificationManager->isPreparingPushNotification() + || (!$room->isFederatedConversation() && $this->participantService->hasActiveSessionsInCall($room)) + || ($room->isFederatedConversation() && $room->getActiveSince()) + ) { $notification = $this->addActionButton($notification, 'call_view', $l->t('Join call'), true, true); $subject = $l->t('A group call has started in {call}'); } else { From 413641ddc9e3d8c6376f47d7533d591b04675c2f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Jul 2024 17:00:04 +0200 Subject: [PATCH 4/5] fix(calls): Also track the participant in_call locally to be able to handle notifications Signed-off-by: Joas Schilling --- lib/Controller/CallController.php | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/lib/Controller/CallController.php b/lib/Controller/CallController.php index d6c8a108dec..0977dd4ba9e 100644 --- a/lib/Controller/CallController.php +++ b/lib/Controller/CallController.php @@ -150,7 +150,13 @@ public function joinCall(?int $flags = null, ?int $forcePermissions = null, bool if ($this->room->isFederatedConversation()) { /** @var \OCA\Talk\Federation\Proxy\TalkV1\Controller\CallController $proxy */ $proxy = \OCP\Server::get(\OCA\Talk\Federation\Proxy\TalkV1\Controller\CallController::class); - return $proxy->joinFederatedCall($this->room, $this->participant, $flags, $silent, $recordingConsent); + $response = $proxy->joinFederatedCall($this->room, $this->participant, $flags, $silent, $recordingConsent); + + if ($response->getStatus() === Http::STATUS_OK) { + $this->participantService->changeInCall($this->room, $this->participant, $flags, false, $silent); + } + + return $response; } if ($forcePermissions !== null && $this->participant->hasModeratorPermissions()) { @@ -325,7 +331,13 @@ public function updateCallFlags(int $flags): DataResponse { if ($this->room->isFederatedConversation()) { /** @var \OCA\Talk\Federation\Proxy\TalkV1\Controller\CallController $proxy */ $proxy = \OCP\Server::get(\OCA\Talk\Federation\Proxy\TalkV1\Controller\CallController::class); - return $proxy->updateFederatedCallFlags($this->room, $this->participant, $flags); + $response = $proxy->updateFederatedCallFlags($this->room, $this->participant, $flags); + + if ($response->getStatus() === Http::STATUS_OK) { + $this->participantService->updateCallFlags($this->room, $this->participant, $flags); + } + + return $response; } try { @@ -391,7 +403,13 @@ public function leaveCall(bool $all = false): DataResponse { if ($this->room->isFederatedConversation()) { /** @var \OCA\Talk\Federation\Proxy\TalkV1\Controller\CallController $proxy */ $proxy = \OCP\Server::get(\OCA\Talk\Federation\Proxy\TalkV1\Controller\CallController::class); - return $proxy->leaveFederatedCall($this->room, $this->participant); + $response = $proxy->leaveFederatedCall($this->room, $this->participant); + + if ($response->getStatus() === Http::STATUS_OK) { + $this->participantService->changeInCall($this->room, $this->participant, Participant::FLAG_DISCONNECTED); + } + + return $response; } if ($all && $this->participant->hasModeratorPermissions()) { From eb345e7732544edfead99fb83e9a73f770b5e9d5 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Jul 2024 18:05:46 +0200 Subject: [PATCH 5/5] fix(call): Fix integration test mix up of local and remote rooms Signed-off-by: Joas Schilling --- .../features/federation/call.feature | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/tests/integration/features/federation/call.feature b/tests/integration/features/federation/call.feature index 8adf8650519..e3b16b8f2ac 100644 --- a/tests/integration/features/federation/call.feature +++ b/tests/integration/features/federation/call.feature @@ -140,8 +140,6 @@ Feature: federation/call | federated_users | participant1@{$LOCAL_URL} | 0 | | users | participant2 | 0 | - - Scenario: normal call notification for federated user Given user "participant1" creates room "room" (v4) | roomType | 2 | @@ -160,8 +158,8 @@ Feature: federation/call When user "participant1" joins call "room" with 200 (v4) Then using server "REMOTE" And user "participant2" has the following notifications - | app | object_type | object_id | subject | - | spreed | call | room | A group call has started in room | + | app | object_type | object_id | subject | + | spreed | call | LOCAL::room | A group call has started in room | Scenario: normal call notification for federated user is cleared when joining Given user "participant1" creates room "room" (v4) @@ -181,9 +179,9 @@ Feature: federation/call When user "participant1" joins call "room" with 200 (v4) And using server "REMOTE" And user "participant2" has the following notifications - | app | object_type | object_id | subject | - | spreed | call | room | A group call has started in room | - When user "participant2" joins call "room" with 200 (v4) + | app | object_type | object_id | subject | + | spreed | call | LOCAL::room | A group call has started in room | + When user "participant2" joins call "LOCAL::room" with 200 (v4) Then user "participant2" has the following notifications | app | object_type | object_id | subject | @@ -206,8 +204,8 @@ Feature: federation/call When user "participant1" leaves call "room" with 200 (v4) Then using server "REMOTE" And user "participant2" has the following notifications - | app | object_type | object_id | subject | - | spreed | call | room | You missed a group call in room | + | app | object_type | object_id | subject | + | spreed | call | LOCAL::room | You missed a group call in room | Scenario: silent call by federated user does not trigger call notification Given user "participant1" creates room "room" (v4) @@ -222,7 +220,7 @@ Feature: federation/call | id | name | type | remoteServer | remoteToken | | LOCAL::room | room | 2 | LOCAL | room | And using server "LOCAL" - And user "participant2" joins room "LOCAL::room" with 200 (v4) + And user "participant1" joins room "room" with 200 (v4) And using server "REMOTE" And user "participant2" joins room "LOCAL::room" with 200 (v4) When user "participant2" joins call "LOCAL::room" with 200 (v4) @@ -231,7 +229,7 @@ Feature: federation/call And user "participant1" has the following notifications | app | object_type | object_id | subject | - Scenario: silent call by federated user does not trigger call notification + Scenario: missed silent call by federated user does not trigger call notification Given user "participant1" creates room "room" (v4) | roomType | 2 | | roomName | room | @@ -244,7 +242,7 @@ Feature: federation/call | id | name | type | remoteServer | remoteToken | | LOCAL::room | room | 2 | LOCAL | room | And using server "LOCAL" - And user "participant2" joins room "LOCAL::room" with 200 (v4) + And user "participant1" joins room "room" with 200 (v4) And using server "REMOTE" And user "participant2" joins room "LOCAL::room" with 200 (v4) And user "participant2" joins call "LOCAL::room" with 200 (v4)