Skip to content

Commit fd7c610

Browse files
provokateurinprovokateurin
committed
perf: Only apply default settings when user is created or settings are requested
Signed-off-by: provokateurin <kate@provokateurin.de>
1 parent 32d0371 commit fd7c610

File tree

9 files changed

+58
-127
lines changed

9 files changed

+58
-127
lines changed

‎lib/AppInfo/Application.php‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
use OCA\Notifications\Capabilities;
1414
use OCA\Notifications\Listener\AddMissingIndicesListener;
1515
use OCA\Notifications\Listener\BeforeTemplateRenderedListener;
16-
use OCA\Notifications\Listener\PostLoginListener;
1716
use OCA\Notifications\Listener\UserCreatedListener;
1817
use OCA\Notifications\Listener\UserDeletedListener;
1918
use OCA\Notifications\Notifier\AdminNotifications;
@@ -24,7 +23,6 @@
2423
use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent;
2524
use OCP\DB\Events\AddMissingIndicesEvent;
2625
use OCP\Notification\IManager;
27-
use OCP\User\Events\PostLoginEvent;
2826
use OCP\User\Events\UserCreatedEvent;
2927
use OCP\User\Events\UserDeletedEvent;
3028

@@ -47,7 +45,6 @@ public function register(IRegistrationContext $context): void {
4745
$context->registerEventListener(UserDeletedEvent::class, UserDeletedListener::class);
4846
$context->registerEventListener(BeforeTemplateRenderedEvent::class, BeforeTemplateRenderedListener::class);
4947
$context->registerEventListener(UserCreatedEvent::class, UserCreatedListener::class);
50-
$context->registerEventListener(PostLoginEvent::class, PostLoginListener::class);
5148
}
5249

5350
#[\Override]

‎lib/BackgroundJob/GenerateUserSettings.php‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
use OCA\Notifications\Model\Settings;
1212
use OCA\Notifications\Model\SettingsMapper;
13-
use OCP\AppFramework\Db\DoesNotExistException;
1413
use OCP\AppFramework\Utility\ITimeFactory;
1514
use OCP\BackgroundJob\TimedJob;
1615
use OCP\IDBConnection;
@@ -47,15 +46,11 @@ protected function run($argument): void {
4746
return;
4847
}
4948

50-
try {
51-
$this->settingsMapper->getSettingsByUser($user->getUID());
52-
} catch (DoesNotExistException) {
53-
$settings = new Settings();
54-
$settings->setUserId($user->getUID());
55-
$settings->setNextSendTime(1);
56-
$settings->setBatchTime(Settings::EMAIL_SEND_3HOURLY);
49+
// Initializes the default settings
50+
$settings = $this->settingsMapper->getSettingsByUser($user->getUID());
51+
if ($settings->getLastSendId() === 0) {
5752
$settings->setLastSendId($maxId);
58-
$this->settingsMapper->insert($settings);
53+
$this->settingsMapper->update($settings);
5954
}
6055
});
6156
}

‎lib/Controller/SettingsController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public function __construct(
4545
#[OpenAPI]
4646
#[ApiRoute(verb: 'POST', url: '/api/{apiVersion}/settings', requirements: ['apiVersion' => '(v2)'])]
4747
public function personal(int $batchSetting, string $soundNotification, string $soundTalk): DataResponse {
48-
$this->settingsMapper->setBatchSettingForUser($this->userId, $batchSetting);
48+
$this->settingsMapper->setBatchSettingForUser($this->settingsMapper->getSettingsByUser($this->userId), $batchSetting);
4949

5050
$this->config->setUserValue($this->userId, Application::APP_ID, 'sound_notification', $soundNotification !== 'no' ? 'yes' : 'no');
5151
$this->config->setUserValue($this->userId, Application::APP_ID, 'sound_talk', $soundTalk !== 'no' ? 'yes' : 'no');

‎lib/Listener/BeforeTemplateRenderedListener.php‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use OCA\Notifications\AppInfo\Application;
1313
use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent;
1414
use OCP\AppFramework\Http\TemplateResponse;
15+
use OCP\AppFramework\Services\IAppConfig;
1516
use OCP\AppFramework\Services\IInitialState;
1617
use OCP\EventDispatcher\Event;
1718
use OCP\EventDispatcher\IEventListener;
@@ -30,6 +31,7 @@ public function __construct(
3031
protected IUserSession $userSession,
3132
protected IInitialState $initialState,
3233
protected IManager $notificationManager,
34+
protected IAppConfig $appConfig,
3335
) {
3436
}
3537

@@ -49,25 +51,14 @@ public function handle(Event $event): void {
4951
return;
5052
}
5153

52-
$this->initialState->provideInitialState(
53-
'sound_notification',
54-
$this->config->getUserValue(
55-
$user->getUID(),
56-
Application::APP_ID,
57-
'sound_notification',
58-
'yes'
59-
) === 'yes'
60-
);
54+
$defaultSoundNotification = $this->appConfig->getAppValueString(Application::APP_ID, 'sound_notification') === 'yes' ? 'yes' : 'no';
55+
$userSoundNotification = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'sound_notification', $defaultSoundNotification) === 'yes';
56+
$defaultSoundTalk = $this->appConfig->getAppValueString(Application::APP_ID, 'sound_talk') === 'yes' ? 'yes' : 'no';
57+
$userSoundTalk = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'sound_talk', $defaultSoundTalk) === 'yes';
6158

62-
$this->initialState->provideInitialState(
63-
'sound_talk',
64-
$this->config->getUserValue(
65-
$user->getUID(),
66-
Application::APP_ID,
67-
'sound_talk',
68-
'yes'
69-
) === 'yes'
70-
);
59+
$this->initialState->provideInitialState('sound_notification', $userSoundNotification);
60+
61+
$this->initialState->provideInitialState('sound_talk', $userSoundTalk);
7162

7263
/**
7364
* We want to keep offering our push notification service for free, but large

‎lib/Listener/PostLoginListener.php‎

Lines changed: 0 additions & 60 deletions
This file was deleted.

‎lib/Listener/UserCreatedListener.php‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
namespace OCA\Notifications\Listener;
1010

11-
use OCA\Notifications\AppInfo\Application;
1211
use OCA\Notifications\Model\Settings;
1312
use OCA\Notifications\Model\SettingsMapper;
1413
use OCP\EventDispatcher\Event;
@@ -35,20 +34,7 @@ public function handle(Event $event): void {
3534

3635
$userId = $event->getUser()->getUID();
3736

38-
$defaultSoundNotification = $this->config->getAppValue(Application::APP_ID, 'sound_notification') === 'yes' ? 'yes' : 'no';
39-
$defaultSoundTalk = $this->config->getAppValue(Application::APP_ID, 'sound_talk') === 'yes' ? 'yes' : 'no';
40-
$defaultBatchtime = (int)$this->config->getAppValue(Application::APP_ID, 'setting_batchtime');
41-
42-
if ($defaultBatchtime !== Settings::EMAIL_SEND_WEEKLY
43-
&& $defaultBatchtime !== Settings::EMAIL_SEND_DAILY
44-
&& $defaultBatchtime !== Settings::EMAIL_SEND_3HOURLY
45-
&& $defaultBatchtime !== Settings::EMAIL_SEND_HOURLY
46-
&& $defaultBatchtime !== Settings::EMAIL_SEND_OFF) {
47-
$defaultBatchtime = Settings::EMAIL_SEND_3HOURLY;
48-
}
49-
50-
$this->config->setUserValue($userId, Application::APP_ID, 'sound_notification', $defaultSoundNotification);
51-
$this->config->setUserValue($userId, Application::APP_ID, 'sound_talk', $defaultSoundTalk);
52-
$this->settingsMapper->setBatchSettingForUser($userId, $defaultBatchtime);
37+
// Initializes the default settings
38+
$this->settingsMapper->getSettingsByUser($userId);
5339
}
5440
}

‎lib/Model/SettingsMapper.php‎

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@
88

99
namespace OCA\Notifications\Model;
1010

11+
use OCA\Notifications\AppInfo\Application;
1112
use OCP\AppFramework\Db\DoesNotExistException;
1213
use OCP\AppFramework\Db\MultipleObjectsReturnedException;
1314
use OCP\AppFramework\Db\QBMapper;
1415
use OCP\DB\Exception as DBException;
1516
use OCP\DB\QueryBuilder\IQueryBuilder;
17+
use OCP\IAppConfig;
1618
use OCP\IDBConnection;
1719

1820
/**
@@ -23,7 +25,10 @@
2325
* @method list<Settings> findEntities(IQueryBuilder $query)
2426
*/
2527
class SettingsMapper extends QBMapper {
26-
public function __construct(IDBConnection $db) {
28+
public function __construct(
29+
IDBConnection $db,
30+
private IAppConfig $appConfig,
31+
) {
2732
parent::__construct($db, 'notifications_settings', Settings::class);
2833
}
2934

@@ -32,16 +37,34 @@ public function __construct(IDBConnection $db) {
3237
* @return Settings
3338
* @throws DBException
3439
* @throws MultipleObjectsReturnedException
35-
* @throws DoesNotExistException
3640
*/
3741
public function getSettingsByUser(string $userId): Settings {
38-
$query = $this->db->getQueryBuilder();
42+
try {
43+
$query = $this->db->getQueryBuilder();
3944

40-
$query->select('*')
41-
->from($this->getTableName())
42-
->where($query->expr()->eq('user_id', $query->createNamedParameter($userId)));
45+
$query->select('*')
46+
->from($this->getTableName())
47+
->where($query->expr()->eq('user_id', $query->createNamedParameter($userId)));
48+
49+
return $this->findEntity($query);
50+
} catch (DoesNotExistException) {
51+
$defaultBatchtime = (int)$this->appConfig->getValueString(Application::APP_ID, 'setting_batchtime');
52+
53+
if ($defaultBatchtime !== Settings::EMAIL_SEND_WEEKLY
54+
&& $defaultBatchtime !== Settings::EMAIL_SEND_DAILY
55+
&& $defaultBatchtime !== Settings::EMAIL_SEND_3HOURLY
56+
&& $defaultBatchtime !== Settings::EMAIL_SEND_HOURLY
57+
&& $defaultBatchtime !== Settings::EMAIL_SEND_OFF) {
58+
$defaultBatchtime = Settings::EMAIL_SEND_3HOURLY;
59+
}
60+
61+
$settings = new Settings();
62+
$settings->setUserId($userId);
63+
/** @var Settings $settings */
64+
$settings = $this->insert($settings);
4365

44-
return $this->findEntity($query);
66+
return $this->setBatchSettingForUser($settings, $defaultBatchtime);
67+
}
4568
}
4669

4770
/**
@@ -57,16 +80,7 @@ public function deleteSettingsByUser(string $userId): void {
5780
$query->executeStatement();
5881
}
5982

60-
public function setBatchSettingForUser(string $userId, int $batchSetting): void {
61-
try {
62-
$settings = $this->getSettingsByUser($userId);
63-
} catch (DoesNotExistException) {
64-
$settings = new Settings();
65-
$settings->setUserId($userId);
66-
/** @var Settings $settings */
67-
$settings = $this->insert($settings);
68-
}
69-
83+
public function setBatchSettingForUser(Settings $settings, int $batchSetting): Settings {
7084
if ($batchSetting === Settings::EMAIL_SEND_WEEKLY) {
7185
$batchTime = 3600 * 24 * 7;
7286
} elseif ($batchSetting === Settings::EMAIL_SEND_DAILY) {
@@ -91,6 +105,7 @@ public function setBatchSettingForUser(string $userId, int $batchSetting): void
91105
$settings->setNextSendTime(1);
92106
}
93107
$this->update($settings);
108+
return $settings;
94109
}
95110

96111
/**

‎lib/Settings/Personal.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use OCA\Notifications\Model\SettingsMapper;
1515
use OCP\AppFramework\Db\DoesNotExistException;
1616
use OCP\AppFramework\Http\TemplateResponse;
17+
use OCP\AppFramework\Services\IAppConfig;
1718
use OCP\AppFramework\Services\IInitialState;
1819
use OCP\IConfig;
1920
use OCP\IL10N;
@@ -25,6 +26,7 @@
2526
class Personal implements ISettings {
2627
public function __construct(
2728
protected IConfig $config,
29+
protected IAppConfig $appConfig,
2830
protected IL10N $l10n,
2931
protected IUserSession $session,
3032
protected SettingsMapper $settingsMapper,
@@ -63,12 +65,17 @@ public function getForm(): TemplateResponse {
6365
$settingBatchTime = Settings::EMAIL_SEND_3HOURLY;
6466
}
6567

68+
$defaultSoundNotification = $this->appConfig->getAppValueString(Application::APP_ID, 'sound_notification') === 'yes' ? 'yes' : 'no';
69+
$userSoundNotification = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'sound_notification', $defaultSoundNotification) === 'yes';
70+
$defaultSoundTalk = $this->appConfig->getAppValueString(Application::APP_ID, 'sound_talk') === 'yes' ? 'yes' : 'no';
71+
$userSoundTalk = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'sound_talk', $defaultSoundTalk) === 'yes';
72+
6673
$this->initialState->provideInitialState('config', [
6774
'setting' => 'personal',
6875
'is_email_set' => (bool)$user->getEMailAddress(),
6976
'setting_batchtime' => $settingBatchTime,
70-
'sound_notification' => $this->config->getUserValue($user->getUID(), Application::APP_ID, 'sound_notification', 'yes') === 'yes',
71-
'sound_talk' => $this->config->getUserValue($user->getUID(), Application::APP_ID, 'sound_talk', 'yes') === 'yes',
77+
'sound_notification' => $userSoundNotification,
78+
'sound_talk' => $userSoundTalk,
7279
]);
7380

7481
return new TemplateResponse('notifications', 'settings/personal');

‎src/views/AdminSettings.vue‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
<template>
77
<NcSettingsSection
88
:name="t('notifications', 'Notifications defaults')"
9-
:description="t('notifications', 'Configure the default notification settings for new users')">
9+
:description="t('notifications', 'Configure the default notification settings')">
1010
<p>
1111
<label for="notify_setting_batchtime" class="notification-frequency__label">
1212
{{ t('notifications', 'Send email reminders about unhandled notifications after:') }}

0 commit comments

Comments
 (0)