Fix review comments

Signed-off-by: Joas Schilling <coding@schilljs.com>

4 files changed, 10 insertions, 6 deletions

diff --git a/lib/BackgroundJob/GenerateUserSettings.php b/lib/BackgroundJob/GenerateUserSettings.php
index 6b67330..fdc9177 100644
--- a/lib/BackgroundJob/GenerateUserSettings.php
+++ b/lib/BackgroundJob/GenerateUserSettings.php
@@ -56,7 +56,7 @@ class GenerateUserSettings extends TimedJob {
 		$this->setInterval(24 * 60 * 60);
 	}
 
-	protected function run($argument) {
+	protected function run($argument): void {
 		$query = $this->connection->getQueryBuilder();
 		$query->selectAlias('id', 'max_id')
 			->from('notifications')
diff --git a/lib/Handler.php b/lib/Handler.php
index 4a294c3..ca7c18e 100644
--- a/lib/Handler.php
+++ b/lib/Handler.php
@@ -197,10 +197,10 @@ class Handler {
 		$sql = $this->connection->getQueryBuilder();
 		$sql->select('*')
 			->from('notifications')
-			->orderBy('notification_id', 'DESC')
-			->setMaxResults($limit)
 			->where($sql->expr()->gt('notification_id', $sql->createNamedParameter($startAfterId)))
-			->andWhere($sql->expr()->eq('user', $sql->createNamedParameter($userId)));
+			->andWhere($sql->expr()->eq('user', $sql->createNamedParameter($userId)))
+			->orderBy('notification_id', 'DESC')
+			->setMaxResults($limit);
 		$statement = $sql->executeQuery();
 
 		$notifications = [];
diff --git a/lib/Model/SettingsMapper.php b/lib/Model/SettingsMapper.php
index dbffc1e..648d908 100644
--- a/lib/Model/SettingsMapper.php
+++ b/lib/Model/SettingsMapper.php
@@ -67,7 +67,6 @@ class SettingsMapper extends QBMapper {
 			$settings = $this->insert($settings);
 		}
 
-		$batchTime = 0; // Off
 		if ($batchSetting === Settings::EMAIL_SEND_WEEKLY) {
 			$batchTime = 3600 * 24 * 7;
 		} elseif ($batchSetting === Settings::EMAIL_SEND_DAILY) {
@@ -76,10 +75,14 @@ class SettingsMapper extends QBMapper {
 			$batchTime = 3600 * 3;
 		} elseif ($batchSetting === Settings::EMAIL_SEND_HOURLY) {
 			$batchTime = 3600;
+		} else {
+			$batchTime = 0; // Off
 		}
 
 		$settings->setBatchTime($batchTime);
 		if ($batchTime === 0) {
+			// When mails are Off, we don't set a "next send time" so it can be
+			// skipped in the background job.
 			$settings->setNextSendTime(0);
 		} else {
 			// This will automatically heal on the first run of the background job.
diff --git a/lib/Settings/Personal.php b/lib/Settings/Personal.php
index 38c631c..c598a6f 100644
--- a/lib/Settings/Personal.php
+++ b/lib/Settings/Personal.php
@@ -75,7 +75,6 @@ class Personal implements ISettings {
 		try {
 			$settings = $this->settingsMapper->getSettingsByUser($user->getUID());
 
-			$settingBatchTime = Settings::EMAIL_SEND_OFF;
 			if ($settings->getBatchTime() === 3600 * 24 * 7) {
 				$settingBatchTime = Settings::EMAIL_SEND_WEEKLY;
 			} elseif ($settings->getBatchTime() === 3600 * 24) {
@@ -84,6 +83,8 @@ class Personal implements ISettings {
 				$settingBatchTime = Settings::EMAIL_SEND_3HOURLY;
 			} elseif ($settings->getBatchTime() === 3600) {
 				$settingBatchTime = Settings::EMAIL_SEND_HOURLY;
+			} else {
+				$settingBatchTime = Settings::EMAIL_SEND_OFF;
 			}
 		} catch (DoesNotExistException $e) {
 			$settings = new Settings();