Skip to content

Commit 109eb0c

Browse files
nickvergessennickvergessen
committed
fix(talkbackend): Make function names positive and remove talk internals from docs
Signed-off-by: Joas Schilling <coding@schilljs.com>
1 parent c55cfc4 commit 109eb0c

4 files changed

Lines changed: 89 additions & 74 deletions

File tree

‎lib/private/Talk/Broker.php‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,19 +92,19 @@ public function deleteConversation(string $id): void {
9292
$this->backend->deleteConversation($id);
9393
}
9494

95-
public function isDisabledForUser(): bool {
96-
if (!$this->hasBackend()) {
97-
return true;
95+
public function isAllowedToCreateConversations(): bool {
96+
if (!$this->isEnabledForUser()) {
97+
return false;
9898
}
9999

100-
return $this->backend->isDisabledForUser();
100+
return $this->backend->isAllowedToCreateConversations();
101101
}
102102

103-
public function isNotAllowedToCreateConversations(): bool {
103+
public function isEnabledForUser(): bool {
104104
if (!$this->hasBackend()) {
105-
return true;
105+
return false;
106106
}
107107

108-
return $this->backend->isNotAllowedToCreateConversations();
108+
return $this->backend->isEnabledForUser();
109109
}
110110
}

‎lib/public/Talk/IBroker.php‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,26 +67,22 @@ public function createConversation(string $name,
6767
public function deleteConversation(string $id): void;
6868

6969
/**
70-
* Check if the logged in user is not allowed to create conversations,
71-
* respecting the per-group restrictions configured in Talk admin settings
72-
* (allowed_groups).
70+
* Check if the logged-in user is allowed to create conversations
7371
*
74-
* Returns false when Talk is not available at all.
72+
* Also returns false when no backend is available
7573
*
7674
* @return bool
7775
* @since 34.0.0
7876
*/
79-
public function isNotAllowedToCreateConversations(): bool;
77+
public function isAllowedToCreateConversations(): bool;
8078

8179
/**
82-
* Check if Talk is disabled for the logged in user, respecting the
83-
* per-group restriction configured in Talk admin settings
84-
* (start_conversations).
80+
* Check if the Talk backend is enabled for the logged-in user
8581
*
86-
* Returns false when Talk is not available at all.
82+
* Also returns false when no backend is available
8783
*
8884
* @return bool
8985
* @since 34.0.0
9086
*/
91-
public function isDisabledForUser(): bool;
87+
public function isEnabledForUser(): bool;
9288
}

‎lib/public/Talk/ITalkBackend.php‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,26 +44,20 @@ public function createConversation(string $name,
4444
public function deleteConversation(string $id): void;
4545

4646
/**
47-
* Check if the logged in user is not allowed to create conversations,
48-
* respecting the per-group restrictions configured in Talk admin settings
49-
* (allowed_groups).
47+
* Check if the logged-in user is allowed to create conversations
5048
*
51-
* Returns false when Talk is not available at all.
49+
* Also returns false when no backend is enabled for the user
5250
*
5351
* @return bool
5452
* @since 34.0.0
5553
*/
56-
public function isNotAllowedToCreateConversations(): bool;
54+
public function isAllowedToCreateConversations(): bool;
5755

5856
/**
59-
* Check if Talk is disabled for the logged in user, respecting the
60-
* per-group restriction configured in Talk admin settings
61-
* (start_conversations).
62-
*
63-
* Returns false when Talk is not available at all.
57+
* Check if the Talk backend is enabled for the logged-in user
6458
*
6559
* @return bool
6660
* @since 34.0.0
6761
*/
68-
public function isDisabledForUser(): bool;
62+
public function isEnabledForUser(): bool;
6963
}

‎tests/lib/Talk/BrokerTest.php‎

Lines changed: 71 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use OCP\IServerContainer;
1818
use OCP\Talk\IConversationOptions;
1919
use OCP\Talk\ITalkBackend;
20+
use PHPUnit\Framework\Attributes\DataProvider;
2021
use Psr\Log\LoggerInterface;
2122
use RuntimeException;
2223
use Test\TestCase;
@@ -51,7 +52,7 @@ public function testHasNoBackendCalledTooEarly(): void {
5152
}
5253

5354
public function testHasNoBackend(): void {
54-
$this->coordinator->expects(self::once())
55+
$this->coordinator->expects($this->once())
5556
->method('getRegistrationContext')
5657
->willReturn($this->createMock(RegistrationContext::class));
5758

@@ -63,16 +64,16 @@ public function testHasNoBackend(): void {
6364
public function testHasFaultyBackend(): void {
6465
$fakeTalkServiceClass = '\\OCA\\Spreed\\TalkBackend';
6566
$registrationContext = $this->createMock(RegistrationContext::class);
66-
$this->coordinator->expects(self::once())
67+
$this->coordinator->expects($this->once())
6768
->method('getRegistrationContext')
6869
->willReturn($registrationContext);
69-
$registrationContext->expects(self::once())
70+
$registrationContext->expects($this->once())
7071
->method('getTalkBackendRegistration')
7172
->willReturn(new ServiceRegistration('spreed', $fakeTalkServiceClass));
72-
$this->container->expects(self::once())
73+
$this->container->expects($this->once())
7374
->method('get')
7475
->willThrowException(new QueryException());
75-
$this->logger->expects(self::once())
76+
$this->logger->expects($this->once())
7677
->method('error');
7778

7879
self::assertFalse(
@@ -83,14 +84,14 @@ public function testHasFaultyBackend(): void {
8384
public function testHasBackend(): void {
8485
$fakeTalkServiceClass = '\\OCA\\Spreed\\TalkBackend';
8586
$registrationContext = $this->createMock(RegistrationContext::class);
86-
$this->coordinator->expects(self::once())
87+
$this->coordinator->expects($this->once())
8788
->method('getRegistrationContext')
8889
->willReturn($registrationContext);
89-
$registrationContext->expects(self::once())
90+
$registrationContext->expects($this->once())
9091
->method('getTalkBackendRegistration')
9192
->willReturn(new ServiceRegistration('spreed', $fakeTalkServiceClass));
9293
$talkService = $this->createMock(ITalkBackend::class);
93-
$this->container->expects(self::once())
94+
$this->container->expects($this->once())
9495
->method('get')
9596
->with($fakeTalkServiceClass)
9697
->willReturn($talkService);
@@ -110,19 +111,19 @@ public function testNewConversationOptions(): void {
110111
public function testCreateConversation(): void {
111112
$fakeTalkServiceClass = '\\OCA\\Spreed\\TalkBackend';
112113
$registrationContext = $this->createMock(RegistrationContext::class);
113-
$this->coordinator->expects(self::once())
114+
$this->coordinator->expects($this->once())
114115
->method('getRegistrationContext')
115116
->willReturn($registrationContext);
116-
$registrationContext->expects(self::once())
117+
$registrationContext->expects($this->once())
117118
->method('getTalkBackendRegistration')
118119
->willReturn(new ServiceRegistration('spreed', $fakeTalkServiceClass));
119120
$talkService = $this->createMock(ITalkBackend::class);
120-
$this->container->expects(self::once())
121+
$this->container->expects($this->once())
121122
->method('get')
122123
->with($fakeTalkServiceClass)
123124
->willReturn($talkService);
124125
$options = $this->createMock(IConversationOptions::class);
125-
$talkService->expects(self::once())
126+
$talkService->expects($this->once())
126127
->method('createConversation')
127128
->with('Watercooler', [], $options);
128129

@@ -133,91 +134,115 @@ public function testCreateConversation(): void {
133134
);
134135
}
135136

136-
public function testIsDisabledForUserNoBackend(): void {
137-
$this->coordinator->expects(self::once())
137+
public function testIsEnabledForUserNoBackend(): void {
138+
$this->coordinator->expects($this->once())
138139
->method('getRegistrationContext')
139140
->willReturn($this->createMock(RegistrationContext::class));
140141

141-
self::assertTrue(
142-
$this->broker->isDisabledForUser()
142+
self::assertFalse(
143+
$this->broker->isEnabledForUser()
143144
);
144145
}
145146

146-
public static function dataIsDisabledForUser(): array {
147+
public static function dataIsEnabledForUser(): array {
147148
return [
148149
[true],
149150
[false],
150151
];
151152
}
152153

153-
/**
154-
* @dataProvider dataIsDisabledForUser
155-
*/
156-
public function testIsDisabledForUser(bool $disabled): void {
154+
#[DataProvider('dataIsEnabledForUser')]
155+
public function testIsEnabledForUser(bool $enabled): void {
157156
$fakeTalkServiceClass = '\\OCA\\Spreed\\TalkBackend';
158157
$registrationContext = $this->createMock(RegistrationContext::class);
159-
$this->coordinator->expects(self::once())
158+
$this->coordinator->expects($this->once())
160159
->method('getRegistrationContext')
161160
->willReturn($registrationContext);
162-
$registrationContext->expects(self::once())
161+
$registrationContext->expects($this->once())
163162
->method('getTalkBackendRegistration')
164163
->willReturn(new ServiceRegistration('spreed', $fakeTalkServiceClass));
165164
$talkService = $this->createMock(ITalkBackend::class);
166-
$this->container->expects(self::once())
165+
$this->container->expects($this->once())
167166
->method('get')
168167
->with($fakeTalkServiceClass)
169168
->willReturn($talkService);
170-
$talkService->expects(self::once())
171-
->method('isDisabledForUser')
172-
->willReturn($disabled);
169+
$talkService->expects($this->once())
170+
->method('isEnabledForUser')
171+
->willReturn($enabled);
173172

174173
self::assertSame(
175-
$disabled,
176-
$this->broker->isDisabledForUser()
174+
$enabled,
175+
$this->broker->isEnabledForUser()
177176
);
178177
}
179178

180-
public function testIsNotAllowedToCreateConversationsNoBackend(): void {
181-
$this->coordinator->expects(self::once())
179+
public function testIsAllowedToCreateConversationsNoBackend(): void {
180+
$this->coordinator->expects($this->once())
182181
->method('getRegistrationContext')
183182
->willReturn($this->createMock(RegistrationContext::class));
184183

185-
self::assertTrue(
186-
$this->broker->isNotAllowedToCreateConversations()
184+
self::assertFalse(
185+
$this->broker->isAllowedToCreateConversations()
186+
);
187+
}
188+
189+
public function testIsAllowedToCreateConversationsBackendDisabled(): void {
190+
$fakeTalkServiceClass = '\\OCA\\Spreed\\TalkBackend';
191+
$registrationContext = $this->createMock(RegistrationContext::class);
192+
$this->coordinator->expects($this->once())
193+
->method('getRegistrationContext')
194+
->willReturn($registrationContext);
195+
$registrationContext->expects($this->once())
196+
->method('getTalkBackendRegistration')
197+
->willReturn(new ServiceRegistration('spreed', $fakeTalkServiceClass));
198+
$talkService = $this->createMock(ITalkBackend::class);
199+
$this->container->expects($this->once())
200+
->method('get')
201+
->with($fakeTalkServiceClass)
202+
->willReturn($talkService);
203+
$talkService->expects($this->once())
204+
->method('isEnabledForUser')
205+
->willReturn(false);
206+
$talkService->expects($this->never())
207+
->method('isAllowedToCreateConversations');
208+
209+
self::assertFalse(
210+
$this->broker->isAllowedToCreateConversations()
187211
);
188212
}
189213

190-
public static function dataIsNotAllowedToCreateConversations(): array {
214+
public static function dataIsAllowedToCreateConversations(): array {
191215
return [
192216
[true],
193217
[false],
194218
];
195219
}
196220

197-
/**
198-
* @dataProvider dataIsNotAllowedToCreateConversations
199-
*/
200-
public function testIsNotAllowedToCreateConversations(bool $notAllowed): void {
221+
#[DataProvider('dataIsAllowedToCreateConversations')]
222+
public function testIsAllowedToCreateConversations(bool $allowed): void {
201223
$fakeTalkServiceClass = '\\OCA\\Spreed\\TalkBackend';
202224
$registrationContext = $this->createMock(RegistrationContext::class);
203-
$this->coordinator->expects(self::once())
225+
$this->coordinator->expects($this->once())
204226
->method('getRegistrationContext')
205227
->willReturn($registrationContext);
206-
$registrationContext->expects(self::once())
228+
$registrationContext->expects($this->once())
207229
->method('getTalkBackendRegistration')
208230
->willReturn(new ServiceRegistration('spreed', $fakeTalkServiceClass));
209231
$talkService = $this->createMock(ITalkBackend::class);
210-
$this->container->expects(self::once())
232+
$this->container->expects($this->once())
211233
->method('get')
212234
->with($fakeTalkServiceClass)
213235
->willReturn($talkService);
214-
$talkService->expects(self::once())
215-
->method('isNotAllowedToCreateConversations')
216-
->willReturn($notAllowed);
236+
$talkService->expects($this->once())
237+
->method('isEnabledForUser')
238+
->willReturn(true);
239+
$talkService->expects($this->once())
240+
->method('isAllowedToCreateConversations')
241+
->willReturn($allowed);
217242

218243
self::assertSame(
219-
$notAllowed,
220-
$this->broker->isNotAllowedToCreateConversations()
244+
$allowed,
245+
$this->broker->isAllowedToCreateConversations()
221246
);
222247
}
223248
}

0 commit comments

Comments
 (0)