Merge pull request #57079 from Pringels/enhancement/15632/persist-user-management-columns

feat(settings): persist user management column visibility

apps/settings/lib/ConfigLexicon.php

@@ -21,6 +21,12 @@ use OCP\Config\ValueType;
  */
 class ConfigLexicon implements ILexicon {
 	public const USER_SETTINGS_EMAIL = 'email';
+	public const USER_LIST_SHOW_STORAGE_PATH = 'user_list_show_storage_path';
+	public const USER_LIST_SHOW_USER_BACKEND = 'user_list_show_user_backend';
+	public const USER_LIST_SHOW_LAST_LOGIN = 'user_list_show_last_login';
+	public const USER_LIST_SHOW_FIRST_LOGIN = 'user_list_show_first_login';
+	public const USER_LIST_SHOW_NEW_USER_FORM = 'user_list_show_new_user_form';
+	public const USER_LIST_SHOW_LANGUAGES = 'user_list_show_languages';
 
 	public function getStrictness(): Strictness {
 		return Strictness::IGNORE;
@@ -32,7 +38,55 @@ class ConfigLexicon implements ILexicon {
 
 	public function getUserConfigs(): array {
 		return [
-			new Entry(key: self::USER_SETTINGS_EMAIL, type: ValueType::STRING, defaultRaw: '', definition: 'account mail address', flags: IUserConfig::FLAG_INDEXED),
+			new Entry(
+				key: self::USER_SETTINGS_EMAIL,
+				type: ValueType::STRING,
+				defaultRaw: '',
+				definition: 'account mail address',
+				flags: IUserConfig::FLAG_INDEXED,
+			),
+			new Entry(
+				key: self::USER_LIST_SHOW_STORAGE_PATH,
+				type: ValueType::BOOL,
+				defaultRaw: false,
+				definition: 'Show storage path column in user list',
+				lazy: true,
+			),
+			new Entry(
+				key: self::USER_LIST_SHOW_USER_BACKEND,
+				type: ValueType::BOOL,
+				defaultRaw: false,
+				definition: 'Show user account backend column in user list',
+				lazy: true,
+			),
+			new Entry(
+				key: self::USER_LIST_SHOW_LAST_LOGIN,
+				type: ValueType::BOOL,
+				defaultRaw: false,
+				definition: 'Show last login date column in user list',
+				lazy: true,
+			),
+			new Entry(
+				key: self::USER_LIST_SHOW_FIRST_LOGIN,
+				type: ValueType::BOOL,
+				defaultRaw: false,
+				definition: 'Show first login date column in user list',
+				lazy: true,
+			),
+			new Entry(
+				key: self::USER_LIST_SHOW_NEW_USER_FORM,
+				type: ValueType::BOOL,
+				defaultRaw: false,
+				definition: 'Show new user form in user list',
+				lazy: true,
+			),
+			new Entry(
+				key: self::USER_LIST_SHOW_LANGUAGES,
+				type: ValueType::BOOL,
+				defaultRaw: false,
+				definition: 'Show languages in user list',
+				lazy: true,
+			),
 		];
 	}
 }

apps/settings/lib/Controller/UsersController.php

@@ -19,6 +19,7 @@ use OC\KnownUser\KnownUserService;
 use OC\Security\IdentityProof\Manager;
 use OC\User\Manager as UserManager;
 use OCA\Settings\BackgroundJobs\VerifyUserData;
+use OCA\Settings\ConfigLexicon;
 use OCA\Settings\Events\BeforeTemplateRenderedEvent;
 use OCA\Settings\Settings\Admin\Users;
 use OCA\User_LDAP\User_Proxy;
@@ -38,9 +39,11 @@ use OCP\AppFramework\Http\JSONResponse;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
 use OCP\BackgroundJob\IJobList;
+use OCP\Config\IUserConfig;
 use OCP\Encryption\IManager;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Group\ISubAdmin;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IGroup;
 use OCP\IGroupManager;
@@ -59,6 +62,15 @@ class UsersController extends Controller {
 	/** Limit for counting users for subadmins, to avoid spending too much time */
 	private const COUNT_LIMIT_FOR_SUBADMINS = 999;
 
+	public const ALLOWED_USER_PREFERENCES = [
+		ConfigLexicon::USER_LIST_SHOW_STORAGE_PATH,
+		ConfigLexicon::USER_LIST_SHOW_USER_BACKEND,
+		ConfigLexicon::USER_LIST_SHOW_FIRST_LOGIN,
+		ConfigLexicon::USER_LIST_SHOW_LAST_LOGIN,
+		ConfigLexicon::USER_LIST_SHOW_NEW_USER_FORM,
+		ConfigLexicon::USER_LIST_SHOW_LANGUAGES,
+	];
+
 	public function __construct(
 		string $appName,
 		IRequest $request,
@@ -66,6 +78,8 @@ class UsersController extends Controller {
 		private IGroupManager $groupManager,
 		private IUserSession $userSession,
 		private IConfig $config,
+		private IAppConfig $appConfig,
+		private IUserConfig $userConfig,
 		private IL10N $l10n,
 		private IMailer $mailer,
 		private IFactory $l10nFactory,
@@ -191,12 +205,12 @@ class UsersController extends Controller {
 		}
 
 		/* QUOTAS PRESETS */
-		$quotaPreset = $this->parseQuotaPreset($this->config->getAppValue('files', 'quota_preset', '1 GB, 5 GB, 10 GB'));
-		$allowUnlimitedQuota = $this->config->getAppValue('files', 'allow_unlimited_quota', '1') === '1';
+		$quotaPreset = $this->parseQuotaPreset($this->appConfig->getValueString('files', 'quota_preset', '1 GB, 5 GB, 10 GB'));
+		$allowUnlimitedQuota = $this->appConfig->getValueBool('files', 'allow_unlimited_quota', true);
 		if (!$allowUnlimitedQuota && count($quotaPreset) > 0) {
-			$defaultQuota = $this->config->getAppValue('files', 'default_quota', $quotaPreset[0]);
+			$defaultQuota = $this->appConfig->getValueString('files', 'default_quota', $quotaPreset[0]);
 		} else {
-			$defaultQuota = $this->config->getAppValue('files', 'default_quota', 'none');
+			$defaultQuota = $this->appConfig->getValueString('files', 'default_quota', 'none');
 		}
 
 		$event = new BeforeTemplateRenderedEvent();
@@ -219,7 +233,7 @@ class UsersController extends Controller {
 		$serverData['isDelegatedAdmin'] = $isDelegatedAdmin;
 		$serverData['sortGroups'] = $forceSortGroupByName
 			? MetaData::SORT_GROUPNAME
-			: (int)$this->config->getAppValue('core', 'group.sortBy', (string)MetaData::SORT_USERCOUNT);
+			: (int)$this->appConfig->getValueString('core', 'group.sortBy', (string)MetaData::SORT_USERCOUNT);
 		$serverData['forceSortGroupByName'] = $forceSortGroupByName;
 		$serverData['quotaPreset'] = $quotaPreset;
 		$serverData['allowUnlimitedQuota'] = $allowUnlimitedQuota;
@@ -230,9 +244,13 @@ class UsersController extends Controller {
 		// Settings
 		$serverData['defaultQuota'] = $defaultQuota;
 		$serverData['canChangePassword'] = $canChangePassword;
-		$serverData['newUserGenerateUserID'] = $this->config->getAppValue('core', 'newUser.generateUserID', 'no') === 'yes';
-		$serverData['newUserRequireEmail'] = $this->config->getAppValue('core', 'newUser.requireEmail', 'no') === 'yes';
-		$serverData['newUserSendEmail'] = $this->config->getAppValue('core', 'newUser.sendEmail', 'yes') === 'yes';
+		$serverData['newUserGenerateUserID'] = $this->appConfig->getValueBool('core', 'newUser.generateUserID', false);
+		$serverData['newUserRequireEmail'] = $this->appConfig->getValueBool('core', 'newUser.requireEmail', false);
+		$serverData['newUserSendEmail'] = $this->appConfig->getValueBool('core', 'newUser.sendEmail', true);
+		$serverData['showConfig'] = [];
+		foreach (self::ALLOWED_USER_PREFERENCES as $key) {
+			$serverData['showConfig'][$key] = $this->userConfig->getValueBool($uid, $this->appName, $key, false);
+		}
 
 		$this->initialState->provideInitialState('usersSettings', $serverData);
 
@@ -250,13 +268,22 @@ class UsersController extends Controller {
 	 */
 	#[AuthorizedAdminSetting(settings:Users::class)]
 	public function setPreference(string $key, string $value): JSONResponse {
-		$allowed = ['newUser.sendEmail', 'group.sortBy'];
-		if (!in_array($key, $allowed, true)) {
-			return new JSONResponse([], Http::STATUS_FORBIDDEN);
+		switch ($key) {
+			case 'newUser.sendEmail':
+				$this->appConfig->setValueBool('core', $key, $value === 'yes');
+				break;
+			case 'group.sortBy':
+				$this->appConfig->setValueString('core', $key, $value);
+				break;
+			default:
+				if (in_array($key, self::ALLOWED_USER_PREFERENCES, true)) {
+					$this->userConfig->setValueBool($this->userSession->getUser()->getUID(), $this->appName, $key, $value === 'true');
+				} else {
+					return new JSONResponse([], Http::STATUS_FORBIDDEN);
+				}
+				break;
 		}
 
-		$this->config->setAppValue('core', $key, $value);
-
 		return new JSONResponse([]);
 	}
 

apps/settings/src/components/UserList.vue

@@ -313,7 +313,7 @@ export default {
 		},
 
 		closeDialog() {
-			this.$store.commit('setShowConfig', {
+			this.$store.dispatch('setShowConfig', {
 				key: 'showNewUserForm',
 				value: false,
 			})

apps/settings/src/components/Users/UserSettingsDialog.vue

@@ -297,7 +297,7 @@ export default {
 		},
 
 		setShowConfig(key, status) {
-			this.$store.commit('setShowConfig', { key, value: status })
+			this.$store.dispatch('setShowConfig', { key, value: status })
 		},
 
 		/**

apps/settings/src/main-apps-users-management.ts

@@ -30,6 +30,9 @@ sync(store, router)
 
 const pinia = createPinia()
 
+// Migrate legacy local storage settings to the database
+store.dispatch('migrateLocalStorage')
+
 export default new Vue({
 	router,
 	store,

apps/settings/src/store/users.js

@@ -4,7 +4,6 @@
  */
 
 import axios from '@nextcloud/axios'
-import { getBuilder } from '@nextcloud/browser-storage'
 import { getCapabilities } from '@nextcloud/capabilities'
 import { showError } from '@nextcloud/dialogs'
 import { parseFileSize } from '@nextcloud/files'
@@ -17,8 +16,6 @@ import api from './api.js'
 
 const usersSettings = loadState('settings', 'usersSettings', {})
 
-const localStorage = getBuilder('settings').persist(true).build()
-
 const defaults = {
 	/**
 	 * @type {import('../views/user-types').IGroup}
@@ -47,12 +44,12 @@ const state = {
 	disabledUsersLimit: 25,
 	userCount: usersSettings.userCount ?? 0,
 	showConfig: {
-		showStoragePath: localStorage.getItem('account_settings__showStoragePath') === 'true',
-		showUserBackend: localStorage.getItem('account_settings__showUserBackend') === 'true',
-		showFirstLogin: localStorage.getItem('account_settings__showFirstLogin') === 'true',
-		showLastLogin: localStorage.getItem('account_settings__showLastLogin') === 'true',
-		showNewUserForm: localStorage.getItem('account_settings__showNewUserForm') === 'true',
-		showLanguages: localStorage.getItem('account_settings__showLanguages') === 'true',
+		showStoragePath: usersSettings.showConfig?.user_list_show_storage_path,
+		showUserBackend: usersSettings.showConfig?.user_list_show_user_backend,
+		showFirstLogin: usersSettings.showConfig?.user_list_show_first_login,
+		showLastLogin: usersSettings.showConfig?.user_list_show_last_login,
+		showNewUserForm: usersSettings.showConfig?.user_list_show_new_user_form,
+		showLanguages: usersSettings.showConfig?.user_list_show_languages,
 	},
 }
 
@@ -241,7 +238,6 @@ const mutations = {
 	},
 
 	setShowConfig(state, { key, value }) {
-		localStorage.setItem(`account_settings__${key}`, JSON.stringify(value))
 		state.showConfig[key] = value
 	},
 
@@ -801,6 +797,68 @@ const actions = {
 				.catch((error) => { throw error })
 		}).catch((error) => context.commit('API_FAILURE', { userid, error }))
 	},
+	/**
+	 * Migrate local storage keys to database
+	 *
+	 * @param {object} context store context
+	 * @param context.commit
+	 */
+	migrateLocalStorage({ commit }) {
+		const preferences = {
+			showStoragePath: 'user_list_show_storage_path',
+			showUserBackend: 'user_list_show_user_backend',
+			showFirstLogin: 'user_list_show_first_login',
+			showLastLogin: 'user_list_show_last_login',
+			showNewUserForm: 'user_list_show_new_user_form',
+			showLanguages: 'user_list_show_languages',
+		}
+
+		for (const [key, dbKey] of Object.entries(preferences)) {
+			const localKey = `account_settings__${key}`
+			const localValue = window.localStorage.getItem(localKey)
+			if (localValue === null) {
+				continue
+			}
+
+			const value = localValue === 'true'
+			commit('setShowConfig', { key, value })
+
+			axios.post(generateUrl(`/settings/users/preferences/${dbKey}`), {
+				value: value ? 'true' : 'false',
+			}).then(() => {
+				window.localStorage.removeItem(localKey)
+			}).catch((error) => {
+				logger.error(`Failed to migrate preference ${key}`, { error })
+			})
+		}
+	},
+
+	/**
+	 * Set show config
+	 *
+	 * @param {object} context store context
+	 * @param {object} options destructuring object
+	 * @param {string} options.key Key to set
+	 * @param {boolean} options.value Value to set
+	 */
+	setShowConfig(context, { key, value }) {
+		context.commit('setShowConfig', { key, value })
+		const keyMap = {
+			showStoragePath: 'user_list_show_storage_path',
+			showUserBackend: 'user_list_show_user_backend',
+			showFirstLogin: 'user_list_show_first_login',
+			showLastLogin: 'user_list_show_last_login',
+			showNewUserForm: 'user_list_show_new_user_form',
+			showLanguages: 'user_list_show_languages',
+		}
+		axios.post(generateUrl(`settings/users/preferences/${keyMap[key]}`), { value: value ? 'true' : 'false' })
+			.catch((error) => logger.error(`Could not update ${key} preference`, { error }))
+	},
 }
 
-export default { state, mutations, getters, actions }
+export default {
+	state,
+	mutations,
+	getters,
+	actions,
+}

apps/settings/src/views/UserManagementNavigation.vue

@@ -149,7 +149,7 @@ const isAdminOrDelegatedAdmin = computed(() => settings.value.isAdmin || setting
  * Open the new-user form dialog
  */
 function showNewUserMenu() {
-	store.commit('setShowConfig', {
+	store.dispatch('setShowConfig', {
 		key: 'showNewUserForm',
 		value: true,
 	})

apps/settings/tests/Controller/UsersControllerTest.php

@@ -14,6 +14,7 @@ use OC\ForbiddenException;
 use OC\Group\Manager;
 use OC\KnownUser\KnownUserService;
 use OC\User\Manager as UserManager;
+use OCA\Settings\ConfigLexicon;
 use OCA\Settings\Controller\UsersController;
 use OCP\Accounts\IAccount;
 use OCP\Accounts\IAccountManager;
@@ -23,9 +24,11 @@ use OCP\App\IAppManager;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Services\IInitialState;
 use OCP\BackgroundJob\IJobList;
+use OCP\Config\IUserConfig;
 use OCP\Encryption\IEncryptionModule;
 use OCP\Encryption\IManager;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IL10N;
@@ -45,6 +48,8 @@ class UsersControllerTest extends \Test\TestCase {
 	private UserManager&MockObject $userManager;
 	private IUserSession&MockObject $userSession;
 	private IConfig&MockObject $config;
+	private IAppConfig&MockObject $appConfig;
+	private IUserConfig&MockObject $userConfig;
 	private IMailer&MockObject $mailer;
 	private IFactory&MockObject $l10nFactory;
 	private IAppManager&MockObject $appManager;
@@ -65,6 +70,8 @@ class UsersControllerTest extends \Test\TestCase {
 		$this->groupManager = $this->createMock(Manager::class);
 		$this->userSession = $this->createMock(IUserSession::class);
 		$this->config = $this->createMock(IConfig::class);
+		$this->appConfig = $this->createMock(IAppConfig::class);
+		$this->userConfig = $this->createMock(IUserConfig::class);
 		$this->l = $this->createMock(IL10N::class);
 		$this->mailer = $this->createMock(IMailer::class);
 		$this->l10nFactory = $this->createMock(IFactory::class);
@@ -106,6 +113,8 @@ class UsersControllerTest extends \Test\TestCase {
 				$this->groupManager,
 				$this->userSession,
 				$this->config,
+				$this->appConfig,
+				$this->userConfig,
 				$this->l,
 				$this->mailer,
 				$this->l10nFactory,
@@ -128,6 +137,8 @@ class UsersControllerTest extends \Test\TestCase {
 						$this->groupManager,
 						$this->userSession,
 						$this->config,
+						$this->appConfig,
+						$this->userConfig,
 						$this->l,
 						$this->mailer,
 						$this->l10nFactory,
@@ -992,4 +1003,56 @@ class UsersControllerTest extends \Test\TestCase {
 			[false, false, false, true],
 		];
 	}
+	#[\PHPUnit\Framework\Attributes\DataProvider('dataSetPreference')]
+	public function testSetPreference(string $key, string $value, bool $isUserValue, bool $isAppValue, int $expectedStatus): void {
+		$controller = $this->getController(false, []);
+		$user = $this->createMock(IUser::class);
+		$user->method('getUID')->willReturn('testUser');
+		$this->userSession->method('getUser')->willReturn($user);
+
+		if ($isAppValue) {
+			if ($value === 'true' || $value === 'false' || $value === 'yes' || $value === 'no') {
+				$this->appConfig->expects($this->once())
+					->method('setValueBool')
+					->with('core', $key, $value === 'yes' || $value === 'true');
+			} else {
+				$this->appConfig->expects($this->once())
+					->method('setValueString')
+					->with('core', $key, $value);
+			}
+			$this->userConfig->expects($this->never())
+				->method('setValueBool');
+		} elseif ($isUserValue) {
+			$this->userConfig->expects($this->once())
+				->method('setValueBool')
+				->with('testUser', 'settings', $key, $value === 'true');
+			$this->appConfig->expects($this->never())
+				->method('setValueString');
+			$this->appConfig->expects($this->never())
+				->method('setValueBool');
+		} else {
+			$this->appConfig->expects($this->never())->method('setValueString');
+			$this->appConfig->expects($this->never())->method('setValueBool');
+			$this->userConfig->expects($this->never())->method('setValueString');
+			$this->userConfig->expects($this->never())->method('setValueBool');
+		}
+
+		$response = $controller->setPreference($key, $value);
+		$this->assertEquals($expectedStatus, $response->getStatus());
+	}
+
+	public static function dataSetPreference(): array {
+		return [
+			['newUser.sendEmail', 'yes', false, true, Http::STATUS_OK],
+			['newUser.sendEmail', 'no', false, true, Http::STATUS_OK],
+			['group.sortBy', '1', false, true, Http::STATUS_OK],
+			[ConfigLexicon::USER_LIST_SHOW_STORAGE_PATH, 'true', true, false, Http::STATUS_OK],
+			[ConfigLexicon::USER_LIST_SHOW_USER_BACKEND, 'false', true, false, Http::STATUS_OK],
+			[ConfigLexicon::USER_LIST_SHOW_FIRST_LOGIN, 'true', true, false, Http::STATUS_OK],
+			[ConfigLexicon::USER_LIST_SHOW_LAST_LOGIN, 'true', true, false, Http::STATUS_OK],
+			[ConfigLexicon::USER_LIST_SHOW_NEW_USER_FORM, 'true', true, false, Http::STATUS_OK],
+			[ConfigLexicon::USER_LIST_SHOW_LANGUAGES, 'true', true, false, Http::STATUS_OK],
+			['invalidKey', 'value', false, false, Http::STATUS_FORBIDDEN],
+		];
+	}
 }

build/psalm-baseline.xml

@@ -2392,15 +2392,6 @@
     </DeprecatedConstant>
     <DeprecatedMethod>
       <code><![CDATA[dispatch]]></code>
-      <code><![CDATA[getAppValue]]></code>
-      <code><![CDATA[getAppValue]]></code>
-      <code><![CDATA[getAppValue]]></code>
-      <code><![CDATA[getAppValue]]></code>
-      <code><![CDATA[getAppValue]]></code>
-      <code><![CDATA[getAppValue]]></code>
-      <code><![CDATA[getAppValue]]></code>
-      <code><![CDATA[getAppValue]]></code>
-      <code><![CDATA[setAppValue]]></code>
       <code><![CDATA[validateMailAddress]]></code>
     </DeprecatedMethod>
   </file>

cypress/e2e/settings/users_columns.cy.ts

@@ -59,6 +59,11 @@ describe('Settings: Show and hide columns', function() {
 		getUserList().find('tbody tr').each(($row) => {
 			cy.wrap($row).get('[data-cy-user-list-cell-language]').should('exist')
 		})
+
+		// Clear local storage and reload to verify user settings DB persistence
+		cy.clearLocalStorage()
+		cy.reload()
+		cy.get('[data-cy-user-list-header-languages]').should('exist')
 	})
 
 	it('Can hide a column', function() {