Skip to content

Commit 644bfe4

Browse files
committed
fix(MailQueueHandler): check enable_email toggle before sending queued emails
The admin Enable notification emails toggle was only checked when queuing new emails. The sendEmails() background job had no knowledge of it and would send all queued entries regardless. Adds an early-return guard using IAppConfig::getValueString() at the top of sendEmails(), consistent with the checks in UserSettings. The queue is left intact so pending notifications can be delivered if the admin re-enables emails. AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Anna Larch <anna@nextcloud.com>
1 parent 29d896c commit 644bfe4

3 files changed

Lines changed: 33 additions & 0 deletions

File tree

lib/AppInfo/Application.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
use OCP\AppFramework\Bootstrap\IBootstrap;
3232
use OCP\AppFramework\Bootstrap\IRegistrationContext;
3333
use OCP\DB\Events\AddMissingIndicesEvent;
34+
use OCP\IAppConfig;
3435
use OCP\IConfig;
3536
use OCP\IDateTimeFormatter;
3637
use OCP\IDBConnection;
@@ -117,6 +118,7 @@ public function register(IRegistrationContext $context): void {
117118
$c->get(IFactory::class),
118119
$c->get(IManager::class),
119120
$c->get(IValidator::class),
121+
$c->get(IAppConfig::class),
120122
$c->get(IConfig::class),
121123
$c->get(LoggerInterface::class),
122124
$c->get(Data::class),

lib/MailQueueHandler.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use OCP\Activity\IManager;
1212
use OCP\DB\QueryBuilder\IQueryBuilder;
1313
use OCP\Defaults;
14+
use OCP\IAppConfig;
1415
use OCP\IConfig;
1516
use OCP\IDateTimeFormatter;
1617
use OCP\IDBConnection;
@@ -51,6 +52,7 @@ public function __construct(
5152
protected IFactory $lFactory,
5253
protected IManager $activityManager,
5354
protected IValidator $richObjectValidator,
55+
protected IAppConfig $appConfig,
5456
protected IConfig $config,
5557
protected LoggerInterface $logger,
5658
protected Data $data,
@@ -70,6 +72,10 @@ public function __construct(
7072
* @return int Number of users we sent an email to
7173
*/
7274
public function sendEmails(int $limit, int $sendTime, bool $forceSending = false, ?int $restrictEmails = null): int {
75+
if ($this->appConfig->getValueString('activity', 'enable_email', 'yes') === 'no') {
76+
return 0;
77+
}
78+
7379
// Get all users which should receive an email
7480
$affectedUsers = $this->getAffectedUsers($limit, $sendTime, $forceSending, $restrictEmails);
7581
if (empty($affectedUsers)) {

tests/MailQueueHandlerTest.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
use OCA\Activity\UserSettings;
3535
use OCP\Activity\IEvent;
3636
use OCP\Activity\IManager;
37+
use OCP\IAppConfig;
3738
use OCP\IConfig;
3839
use OCP\IDateTimeFormatter;
3940
use OCP\IDBConnection;
@@ -65,6 +66,7 @@ class MailQueueHandlerTest extends TestCase {
6566
protected IFactory&MockObject $lFactory;
6667
protected IManager&MockObject $activityManager;
6768
protected IValidator&MockObject $richObjectValidator;
69+
protected IAppConfig&MockObject $appConfig;
6870
protected IConfig&MockObject $config;
6971
protected MockObject&LoggerInterface $logger;
7072

@@ -81,6 +83,7 @@ protected function setUp(): void {
8183
$app = self::getUniqueID('MailQueueHandlerTest', 10);
8284
$this->userManager = $this->createMock(IUserManager::class);
8385
$this->lFactory = $this->createMock(IFactory::class);
86+
$this->appConfig = $this->createMock(IAppConfig::class);
8487
$this->config = $this->createMock(IConfig::class);
8588
$this->logger = $this->createMock(LoggerInterface::class);
8689
$this->dateTimeFormatter = $this->createMock(IDateTimeFormatter::class);
@@ -141,6 +144,7 @@ protected function setUp(): void {
141144
$this->lFactory,
142145
$this->activityManager,
143146
$this->richObjectValidator,
147+
$this->appConfig,
144148
$this->config,
145149
$this->logger,
146150
$this->data,
@@ -370,6 +374,27 @@ public function testSendEmailsDeletesQueueOnSendReturnFalse(): void {
370374
}
371375
}
372376

377+
public function testSendEmailsSkipsWhenAdminEmailDisabled(): void {
378+
$maxTime = 200;
379+
380+
$this->appConfig->method('getValueString')
381+
->with('activity', 'enable_email', 'yes')
382+
->willReturn('no');
383+
384+
$this->mailer->expects($this->never())
385+
->method('send');
386+
387+
$result = $this->mailQueueHandler->sendEmails(3, $maxTime);
388+
389+
$this->assertSame(0, $result);
390+
391+
// Queue must be untouched so emails can be sent if admin re-enables the toggle
392+
foreach (['user1', 'user2', 'user3'] as $user) {
393+
[$data,] = self::invokePrivate($this->mailQueueHandler, 'getItemsForUser', [$user, $maxTime]);
394+
$this->assertNotEmpty($data, "Queue entries for $user must survive when email is globally disabled");
395+
}
396+
}
397+
373398
/**
374399
* @param array $users
375400
* @param int $maxTime

0 commit comments

Comments
 (0)