Skip to content

Commit 083b80e

Browse files
PringelsPringels
committed
feat(settings): persist user management column visibility
Previously, column visibility settings were stored in localStorage, causing them to be lost when logging out or switching browsers. This change moves the persistence to the database as user preferences. It also refactors the frontend to use clean `userList.*` keys for better consistency between the store and the API. Signed-off-by: Peter Ringelmann <[email protected]>
1 parent 0edaff9 commit 083b80e

File tree

7 files changed

+94
-18
lines changed

7 files changed

+94
-18
lines changed

‎apps/settings/lib/Controller/UsersController.php‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,15 @@ class UsersController extends Controller {
5959
/** Limit for counting users for subadmins, to avoid spending too much time */
6060
private const COUNT_LIMIT_FOR_SUBADMINS = 999;
6161

62+
private const ALLOWED_USER_PREFERENCES = [
63+
'userList.showStoragePath',
64+
'userList.showUserBackend',
65+
'userList.showFirstLogin',
66+
'userList.showLastLogin',
67+
'userList.showNewUserForm',
68+
'userList.showLanguages',
69+
];
70+
6271
public function __construct(
6372
string $appName,
6473
IRequest $request,
@@ -233,6 +242,10 @@ public function usersList(INavigationManager $navigationManager, ISubAdmin $subA
233242
$serverData['newUserGenerateUserID'] = $this->config->getAppValue('core', 'newUser.generateUserID', 'no') === 'yes';
234243
$serverData['newUserRequireEmail'] = $this->config->getAppValue('core', 'newUser.requireEmail', 'no') === 'yes';
235244
$serverData['newUserSendEmail'] = $this->config->getAppValue('core', 'newUser.sendEmail', 'yes') === 'yes';
245+
$serverData['showConfig'] = [];
246+
foreach (self::ALLOWED_USER_PREFERENCES as $key) {
247+
$serverData['showConfig'][$key] = $this->config->getUserValue($uid, $this->appName, $key, 'false') === 'true';
248+
}
236249

237250
$this->initialState->provideInitialState('usersSettings', $serverData);
238251

@@ -250,13 +263,16 @@ public function usersList(INavigationManager $navigationManager, ISubAdmin $subA
250263
*/
251264
#[AuthorizedAdminSetting(settings:Users::class)]
252265
public function setPreference(string $key, string $value): JSONResponse {
253-
$allowed = ['newUser.sendEmail', 'group.sortBy'];
254-
if (!in_array($key, $allowed, true)) {
266+
$allowedAppValues = ['newUser.sendEmail', 'group.sortBy'];
267+
268+
if (in_array($key, $allowedAppValues, true)) {
269+
$this->config->setAppValue('core', $key, $value);
270+
} elseif (in_array($key, self::ALLOWED_USER_PREFERENCES, true)) {
271+
$this->config->setUserValue($this->userSession->getUser()->getUID(), $this->appName, $key, $value);
272+
} else {
255273
return new JSONResponse([], Http::STATUS_FORBIDDEN);
256274
}
257275

258-
$this->config->setAppValue('core', $key, $value);
259-
260276
return new JSONResponse([]);
261277
}
262278

‎apps/settings/src/components/UserList.vue‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ export default {
313313
},
314314
315315
closeDialog() {
316-
this.$store.commit('setShowConfig', {
316+
this.$store.dispatch('setShowConfig', {
317317
key: 'showNewUserForm',
318318
value: false,
319319
})

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ export default {
297297
},
298298
299299
setShowConfig(key, status) {
300-
this.$store.commit('setShowConfig', { key, value: status })
300+
this.$store.dispatch('setShowConfig', { key, value: status })
301301
},
302302
303303
/**

‎apps/settings/src/store/users.js‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
*/
55

66
import axios from '@nextcloud/axios'
7-
import { getBuilder } from '@nextcloud/browser-storage'
87
import { getCapabilities } from '@nextcloud/capabilities'
98
import { showError } from '@nextcloud/dialogs'
109
import { parseFileSize } from '@nextcloud/files'
@@ -17,8 +16,6 @@ import api from './api.js'
1716

1817
const usersSettings = loadState('settings', 'usersSettings', {})
1918

20-
const localStorage = getBuilder('settings').persist(true).build()
21-
2219
const defaults = {
2320
/**
2421
* @type {import('../views/user-types').IGroup}
@@ -47,12 +44,12 @@ const state = {
4744
disabledUsersLimit: 25,
4845
userCount: usersSettings.userCount ?? 0,
4946
showConfig: {
50-
showStoragePath: localStorage.getItem('account_settings__showStoragePath') === 'true',
51-
showUserBackend: localStorage.getItem('account_settings__showUserBackend') === 'true',
52-
showFirstLogin: localStorage.getItem('account_settings__showFirstLogin') === 'true',
53-
showLastLogin: localStorage.getItem('account_settings__showLastLogin') === 'true',
54-
showNewUserForm: localStorage.getItem('account_settings__showNewUserForm') === 'true',
55-
showLanguages: localStorage.getItem('account_settings__showLanguages') === 'true',
47+
showStoragePath: usersSettings.showConfig['userList.showStoragePath'],
48+
showUserBackend: usersSettings.showConfig['userList.showUserBackend'],
49+
showFirstLogin: usersSettings.showConfig['userList.showFirstLogin'],
50+
showLastLogin: usersSettings.showConfig['userList.showLastLogin'],
51+
showNewUserForm: usersSettings.showConfig['userList.showNewUserForm'],
52+
showLanguages: usersSettings.showConfig['userList.showLanguages'],
5653
},
5754
}
5855

@@ -241,7 +238,6 @@ const mutations = {
241238
},
242239

243240
setShowConfig(state, { key, value }) {
244-
localStorage.setItem(`account_settings__${key}`, JSON.stringify(value))
245241
state.showConfig[key] = value
246242
},
247243

@@ -801,6 +797,24 @@ const actions = {
801797
.catch((error) => { throw error })
802798
}).catch((error) => context.commit('API_FAILURE', { userid, error }))
803799
},
800+
/**
801+
* Set show config
802+
*
803+
* @param {object} context store context
804+
* @param {object} options destructuring object
805+
* @param {string} options.key Key to set
806+
* @param {boolean} options.value Value to set
807+
*/
808+
setShowConfig(context, { key, value }) {
809+
context.commit('setShowConfig', { key, value })
810+
axios.post(generateUrl(`settings/users/preferences/userList.${key}`), { value: value ? 'true' : 'false' })
811+
.catch((error) => logger.error(`Could not update ${key} preference`, { error }))
812+
},
804813
}
805814

806-
export default { state, mutations, getters, actions }
815+
export default {
816+
state,
817+
mutations,
818+
getters,
819+
actions,
820+
}

‎apps/settings/src/views/UserManagementNavigation.vue‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ const isAdminOrDelegatedAdmin = computed(() => settings.value.isAdmin || setting
149149
* Open the new-user form dialog
150150
*/
151151
function showNewUserMenu() {
152-
store.commit('setShowConfig', {
152+
store.dispatch('setShowConfig', {
153153
key: 'showNewUserForm',
154154
value: true,
155155
})

‎apps/settings/tests/Controller/UsersControllerTest.php‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -992,4 +992,45 @@ public static function dataTestCanAdminChangeUserPasswords(): array {
992992
[false, false, false, true],
993993
];
994994
}
995+
#[\PHPUnit\Framework\Attributes\DataProvider('dataSetPreference')]
996+
public function testSetPreference(string $key, string $value, bool $isUserValue, bool $isAppValue, int $expectedStatus): void {
997+
$controller = $this->getController(false, []);
998+
$user = $this->createMock(IUser::class);
999+
$user->method('getUID')->willReturn('testUser');
1000+
$this->userSession->method('getUser')->willReturn($user);
1001+
1002+
if ($isAppValue) {
1003+
$this->config->expects($this->once())
1004+
->method('setAppValue')
1005+
->with('core', $key, $value);
1006+
$this->config->expects($this->never())
1007+
->method('setUserValue');
1008+
} elseif ($isUserValue) {
1009+
$this->config->expects($this->once())
1010+
->method('setUserValue')
1011+
->with('testUser', 'settings', $key, $value);
1012+
$this->config->expects($this->never())
1013+
->method('setAppValue');
1014+
} else {
1015+
$this->config->expects($this->never())->method('setAppValue');
1016+
$this->config->expects($this->never())->method('setUserValue');
1017+
}
1018+
1019+
$response = $controller->setPreference($key, $value);
1020+
$this->assertEquals($expectedStatus, $response->getStatus());
1021+
}
1022+
1023+
public static function dataSetPreference(): array {
1024+
return [
1025+
['newUser.sendEmail', 'yes', false, true, Http::STATUS_OK],
1026+
['group.sortBy', '1', false, true, Http::STATUS_OK],
1027+
['userList.showStoragePath', 'true', true, false, Http::STATUS_OK],
1028+
['userList.showUserBackend', 'false', true, false, Http::STATUS_OK],
1029+
['userList.showFirstLogin', 'true', true, false, Http::STATUS_OK],
1030+
['userList.showLastLogin', 'true', true, false, Http::STATUS_OK],
1031+
['userList.showNewUserForm', 'true', true, false, Http::STATUS_OK],
1032+
['userList.showLanguages', 'true', true, false, Http::STATUS_OK],
1033+
['invalidKey', 'value', false, false, Http::STATUS_FORBIDDEN],
1034+
];
1035+
}
9951036
}

‎cypress/e2e/settings/users_columns.cy.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ describe('Settings: Show and hide columns', function() {
5959
getUserList().find('tbody tr').each(($row) => {
6060
cy.wrap($row).get('[data-cy-user-list-cell-language]').should('exist')
6161
})
62+
63+
// Clear local storage and reload to verify user settings DB persistence
64+
cy.clearLocalStorage()
65+
cy.reload()
66+
cy.get('[data-cy-user-list-header-languages]').should('exist')
6267
})
6368

6469
it('Can hide a column', function() {

0 commit comments

Comments
 (0)