Merge pull request #2982 from nextcloud/fix/preferences

Fix preferences

CHANGELOG.md

@@ -6,6 +6,9 @@ All notable changes to this project will be documented in this file.
  - Fix date shifting and sequence problems with daylight saving times
  - Bring back notifications
  - fix notification subscription
+ - Eliminate preferences error logging on first time usage
+### Changes
+ - Set default view to table layout even for text polls, list layout is still the default layout when in mobile mode
 ## [5.1.0] - 2023-06-27
 ### Changes
  - Added user option to remove outdated polls from th "Relevant" list

lib/Controller/PreferencesController.php

@@ -38,7 +38,7 @@ public function __construct(
 		ISession $session,
 		private PreferencesService $preferencesService,
 		private CalendarService $calendarService,
-		private IUserSession $userSession
+		private IUserSession $userSession,
 	) {
 		parent::__construct($appName, $request, $session);
 	}
@@ -56,11 +56,11 @@ public function get(): JSONResponse {
 	 * Write preferences
 	 * @NoAdminRequired
 	 */
-	public function write(array $settings): JSONResponse {
+	public function write(array $preferences): JSONResponse {
 		if (!$this->userSession->isLoggedIn()) {
 			return new JSONResponse([], Http::STATUS_OK);
 		}
-		return $this->response(fn () => $this->preferencesService->write($settings));
+		return $this->response(fn () => $this->preferencesService->write($preferences));
 	}
 
 	/**

lib/Cron/UserDeletedJob.php

@@ -61,9 +61,9 @@ public function __construct(
 	 * @return void
 	 */
 	protected function run($argument) {
-		$owner = $argument['owner'];
-		$this->logger->info('Deleting polls for deleted user {user}', [
-			'user' => $owner
+		$userId = $argument['userId'];
+		$this->logger->info('Deleting polls for deleted user id {user}', [
+			'user' => $userId
 		]);
 
 		$replacementName = 'deleted_' . $this->secureRandom->generate(
@@ -73,13 +73,13 @@ protected function run($argument) {
 			ISecureRandom::CHAR_UPPER
 		);
 
-		$this->pollMapper->deleteByUserId($owner);
-		$this->logMapper->deleteByUserId($owner);
-		$this->shareMapper->deleteByIdAndType($owner, Share::TYPE_USER);
-		$this->preferencesMapper->deleteByUserId($owner);
-		$this->subscriptionMapper->deleteByUserId($owner);
-		$this->commentMapper->renameUserId($owner, $replacementName);
-		$this->optionMapper->renameUserId($owner, $replacementName);
-		$this->voteMapper->renameUserId($owner, $replacementName);
+		$this->pollMapper->deleteByUserId($userId);
+		$this->logMapper->deleteByUserId($userId);
+		$this->shareMapper->deleteByIdAndType($userId, Share::TYPE_USER);
+		$this->preferencesMapper->deleteByUserId($userId);
+		$this->subscriptionMapper->deleteByUserId($userId);
+		$this->commentMapper->renameUserId($userId, $replacementName);
+		$this->optionMapper->renameUserId($userId, $replacementName);
+		$this->voteMapper->renameUserId($userId, $replacementName);
 	}
 }

lib/Db/Preferences.php

@@ -42,7 +42,7 @@ class Preferences extends Entity implements JsonSerializable {
 	public const DEFAULT = [
 		'useCommentsAlternativeStyling' => false,
 		'useAlternativeStyling' => false,
-		'calendarPeek' => true,
+		'calendarPeek' => false,
 		'checkCalendars' => [],
 		'checkCalendarsBefore' => 0,
 		'checkCalendarsAfter' => 0,
@@ -60,6 +60,9 @@ class Preferences extends Entity implements JsonSerializable {
 
 	public function __construct() {
 		$this->addType('timestamp', 'int');
+
+		// initialize with default values
+		$this->setPreferences(json_encode(self::DEFAULT));
 	}
 
 	public function getPreferences_decoded() {
@@ -102,9 +105,6 @@ public function getCheckCalendarsAfter(): int {
 	 */
 	public function jsonSerialize(): array {
 		return [
-			'id' => $this->getId(),
-			'userId' => $this->getUserId(),
-			'timestamp' => $this->getTimestamp(),
 			'preferences' => json_decode($this->preferences) ?? '',
 		];
 	}

lib/Db/PreferencesMapper.php

@@ -43,7 +43,7 @@ public function __construct(IDBConnection $db) {
 	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException if more than one result
 	 * @return Preferences
 	 */
-	public function find(string $userId): Preferences {
+	public function find(?string $userId): Preferences {
 		$qb = $this->db->getQueryBuilder();
 
 		$qb->select('*')

lib/Db/TableManager.php

@@ -42,11 +42,7 @@ public function __construct(
 		private IConfig $config,
 		private IDBConnection $connection,
 		private LoggerInterface $logger,
-		private LogMapper $logMapper,
 		private OptionMapper $optionMapper,
-		private PreferencesMapper $preferencesMapper,
-		private ShareMapper $shareMapper,
-		private SubscriptionMapper $subscriptionMapper,
 		private VoteMapper $voteMapper,
 		private WatchMapper $watchMapper,
 	) {

lib/Listener/UserDeletedListener.php

@@ -40,6 +40,6 @@ protected function addCronJob() : void {
 		if (!($this->event instanceof UserDeletedEvent)) {
 			return;
 		}
-		$this->jobList->add(UserDeletedJob::class, ['owner' => $this->event->getUser()->getUID()]);
+		$this->jobList->add(UserDeletedJob::class, ['userId' => $this->event->getUser()->getUID()]);
 	}
 }

lib/Migration/RepairSteps/DeleteInvalidRecords.php

@@ -24,16 +24,9 @@
 
 namespace OCA\Polls\Migration\RepairSteps;
 
-use OCA\Polls\Db\LogMapper;
-use OCA\Polls\Db\OptionMapper;
 use OCA\Polls\Db\Poll;
-use OCA\Polls\Db\PreferencesMapper;
-use OCA\Polls\Db\ShareMapper;
-use OCA\Polls\Db\SubscriptionMapper;
 use OCA\Polls\Db\TableManager;
-use OCA\Polls\Db\VoteMapper;
 use OCA\Polls\Db\WatchMapper;
-use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\Migration\IOutput;
 use OCP\Migration\IRepairStep;
@@ -44,14 +37,7 @@
  */
 class DeleteInvalidRecords implements IRepairStep {
 	public function __construct(
-		private IConfig $config,
 		private IDBConnection $connection,
-		private LogMapper $logMapper,
-		private OptionMapper $optionMapper,
-		private PreferencesMapper $preferencesMapper,
-		private ShareMapper $shareMapper,
-		private SubscriptionMapper $subscriptionMapper,
-		private VoteMapper $voteMapper,
 		private WatchMapper $watchMapper,
 		private TableManager $tableManager
 	) {

lib/Service/CalendarService.php

@@ -40,14 +40,14 @@
 class CalendarService {
 	/** @var ICalendar[] */
 	private array $calendars;
-	private Preferences $preferences;
-
+
 	public function __construct(
 		private CalendarManager $calendarManager,
+		private CurrentUser $currentUser,
 		private ISession $session,
-		private PreferencesService $preferencesService,
 		private OptionMapper $optionMapper,
-		private CurrentUser $currentUser,
+		private Preferences $preferences,
+		private PreferencesService $preferencesService,
 	) {
 		$this->preferences = $this->preferencesService->get();
 		$this->getCalendarsForPrincipal();

lib/Service/PollService.php

@@ -27,7 +27,6 @@
 use OCA\Polls\Db\Poll;
 use OCA\Polls\Db\PollMapper;
 use OCA\Polls\Db\Preferences;
-use OCA\Polls\Db\PreferencesMapper;
 use OCA\Polls\Db\VoteMapper;
 use OCA\Polls\Event\PollArchivedEvent;
 use OCA\Polls\Event\PollCreatedEvent;
@@ -67,13 +66,11 @@ public function __construct(
 		private Poll $poll,
 		private PollMapper $pollMapper,
 		private Preferences $preferences,
-		private PreferencesMapper $preferencesMapper,
+		private PreferencesService $preferencesService,
 		private VoteMapper $voteMapper,
 	) {
 		$this->userId = $this->userSession->getUser()?->getUID() ?? '';
-		if ($this->userId) {
-			$this->preferences = $this->preferencesMapper->find($this->userId);
-		}
+		$this->preferences = $this->preferencesService->get();
 	}
 
 	/**

lib/Service/PreferencesService.php

@@ -30,51 +30,49 @@
 use OCP\IUserSession;
 
 class PreferencesService {
-	private Preferences $preferences;
-	private string $userId;
-
+	private ?string $userId = null;
+
 	public function __construct(
-		private PreferencesMapper $preferencesMapper,
 		private IUserSession $userSession,
+		private PreferencesMapper $preferencesMapper,
+		private Preferences $preferences,
 	) {
-		$this->userId = $this->userSession->getUser()?->getUID() ?? '';
-		$this->preferences = new Preferences;
+		$this->userId = $this->userSession->getUser()?->getUID();
 		$this->load();
 	}
 
-	public function load(): void {
+	public function load(?string $userId = null): Preferences {
 		try {
-			$this->preferences = $this->preferencesMapper->find($this->userId);
-		} catch (DoesNotExistException $e) {
-			if ($this->userId) {
-				$this->preferences = new Preferences();
-				$this->preferences->setUserId($this->userId);
-				$this->preferences->setTimestamp(time());
-				$this->preferences->setPreferences(json_encode(Preferences::DEFAULT));
-				$this->preferences = $this->preferencesMapper->insert($this->preferences);
-			} else {
-				throw new NotAuthorizedException;
-			}
+			$this->preferences = $this->preferencesMapper->find($userId ?? $this->userId);
+		} catch	(DoesNotExistException $e) {
+			$this->preferences = new Preferences;
 		}
+
+		return $this->preferences;
 	}
 
-	/**
-	 * Read all preferences
-	 */
 	public function get(): Preferences {
 		return $this->preferences;
 	}
-
+	
 	/**
 	 * Write references
 	 */
-	public function write(array $settings): Preferences {
+	public function write(array $preferences): Preferences {
 		if (!$this->userId) {
-			throw new NotAuthorizedException;
+			throw new NotAuthorizedException();
 		}
 
-		$this->preferences->setPreferences(json_encode($settings));
+		$this->preferences->setPreferences(json_encode($preferences));
 		$this->preferences->setTimestamp(time());
-		return $this->preferencesMapper->update($this->preferences);
+		$this->preferences->setUserId($this->userId);
+
+		if ($this->preferences->getId() > 0) {
+			return $this->preferencesMapper->update($this->preferences);
+		} else {
+			return $this->preferencesMapper->insert($this->preferences);
+
+		}
+
 	}
 }

src/js/Api/userSettings.js

@@ -32,11 +32,11 @@ const userSettings = {
 		})
 	},
 
-	writeUserSettings(settings) {
+	writeUserSettings(preferences) {
 		return httpInstance.request({
 			method: 'POST',
 			url: 'preferences',
-			data: { settings },
+			data: { preferences },
 			cancelToken: cancelTokenHandlerObject[this.writeUserSettings.name].handleRequestCancellation().token,
 		})
 	},

src/js/store/modules/settings.js

@@ -32,7 +32,7 @@ const defaultSettings = () => ({
 		checkCalendars: [],
 		checkCalendarsBefore: 0,
 		checkCalendarsAfter: 0,
-		defaultViewTextPoll: 'list-view',
+		defaultViewTextPoll: 'table-view',
 		defaultViewDatePoll: 'table-view',
 		performanceThreshold: 1000,
 		pollCombo: [],
@@ -58,20 +58,6 @@ const mutations = {
 	},
 
 	setPreference(state, payload) {
-		// change values in case of old settings
-		if (payload.defaultViewTextPoll === 'desktop') {
-			payload.defaultViewTextPoll = 'table-view'
-		}
-		if (payload.defaultViewTextPoll === 'mobile') {
-			payload.defaultViewTextPoll = 'list-view'
-		}
-		if (payload.defaultViewDatePoll === 'desktop') {
-			payload.defaultViewDatePoll = 'table-view'
-		}
-		if (payload.defaultViewDatePoll === 'mobile') {
-			payload.defaultViewDatePoll = 'list-view'
-		}
-
 		Object.keys(payload).filter((key) => key in state.user).forEach((key) => {
 			state.user[key] = payload[key]
 		})
@@ -125,18 +111,6 @@ const actions = {
 	async get(context) {
 		try {
 			const response = await UserSettingsAPI.getUserSettings()
-			if (response.data.preferences.defaultViewTextPoll === 'desktop') {
-				response.data.preferences.defaultViewTextPoll = 'table-view'
-			}
-			if (response.data.preferences.defaultViewTextPoll === 'mobile') {
-				response.data.preferences.defaultViewTextPoll = 'list-view'
-			}
-			if (response.data.preferences.defaultViewDatePoll === 'desktop') {
-				response.data.preferences.defaultViewDatePoll = 'table-view'
-			}
-			if (response.data.preferences.defaultViewDatePoll === 'mobile') {
-				response.data.preferences.defaultViewDatePoll = 'list-view'
-			}
 			context.commit('setPreference', response.data.preferences)
 		} catch (e) {
 			if (e?.code === 'ERR_CANCELED') return