Skip to content

Commit d59736e

Browse files
miaulalalamiaulalala
authored
Merge pull request #2608 from nextcloud/backport/2562/stable33
[stable33] fix(bulkReceive): honour admin email toggle and ISetting notification defaults
2 parents fd10302 + bb7939e commit d59736e

2 files changed

Lines changed: 273 additions & 7 deletions

File tree

‎lib/Consumer.php‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use OCP\Config\IUserConfig;
1717
use OCP\Config\ValueType;
1818
use OCP\DB\Exception;
19+
use OCP\IAppConfig;
1920

2021
class Consumer implements IConsumer, IBulkConsumer {
2122

@@ -25,6 +26,7 @@ public function __construct(
2526
protected UserSettings $userSettings,
2627
protected NotificationGenerator $notificationGenerator,
2728
protected IUserConfig $userConfig,
29+
protected IAppConfig $appConfig,
2830
) {
2931
}
3032

@@ -78,13 +80,16 @@ public function bulkReceive(IEvent $event, array $affectedUserIds, ISetting $set
7880

7981
$canChangeMail = $setting->canChangeMail();
8082
$canChangePush = $setting instanceof ActivitySettings && $setting->canChangeNotification() === true;
83+
$defaultPushEnabled = $setting instanceof ActivitySettings && $setting->isDefaultEnabledNotification();
8184

82-
$userPushSettings = $userEmailSettings = $batchTimeSettings = null;
85+
$userPushSettings = null;
8386
if ($canChangePush === true) {
8487
$userPushSettings = $this->userConfig->getValuesByUsers('activity', 'notify_notification_' . $event->getType(), ValueType::BOOL, $affectedUserIds);
8588
}
8689

87-
if ($canChangeMail === true || $setting->isDefaultEnabledMail() === true) {
90+
$userEmailSettings = $batchTimeSettings = null;
91+
$emailEnabled = $this->appConfig->getValueString('activity', 'enable_email', 'yes') !== 'no';
92+
if ($emailEnabled && ($canChangeMail === true || $setting->isDefaultEnabledMail() === true)) {
8893
$userEmailSettings = $this->userConfig->getValuesByUsers('activity', 'notify_email_' . $event->getType(), ValueType::BOOL, $affectedUserIds);
8994
$batchTimeSettings = $this->userConfig->getValuesByUsers('activity', 'notify_setting_batchtime', ValueType::INT, $affectedUserIds);
9095
}
@@ -95,14 +100,17 @@ public function bulkReceive(IEvent $event, array $affectedUserIds, ISetting $set
95100
continue;
96101
}
97102
$event->setAffectedUser($affectedUser);
98-
$notificationSetting = $userPushSettings[$affectedUser] ?? null;
99-
if ($notificationSetting !== null) {
100-
$notificationSetting = (bool)$notificationSetting;
103+
104+
if ($canChangePush === true) {
105+
$notificationSetting = isset($userPushSettings[$affectedUser]) ? (bool)$userPushSettings[$affectedUser] : $defaultPushEnabled;
106+
} else {
107+
$notificationSetting = $defaultPushEnabled;
101108
}
109+
102110
$emailSetting = $userEmailSettings[$affectedUser] ?? false;
103111
$emailSetting = ($emailSetting) ? ($batchTimeSettings[$affectedUser] ?? false) : false;
104112

105-
if ($notificationSetting !== false) {
113+
if ($notificationSetting === true) {
106114
$this->notificationGenerator->sendNotificationForEvent($event, $activityId, $notificationSetting);
107115
}
108116

‎tests/ConsumerTest.php‎

Lines changed: 259 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@
3131
use OCP\Activity\ActivitySettings;
3232
use OCP\Activity\IEvent;
3333
use OCP\Activity\IManager;
34+
use OCP\Activity\ISetting;
3435
use OCP\Config\IUserConfig;
36+
use OCP\IAppConfig;
3537
use OCP\IDBConnection;
3638
use OCP\IL10N;
3739
use OCP\L10N\IFactory;
@@ -52,6 +54,7 @@ class ConsumerTest extends TestCase {
5254
protected NotificationGenerator&MockObject $notificationGenerator;
5355
protected UserSettings $userSettings;
5456
private IUserConfig&MockObject $userConfig;
57+
private IAppConfig&MockObject $appConfig;
5558
private IEvent $event;
5659
private Consumer $consumer;
5760

@@ -69,7 +72,10 @@ protected function setUp(): void {
6972
$this->notificationGenerator = $this->createMock(NotificationGenerator::class);
7073
$this->l10nFactory = $this->createMock(IFactory::class);
7174
$this->userConfig = $this->createMock(IUserConfig::class);
72-
75+
$this->appConfig = $this->createMock(IAppConfig::class);
76+
$this->appConfig->method('getValueString')
77+
->with('activity', 'enable_email', 'yes')
78+
->willReturn('yes');
7379
$this->data->method('send')
7480
->willReturn(1);
7581
$this->l10nFactory
@@ -94,6 +100,7 @@ protected function setUp(): void {
94100
$this->userSettings,
95101
$this->notificationGenerator,
96102
$this->userConfig,
103+
$this->appConfig,
97104
);
98105

99106
$this->event = Server::get(IManager::class)->generateEvent();
@@ -142,6 +149,7 @@ public function testReceiveStream(string $type, string $author, string $affected
142149
$this->userSettings,
143150
$this->notificationGenerator,
144151
$this->userConfig,
152+
$this->appConfig,
145153
);
146154
$event = Server::get(IManager::class)->generateEvent();
147155
$event->setApp('test')
@@ -263,4 +271,254 @@ public function testBulkReceiveNotification(string $type, string $author, string
263271
$this->consumer->bulkReceive($this->event, $affectedUsers, $settings);
264272
}
265273

274+
public function testBulkReceiveEmail(): void {
275+
$time = time();
276+
$this->event->setApp('activity')
277+
->setType('type')
278+
->setAuthor('author')
279+
->setTimestamp($time)
280+
->setSubject('subject', ['subjectParam1'])
281+
->setMessage('message', ['messageParam1'])
282+
->setObject('', 0, 'file')
283+
->setLink('link');
284+
285+
$settings = $this->createMock(ActivitySettings::class);
286+
$settings->method('canChangeMail')->willReturn(true);
287+
$settings->method('isDefaultEnabledMail')->willReturn(true);
288+
$settings->method('canChangeNotification')->willReturn(false);
289+
290+
$this->data->expects($this->once())
291+
->method('bulkSend')
292+
->willReturn([1 => 'affectedUser', 2 => 'affectedUser2']);
293+
294+
$this->userConfig->method('getValuesByUsers')
295+
->willReturnCallback(function (string $app, string $key, mixed $type, array $users): array {
296+
if (str_contains($key, 'email')) {
297+
return array_fill_keys($users, true);
298+
}
299+
if (str_contains($key, 'batchtime')) {
300+
return array_fill_keys($users, 10);
301+
}
302+
return [];
303+
});
304+
305+
$this->data->expects($this->exactly(2))
306+
->method('storeMail');
307+
308+
$this->consumer->bulkReceive($this->event, ['affectedUser', 'affectedUser2'], $settings);
309+
}
310+
311+
public function testBulkReceiveNoMailWhenSettingDisabled(): void {
312+
$this->event->setApp('activity')
313+
->setType('type')
314+
->setAuthor('author')
315+
->setTimestamp(time())
316+
->setSubject('subject', ['subjectParam1'])
317+
->setMessage('message', ['messageParam1'])
318+
->setObject('', 0, 'file')
319+
->setLink('link');
320+
321+
$settings = $this->createMock(ActivitySettings::class);
322+
$settings->method('canChangeMail')->willReturn(false);
323+
$settings->method('isDefaultEnabledMail')->willReturn(false);
324+
$settings->method('canChangeNotification')->willReturn(false);
325+
326+
$this->data->expects($this->once())
327+
->method('bulkSend')
328+
->willReturn([1 => 'affectedUser']);
329+
330+
$this->data->expects($this->never())
331+
->method('storeMail');
332+
$this->notificationGenerator->expects($this->never())
333+
->method('sendNotificationForEvent');
334+
335+
$this->consumer->bulkReceive($this->event, ['affectedUser'], $settings);
336+
}
337+
338+
public function testBulkReceiveDeferAndFlushNotifications(): void {
339+
$this->event->setApp('activity')
340+
->setType('type')
341+
->setAuthor('author')
342+
->setTimestamp(time())
343+
->setSubject('subject', ['subjectParam1'])
344+
->setMessage('message', ['messageParam1'])
345+
->setObject('', 0, 'file')
346+
->setLink('link');
347+
348+
$settings = $this->createMock(ActivitySettings::class);
349+
$settings->method('canChangeMail')->willReturn(false);
350+
$settings->method('isDefaultEnabledMail')->willReturn(false);
351+
$settings->method('canChangeNotification')->willReturn(true);
352+
353+
$this->data->expects($this->once())
354+
->method('bulkSend')
355+
->willReturn([1 => 'affectedUser']);
356+
357+
$this->userConfig->method('getValuesByUsers')
358+
->willReturnCallback(function (string $app, string $key, mixed $type, array $users): array {
359+
return array_fill_keys($users, true);
360+
});
361+
362+
$this->notificationGenerator->expects($this->once())
363+
->method('deferNotifications')
364+
->willReturn(true);
365+
$this->notificationGenerator->expects($this->once())
366+
->method('flushNotifications');
367+
$this->notificationGenerator->expects($this->once())
368+
->method('sendNotificationForEvent');
369+
370+
$this->consumer->bulkReceive($this->event, ['affectedUser'], $settings);
371+
}
372+
373+
public function testBulkReceiveNoFlushWhenDeferReturnsFalse(): void {
374+
$this->event->setApp('activity')
375+
->setType('type')
376+
->setAuthor('author')
377+
->setTimestamp(time())
378+
->setSubject('subject', ['subjectParam1'])
379+
->setMessage('message', ['messageParam1'])
380+
->setObject('', 0, 'file')
381+
->setLink('link');
382+
383+
$settings = $this->createMock(ActivitySettings::class);
384+
$settings->method('canChangeMail')->willReturn(false);
385+
$settings->method('isDefaultEnabledMail')->willReturn(false);
386+
$settings->method('canChangeNotification')->willReturn(true);
387+
388+
$this->data->expects($this->once())
389+
->method('bulkSend')
390+
->willReturn([1 => 'affectedUser']);
391+
392+
$this->userConfig->method('getValuesByUsers')
393+
->willReturnCallback(function (string $app, string $key, mixed $type, array $users): array {
394+
return array_fill_keys($users, true);
395+
});
396+
397+
$this->notificationGenerator->expects($this->once())
398+
->method('deferNotifications')
399+
->willReturn(false);
400+
$this->notificationGenerator->expects($this->never())
401+
->method('flushNotifications');
402+
403+
$this->consumer->bulkReceive($this->event, ['affectedUser'], $settings);
404+
}
405+
406+
public function testBulkReceiveMultipleUsersWithMixedSettings(): void {
407+
$time = time();
408+
$this->event->setApp('activity')
409+
->setType('type')
410+
->setAuthor('author')
411+
->setTimestamp($time)
412+
->setSubject('subject', ['subjectParam1'])
413+
->setMessage('message', ['messageParam1'])
414+
->setObject('', 0, 'file')
415+
->setLink('link');
416+
417+
$settings = $this->createMock(ActivitySettings::class);
418+
$settings->method('canChangeMail')->willReturn(true);
419+
$settings->method('isDefaultEnabledMail')->willReturn(true);
420+
$settings->method('canChangeNotification')->willReturn(true);
421+
422+
$this->data->expects($this->once())
423+
->method('bulkSend')
424+
->willReturn([1 => 'user1', 2 => 'user2', 3 => 'author']);
425+
426+
$this->userConfig->method('getValuesByUsers')
427+
->willReturnCallback(function (string $app, string $key, mixed $type, array $users): array {
428+
if (str_contains($key, 'notification')) {
429+
// Only user1 has notifications enabled
430+
return ['user1' => true];
431+
}
432+
if (str_contains($key, 'email')) {
433+
// Only user2 has email enabled
434+
return ['user2' => true];
435+
}
436+
if (str_contains($key, 'batchtime')) {
437+
return ['user2' => 15];
438+
}
439+
return [];
440+
});
441+
442+
// only user1 has an explicit notification=true in DB; user2 has no entry so falls back to isDefaultEnabledNotification()=false
443+
$this->notificationGenerator->expects($this->once())
444+
->method('sendNotificationForEvent');
445+
// user2 gets email, author is skipped
446+
$this->data->expects($this->once())
447+
->method('storeMail')
448+
->with($this->event, $time + 15);
449+
450+
$this->consumer->bulkReceive($this->event, ['user1', 'user2', 'author'], $settings);
451+
}
452+
453+
public function testBulkReceiveNoMailWhenAdminEmailDisabled(): void {
454+
$time = time();
455+
$this->event->setApp('activity')
456+
->setType('type')
457+
->setAuthor('author')
458+
->setTimestamp($time)
459+
->setSubject('subject', ['subjectParam1'])
460+
->setMessage('message', ['messageParam1'])
461+
->setObject('', 0, 'file')
462+
->setLink('link');
463+
464+
$appConfig = $this->createMock(IAppConfig::class);
465+
$appConfig->method('getValueString')
466+
->with('activity', 'enable_email', 'yes')
467+
->willReturn('no');
468+
469+
$consumer = new Consumer(
470+
$this->data,
471+
$this->activityManager,
472+
$this->userSettings,
473+
$this->notificationGenerator,
474+
$this->userConfig,
475+
$appConfig,
476+
);
477+
478+
$settings = $this->createMock(ActivitySettings::class);
479+
$settings->method('canChangeMail')->willReturn(true);
480+
$settings->method('isDefaultEnabledMail')->willReturn(true);
481+
$settings->method('canChangeNotification')->willReturn(false);
482+
$settings->method('isDefaultEnabledNotification')->willReturn(false);
483+
484+
$this->data->expects($this->once())
485+
->method('bulkSend')
486+
->willReturn([1 => 'affectedUser']);
487+
488+
$this->userConfig->expects($this->never())
489+
->method('getValuesByUsers');
490+
491+
$this->data->expects($this->never())
492+
->method('storeMail');
493+
494+
$consumer->bulkReceive($this->event, ['affectedUser'], $settings);
495+
}
496+
497+
public function testBulkReceiveWithISetting(): void {
498+
$this->event->setApp('activity')
499+
->setType('type')
500+
->setAuthor('author')
501+
->setTimestamp(time())
502+
->setSubject('subject', ['subjectParam1'])
503+
->setMessage('message', ['messageParam1'])
504+
->setObject('', 0, 'file')
505+
->setLink('link');
506+
507+
// ISetting (not ActivitySettings) — canChangeNotification is not available
508+
$settings = $this->createMock(ISetting::class);
509+
$settings->method('canChangeMail')->willReturn(false);
510+
511+
$this->data->expects($this->once())
512+
->method('bulkSend')
513+
->willReturn([1 => 'affectedUser']);
514+
515+
// ISetting is not ActivitySettings so $defaultPushEnabled is false — no notification sent
516+
$this->notificationGenerator->expects($this->never())
517+
->method('sendNotificationForEvent');
518+
$this->data->expects($this->never())
519+
->method('storeMail');
520+
521+
$this->consumer->bulkReceive($this->event, ['affectedUser'], $settings);
522+
}
523+
266524
}

0 commit comments

Comments
 (0)