From 950a3f8d370330cdd9f1be0c4c5d4b1f1423dbed Mon Sep 17 00:00:00 2001 From: Hamza Date: Sun, 26 Apr 2026 16:58:21 +0200 Subject: [PATCH 1/3] feat(ocp): expose whether talk is enabled for user Signed-off-by: Hamza --- lib/private/Talk/Broker.php | 16 ++++++++++++++++ lib/public/Talk/IBroker.php | 24 ++++++++++++++++++++++++ lib/public/Talk/ITalkBackend.php | 24 ++++++++++++++++++++++++ 3 files changed, 64 insertions(+) diff --git a/lib/private/Talk/Broker.php b/lib/private/Talk/Broker.php index eaf5fad96c68e..9bae84ea68446 100644 --- a/lib/private/Talk/Broker.php +++ b/lib/private/Talk/Broker.php @@ -91,4 +91,20 @@ public function deleteConversation(string $id): void { $this->backend->deleteConversation($id); } + + public function isDisabledForUser(): bool { + if (!$this->hasBackend()) { + return true; + } + + return $this->backend->isDisabledForUser(); + } + + public function isNotAllowedToCreateConversations(): bool { + if (!$this->hasBackend()) { + return true; + } + + return $this->backend->isNotAllowedToCreateConversations(); + } } diff --git a/lib/public/Talk/IBroker.php b/lib/public/Talk/IBroker.php index cfea4af687372..be866ea5f4deb 100644 --- a/lib/public/Talk/IBroker.php +++ b/lib/public/Talk/IBroker.php @@ -65,4 +65,28 @@ public function createConversation(string $name, * @since 26.0.0 */ public function deleteConversation(string $id): void; + + /** + * Check if the logged in user is not allowed to create conversations, + * respecting the per-group restrictions configured in Talk admin settings + * (allowed_groups). + * + * Returns false when Talk is not available at all. + * + * @return bool + * @since 34.0.0 + */ + public function isNotAllowedToCreateConversations(): bool; + + /** + * Check if Talk is disabled for the logged in user, respecting the + * per-group restriction configured in Talk admin settings + * (start_conversations). + * + * Returns false when Talk is not available at all. + * + * @return bool + * @since 34.0.0 + */ + public function isDisabledForUser(): bool; } diff --git a/lib/public/Talk/ITalkBackend.php b/lib/public/Talk/ITalkBackend.php index 66dbd3c485dd2..e8faa37822300 100644 --- a/lib/public/Talk/ITalkBackend.php +++ b/lib/public/Talk/ITalkBackend.php @@ -42,4 +42,28 @@ public function createConversation(string $name, * @since 26.0.0 */ public function deleteConversation(string $id): void; + + /** + * Check if the logged in user is not allowed to create conversations, + * respecting the per-group restrictions configured in Talk admin settings + * (allowed_groups). + * + * Returns false when Talk is not available at all. + * + * @return bool + * @since 34.0.0 + */ + public function isNotAllowedToCreateConversations(): bool; + + /** + * Check if Talk is disabled for the logged in user, respecting the + * per-group restriction configured in Talk admin settings + * (start_conversations). + * + * Returns false when Talk is not available at all. + * + * @return bool + * @since 34.0.0 + */ + public function isDisabledForUser(): bool; } From c55cfc4367763e8375bf773b0ec8c4ad77e6d057 Mon Sep 17 00:00:00 2001 From: Hamza Date: Sun, 26 Apr 2026 17:08:39 +0200 Subject: [PATCH 2/3] test: Add tests for whether spreed is enabled Signed-off-by: Hamza --- tests/lib/Talk/BrokerTest.php | 88 +++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/tests/lib/Talk/BrokerTest.php b/tests/lib/Talk/BrokerTest.php index dc901047b92fa..579ece658e03e 100644 --- a/tests/lib/Talk/BrokerTest.php +++ b/tests/lib/Talk/BrokerTest.php @@ -132,4 +132,92 @@ public function testCreateConversation(): void { $options ); } + + public function testIsDisabledForUserNoBackend(): void { + $this->coordinator->expects(self::once()) + ->method('getRegistrationContext') + ->willReturn($this->createMock(RegistrationContext::class)); + + self::assertTrue( + $this->broker->isDisabledForUser() + ); + } + + public static function dataIsDisabledForUser(): array { + return [ + [true], + [false], + ]; + } + + /** + * @dataProvider dataIsDisabledForUser + */ + public function testIsDisabledForUser(bool $disabled): void { + $fakeTalkServiceClass = '\\OCA\\Spreed\\TalkBackend'; + $registrationContext = $this->createMock(RegistrationContext::class); + $this->coordinator->expects(self::once()) + ->method('getRegistrationContext') + ->willReturn($registrationContext); + $registrationContext->expects(self::once()) + ->method('getTalkBackendRegistration') + ->willReturn(new ServiceRegistration('spreed', $fakeTalkServiceClass)); + $talkService = $this->createMock(ITalkBackend::class); + $this->container->expects(self::once()) + ->method('get') + ->with($fakeTalkServiceClass) + ->willReturn($talkService); + $talkService->expects(self::once()) + ->method('isDisabledForUser') + ->willReturn($disabled); + + self::assertSame( + $disabled, + $this->broker->isDisabledForUser() + ); + } + + public function testIsNotAllowedToCreateConversationsNoBackend(): void { + $this->coordinator->expects(self::once()) + ->method('getRegistrationContext') + ->willReturn($this->createMock(RegistrationContext::class)); + + self::assertTrue( + $this->broker->isNotAllowedToCreateConversations() + ); + } + + public static function dataIsNotAllowedToCreateConversations(): array { + return [ + [true], + [false], + ]; + } + + /** + * @dataProvider dataIsNotAllowedToCreateConversations + */ + public function testIsNotAllowedToCreateConversations(bool $notAllowed): void { + $fakeTalkServiceClass = '\\OCA\\Spreed\\TalkBackend'; + $registrationContext = $this->createMock(RegistrationContext::class); + $this->coordinator->expects(self::once()) + ->method('getRegistrationContext') + ->willReturn($registrationContext); + $registrationContext->expects(self::once()) + ->method('getTalkBackendRegistration') + ->willReturn(new ServiceRegistration('spreed', $fakeTalkServiceClass)); + $talkService = $this->createMock(ITalkBackend::class); + $this->container->expects(self::once()) + ->method('get') + ->with($fakeTalkServiceClass) + ->willReturn($talkService); + $talkService->expects(self::once()) + ->method('isNotAllowedToCreateConversations') + ->willReturn($notAllowed); + + self::assertSame( + $notAllowed, + $this->broker->isNotAllowedToCreateConversations() + ); + } } From 109eb0c8a91e35ecc070573b0fbfc71d0d000147 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 27 Apr 2026 21:19:48 +0200 Subject: [PATCH 3/3] fix(talkbackend): Make function names positive and remove talk internals from docs Signed-off-by: Joas Schilling --- lib/private/Talk/Broker.php | 14 ++-- lib/public/Talk/IBroker.php | 16 ++--- lib/public/Talk/ITalkBackend.php | 16 ++--- tests/lib/Talk/BrokerTest.php | 117 +++++++++++++++++++------------ 4 files changed, 89 insertions(+), 74 deletions(-) diff --git a/lib/private/Talk/Broker.php b/lib/private/Talk/Broker.php index 9bae84ea68446..114747b6fbf74 100644 --- a/lib/private/Talk/Broker.php +++ b/lib/private/Talk/Broker.php @@ -92,19 +92,19 @@ public function deleteConversation(string $id): void { $this->backend->deleteConversation($id); } - public function isDisabledForUser(): bool { - if (!$this->hasBackend()) { - return true; + public function isAllowedToCreateConversations(): bool { + if (!$this->isEnabledForUser()) { + return false; } - return $this->backend->isDisabledForUser(); + return $this->backend->isAllowedToCreateConversations(); } - public function isNotAllowedToCreateConversations(): bool { + public function isEnabledForUser(): bool { if (!$this->hasBackend()) { - return true; + return false; } - return $this->backend->isNotAllowedToCreateConversations(); + return $this->backend->isEnabledForUser(); } } diff --git a/lib/public/Talk/IBroker.php b/lib/public/Talk/IBroker.php index be866ea5f4deb..11259e53e33c3 100644 --- a/lib/public/Talk/IBroker.php +++ b/lib/public/Talk/IBroker.php @@ -67,26 +67,22 @@ public function createConversation(string $name, public function deleteConversation(string $id): void; /** - * Check if the logged in user is not allowed to create conversations, - * respecting the per-group restrictions configured in Talk admin settings - * (allowed_groups). + * Check if the logged-in user is allowed to create conversations * - * Returns false when Talk is not available at all. + * Also returns false when no backend is available * * @return bool * @since 34.0.0 */ - public function isNotAllowedToCreateConversations(): bool; + public function isAllowedToCreateConversations(): bool; /** - * Check if Talk is disabled for the logged in user, respecting the - * per-group restriction configured in Talk admin settings - * (start_conversations). + * Check if the Talk backend is enabled for the logged-in user * - * Returns false when Talk is not available at all. + * Also returns false when no backend is available * * @return bool * @since 34.0.0 */ - public function isDisabledForUser(): bool; + public function isEnabledForUser(): bool; } diff --git a/lib/public/Talk/ITalkBackend.php b/lib/public/Talk/ITalkBackend.php index e8faa37822300..f2fbec7740a95 100644 --- a/lib/public/Talk/ITalkBackend.php +++ b/lib/public/Talk/ITalkBackend.php @@ -44,26 +44,20 @@ public function createConversation(string $name, public function deleteConversation(string $id): void; /** - * Check if the logged in user is not allowed to create conversations, - * respecting the per-group restrictions configured in Talk admin settings - * (allowed_groups). + * Check if the logged-in user is allowed to create conversations * - * Returns false when Talk is not available at all. + * Also returns false when no backend is enabled for the user * * @return bool * @since 34.0.0 */ - public function isNotAllowedToCreateConversations(): bool; + public function isAllowedToCreateConversations(): bool; /** - * Check if Talk is disabled for the logged in user, respecting the - * per-group restriction configured in Talk admin settings - * (start_conversations). - * - * Returns false when Talk is not available at all. + * Check if the Talk backend is enabled for the logged-in user * * @return bool * @since 34.0.0 */ - public function isDisabledForUser(): bool; + public function isEnabledForUser(): bool; } diff --git a/tests/lib/Talk/BrokerTest.php b/tests/lib/Talk/BrokerTest.php index 579ece658e03e..1bbdcf64e9b7f 100644 --- a/tests/lib/Talk/BrokerTest.php +++ b/tests/lib/Talk/BrokerTest.php @@ -17,6 +17,7 @@ use OCP\IServerContainer; use OCP\Talk\IConversationOptions; use OCP\Talk\ITalkBackend; +use PHPUnit\Framework\Attributes\DataProvider; use Psr\Log\LoggerInterface; use RuntimeException; use Test\TestCase; @@ -51,7 +52,7 @@ public function testHasNoBackendCalledTooEarly(): void { } public function testHasNoBackend(): void { - $this->coordinator->expects(self::once()) + $this->coordinator->expects($this->once()) ->method('getRegistrationContext') ->willReturn($this->createMock(RegistrationContext::class)); @@ -63,16 +64,16 @@ public function testHasNoBackend(): void { public function testHasFaultyBackend(): void { $fakeTalkServiceClass = '\\OCA\\Spreed\\TalkBackend'; $registrationContext = $this->createMock(RegistrationContext::class); - $this->coordinator->expects(self::once()) + $this->coordinator->expects($this->once()) ->method('getRegistrationContext') ->willReturn($registrationContext); - $registrationContext->expects(self::once()) + $registrationContext->expects($this->once()) ->method('getTalkBackendRegistration') ->willReturn(new ServiceRegistration('spreed', $fakeTalkServiceClass)); - $this->container->expects(self::once()) + $this->container->expects($this->once()) ->method('get') ->willThrowException(new QueryException()); - $this->logger->expects(self::once()) + $this->logger->expects($this->once()) ->method('error'); self::assertFalse( @@ -83,14 +84,14 @@ public function testHasFaultyBackend(): void { public function testHasBackend(): void { $fakeTalkServiceClass = '\\OCA\\Spreed\\TalkBackend'; $registrationContext = $this->createMock(RegistrationContext::class); - $this->coordinator->expects(self::once()) + $this->coordinator->expects($this->once()) ->method('getRegistrationContext') ->willReturn($registrationContext); - $registrationContext->expects(self::once()) + $registrationContext->expects($this->once()) ->method('getTalkBackendRegistration') ->willReturn(new ServiceRegistration('spreed', $fakeTalkServiceClass)); $talkService = $this->createMock(ITalkBackend::class); - $this->container->expects(self::once()) + $this->container->expects($this->once()) ->method('get') ->with($fakeTalkServiceClass) ->willReturn($talkService); @@ -110,19 +111,19 @@ public function testNewConversationOptions(): void { public function testCreateConversation(): void { $fakeTalkServiceClass = '\\OCA\\Spreed\\TalkBackend'; $registrationContext = $this->createMock(RegistrationContext::class); - $this->coordinator->expects(self::once()) + $this->coordinator->expects($this->once()) ->method('getRegistrationContext') ->willReturn($registrationContext); - $registrationContext->expects(self::once()) + $registrationContext->expects($this->once()) ->method('getTalkBackendRegistration') ->willReturn(new ServiceRegistration('spreed', $fakeTalkServiceClass)); $talkService = $this->createMock(ITalkBackend::class); - $this->container->expects(self::once()) + $this->container->expects($this->once()) ->method('get') ->with($fakeTalkServiceClass) ->willReturn($talkService); $options = $this->createMock(IConversationOptions::class); - $talkService->expects(self::once()) + $talkService->expects($this->once()) ->method('createConversation') ->with('Watercooler', [], $options); @@ -133,91 +134,115 @@ public function testCreateConversation(): void { ); } - public function testIsDisabledForUserNoBackend(): void { - $this->coordinator->expects(self::once()) + public function testIsEnabledForUserNoBackend(): void { + $this->coordinator->expects($this->once()) ->method('getRegistrationContext') ->willReturn($this->createMock(RegistrationContext::class)); - self::assertTrue( - $this->broker->isDisabledForUser() + self::assertFalse( + $this->broker->isEnabledForUser() ); } - public static function dataIsDisabledForUser(): array { + public static function dataIsEnabledForUser(): array { return [ [true], [false], ]; } - /** - * @dataProvider dataIsDisabledForUser - */ - public function testIsDisabledForUser(bool $disabled): void { + #[DataProvider('dataIsEnabledForUser')] + public function testIsEnabledForUser(bool $enabled): void { $fakeTalkServiceClass = '\\OCA\\Spreed\\TalkBackend'; $registrationContext = $this->createMock(RegistrationContext::class); - $this->coordinator->expects(self::once()) + $this->coordinator->expects($this->once()) ->method('getRegistrationContext') ->willReturn($registrationContext); - $registrationContext->expects(self::once()) + $registrationContext->expects($this->once()) ->method('getTalkBackendRegistration') ->willReturn(new ServiceRegistration('spreed', $fakeTalkServiceClass)); $talkService = $this->createMock(ITalkBackend::class); - $this->container->expects(self::once()) + $this->container->expects($this->once()) ->method('get') ->with($fakeTalkServiceClass) ->willReturn($talkService); - $talkService->expects(self::once()) - ->method('isDisabledForUser') - ->willReturn($disabled); + $talkService->expects($this->once()) + ->method('isEnabledForUser') + ->willReturn($enabled); self::assertSame( - $disabled, - $this->broker->isDisabledForUser() + $enabled, + $this->broker->isEnabledForUser() ); } - public function testIsNotAllowedToCreateConversationsNoBackend(): void { - $this->coordinator->expects(self::once()) + public function testIsAllowedToCreateConversationsNoBackend(): void { + $this->coordinator->expects($this->once()) ->method('getRegistrationContext') ->willReturn($this->createMock(RegistrationContext::class)); - self::assertTrue( - $this->broker->isNotAllowedToCreateConversations() + self::assertFalse( + $this->broker->isAllowedToCreateConversations() + ); + } + + public function testIsAllowedToCreateConversationsBackendDisabled(): void { + $fakeTalkServiceClass = '\\OCA\\Spreed\\TalkBackend'; + $registrationContext = $this->createMock(RegistrationContext::class); + $this->coordinator->expects($this->once()) + ->method('getRegistrationContext') + ->willReturn($registrationContext); + $registrationContext->expects($this->once()) + ->method('getTalkBackendRegistration') + ->willReturn(new ServiceRegistration('spreed', $fakeTalkServiceClass)); + $talkService = $this->createMock(ITalkBackend::class); + $this->container->expects($this->once()) + ->method('get') + ->with($fakeTalkServiceClass) + ->willReturn($talkService); + $talkService->expects($this->once()) + ->method('isEnabledForUser') + ->willReturn(false); + $talkService->expects($this->never()) + ->method('isAllowedToCreateConversations'); + + self::assertFalse( + $this->broker->isAllowedToCreateConversations() ); } - public static function dataIsNotAllowedToCreateConversations(): array { + public static function dataIsAllowedToCreateConversations(): array { return [ [true], [false], ]; } - /** - * @dataProvider dataIsNotAllowedToCreateConversations - */ - public function testIsNotAllowedToCreateConversations(bool $notAllowed): void { + #[DataProvider('dataIsAllowedToCreateConversations')] + public function testIsAllowedToCreateConversations(bool $allowed): void { $fakeTalkServiceClass = '\\OCA\\Spreed\\TalkBackend'; $registrationContext = $this->createMock(RegistrationContext::class); - $this->coordinator->expects(self::once()) + $this->coordinator->expects($this->once()) ->method('getRegistrationContext') ->willReturn($registrationContext); - $registrationContext->expects(self::once()) + $registrationContext->expects($this->once()) ->method('getTalkBackendRegistration') ->willReturn(new ServiceRegistration('spreed', $fakeTalkServiceClass)); $talkService = $this->createMock(ITalkBackend::class); - $this->container->expects(self::once()) + $this->container->expects($this->once()) ->method('get') ->with($fakeTalkServiceClass) ->willReturn($talkService); - $talkService->expects(self::once()) - ->method('isNotAllowedToCreateConversations') - ->willReturn($notAllowed); + $talkService->expects($this->once()) + ->method('isEnabledForUser') + ->willReturn(true); + $talkService->expects($this->once()) + ->method('isAllowedToCreateConversations') + ->willReturn($allowed); self::assertSame( - $notAllowed, - $this->broker->isNotAllowedToCreateConversations() + $allowed, + $this->broker->isAllowedToCreateConversations() ); } }