Persist the config changes of usermanagement #23333

sharidas · 2020-10-10T10:33:28Z

Persist the changes made to usermanagemnt like:

Show Languages
Show last login
Show user backend
Show storage path

Signed-off-by: Sujith Haridasan sujith.h@gmail.com

sharidas · 2020-10-10T10:43:26Z

Summary

In the user management page, the settings to show languages, last login etc were not saved in the db.

Description

The idea behind this PR is to save the changes made to settings in the user management page for:

Show Languages
Show last login
Show user backend
Show store path

The only option that was being saved was Send email to new user. And hence prior to this change set the changes made by the user were lost after relogin. This change set however would help the user regain the configuration which was set.

How was it tested?

Create admin user
Enable/Disable any of the options like Show Languages, Show last login, Show user backend, Show storage path
Relogin again to see if these options are same as before.
It worked as expected.

apps/settings/src/views/Users.vue

sharidas · 2020-10-12T13:24:35Z

make build-js-production looks clean in my instance. The files mentioned in Node / 12.x github action are already updated in the PR. Not sure what is causing the failure for Node / 12.x...

skjnldsv · 2020-10-12T13:28:59Z

make build-js-production looks clean in my instance. The files mentioned in Node / 12.x github action are already updated in the PR. Not sure what is causing the failure for Node / 12.x...

do you npm install or npm ci ?
You need to use npm ci

sharidas · 2020-10-12T13:36:47Z

make build-js-production looks clean in my instance. The files mentioned in Node / 12.x github action are already updated in the PR. Not sure what is causing the failure for Node / 12.x...

do you npm install or npm ci ?
You need to use npm ci

Yah I did. Here is the log captured from my console:

nextcloud2 git:(save-config-user-management) ✗ npm ci       
npm WARN prepare removing existing node_modules/ before installation

> fsevents@1.2.13 install /home/sujith/test/nextcloud2/node_modules/fsevents
> node install.js


Skipping 'fsevents' build as platform linux is not supported

> core-js@3.6.5 postinstall /home/sujith/test/nextcloud2/node_modules/core-js
> node -e "try{require('./postinstall')}catch(e){}"

Thank you for using core-js ( https://github.com/zloirock/core-js ) for polyfilling JavaScript standard library!

The project needs your help! Please consider supporting of core-js on Open Collective or Patreon: 
> https://opencollective.com/core-js 
> https://www.patreon.com/zloirock 

Also, the author of core-js ( https://github.com/zloirock ) is looking for a good job -)


> core-js@2.6.11 postinstall /home/sujith/test/nextcloud2/node_modules/@babel/polyfill/node_modules/core-js
> node -e "try{require('./postinstall')}catch(e){}"


> core-js@3.6.4 postinstall /home/sujith/test/nextcloud2/node_modules/@nextcloud/moment/node_modules/core-js
> node -e "try{require('./postinstall')}catch(e){}"


> core-js@2.6.11 postinstall /home/sujith/test/nextcloud2/node_modules/babel-runtime/node_modules/core-js
> node -e "try{require('./postinstall')}catch(e){}"


> node-sass@4.14.1 install /home/sujith/test/nextcloud2/node_modules/node-sass
> node scripts/install.js

Cached binary found at /home/sujith/.npm/_cacache/node-sass/4.14.1/linux-x64-72_binding.node

> node-sass@4.14.1 postinstall /home/sujith/test/nextcloud2/node_modules/node-sass
> node scripts/build.js

Binary found at /home/sujith/test/nextcloud2/node_modules/node-sass/vendor/linux-x64-72/binding.node
Testing binary
Binary is fine

> core-js@2.6.11 postinstall /home/sujith/test/nextcloud2/node_modules/@babel/runtime-corejs2/node_modules/core-js
> node -e "try{require('./postinstall')}catch(e){}"


> nodent-runtime@3.2.1 install /home/sujith/test/nextcloud2/node_modules/nodent-runtime
> node build.js

## Built /home/sujith/test/nextcloud2/node_modules/nodent-runtime/dist/index.js
added 1157 packages in 9.404s
➜  nextcloud2 git:(save-config-user-management) ✗

apps/settings/lib/Controller/UsersController.php

Persist the changes made to usermanagemnt like: - Show Languages - Show last login - Show user backend - Show storage path Signed-off-by: Sujith Haridasan <sujith.h@gmail.com>

skjnldsv · 2020-11-03T16:14:32Z

What is the status here? :)

sharidas · 2020-11-04T14:31:26Z

What is the status here? :)

I have updated the changes requested :) The conflicts need to be resolved in the branch...

skjnldsv · 2020-11-04T14:33:02Z

Please rebase to master :)
You can drop the compiled files conflicts

juliusknorr · 2020-11-04T15:12:09Z

apps/settings/lib/Controller/UsersController.php

@@ -260,6 +260,10 @@ public function usersList() {
 		$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['showLastLogin'] = $this->config->getAppValue('core', 'users-showlastlogin', 'no') === 'yes';


I'd prefer to have those stored in the 'settings' app instead of 'core', the other one make sense for core since core sends out the email and generates the user id, but the show* settings are really only settings app relevant.

juliusknorr · 2020-11-04T15:13:52Z

apps/settings/lib/Controller/UsersController.php

 		if (!in_array($key, $allowed, true)) {
 			return new JSONResponse([], Http::STATUS_FORBIDDEN);
 		}

-		$this->config->setAppValue('core', $key, $value);
+		$this->config->setAppValue('core', 'users-'.$key, $value);


Ideally we could introduce them as user values so that each admin user has their own setting for 'showlastlogin', 'showlanguages', 'showuserbackend', 'showstoragepath' using setUserValue instead

Also wouldn't this basically break the existing newUser.sendMail setting since the key would now be prefixed with 'users-' ?

juliusknorr

See inline 😉

J0WI · 2021-04-28T12:39:09Z

🏓 @sharidas

skjnldsv · 2021-10-13T10:06:36Z

As there is no feedback since a while I will close this PR. If you are still willing to get this in, please address the potential comments and rebase to latest master. Then, feel free to re-open.

sharidas linked an issue Oct 10, 2020 that may be closed by this pull request

User Management settings not saved on logout #22031

Closed

sharidas self-assigned this Oct 10, 2020

sharidas added the 2. developing Work in progress label Oct 10, 2020

sharidas force-pushed the save-config-user-management branch 2 times, most recently from e30d8a4 to fa8a6c6 Compare October 10, 2020 13:20

sharidas added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 10, 2020

sharidas requested review from skjnldsv, ChristophWurst and schiessle and removed request for skjnldsv and ChristophWurst October 10, 2020 16:04

skjnldsv reviewed Oct 10, 2020

View reviewed changes

apps/settings/src/views/Users.vue Outdated Show resolved Hide resolved

skjnldsv requested review from juliusknorr and GretaD October 10, 2020 16:13

skjnldsv added feature: users and groups enhancement labels Oct 10, 2020

skjnldsv added this to the Nextcloud 21 milestone Oct 10, 2020

sharidas force-pushed the save-config-user-management branch 2 times, most recently from 540b211 to 82eee59 Compare October 11, 2020 16:43

skjnldsv reviewed Oct 11, 2020

View reviewed changes

apps/settings/src/views/Users.vue Outdated Show resolved Hide resolved

skjnldsv reviewed Oct 11, 2020

View reviewed changes

apps/settings/src/views/Users.vue Outdated Show resolved Hide resolved

sharidas force-pushed the save-config-user-management branch 2 times, most recently from a0dfe4a to b74add5 Compare October 12, 2020 12:25

ChristophWurst reviewed Oct 14, 2020

View reviewed changes

apps/settings/lib/Controller/UsersController.php Outdated Show resolved Hide resolved

Persist the config changes of usermanagement

af74889

Persist the changes made to usermanagemnt like: - Show Languages - Show last login - Show user backend - Show storage path Signed-off-by: Sujith Haridasan <sujith.h@gmail.com>

sharidas force-pushed the save-config-user-management branch from 7765be2 to af74889 Compare October 14, 2020 09:15

skjnldsv requested review from ChristophWurst and skjnldsv November 4, 2020 14:32

skjnldsv approved these changes Nov 4, 2020

View reviewed changes

juliusknorr reviewed Nov 4, 2020

View reviewed changes

juliusknorr requested changes Nov 4, 2020

View reviewed changes

This was referenced Dec 14, 2020

21 beta2 #24685

Merged

21 beta3 #24765

Merged

This was referenced Dec 28, 2020

21 beta4 #24852

Merged

21 beta5 #24951

Merged

MorrisJobke modified the milestones: Nextcloud 21, Nextcloud 22 Jan 7, 2021

J0WI added feature: settings javascript labels Apr 28, 2021

This was referenced May 20, 2021

22 beta1 #26953

Merged

22.0.0 beta 2 #27111

Merged

blizzz mentioned this pull request Jun 2, 2021

22.0.0 beta 3 #27342

Merged

57 tasks

blizzz modified the milestones: Nextcloud 22, Nextcloud 23 Jun 2, 2021

skjnldsv mentioned this pull request Oct 13, 2021

23.0.0 beta 1 #29202

Merged

skjnldsv closed this Oct 13, 2021

skjnldsv removed this from the Nextcloud 23 milestone Oct 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Persist the config changes of usermanagement #23333

Persist the config changes of usermanagement #23333

sharidas commented Oct 10, 2020 •

edited

Loading

sharidas commented Oct 10, 2020

sharidas commented Oct 12, 2020

skjnldsv commented Oct 12, 2020

sharidas commented Oct 12, 2020

skjnldsv commented Nov 3, 2020

sharidas commented Nov 4, 2020

skjnldsv commented Nov 4, 2020 •

edited

Loading

juliusknorr Nov 4, 2020 •

edited

Loading

juliusknorr Nov 4, 2020

juliusknorr Nov 4, 2020

juliusknorr left a comment

J0WI commented Apr 28, 2021

skjnldsv commented Oct 13, 2021

Persist the config changes of usermanagement #23333

Persist the config changes of usermanagement #23333

Conversation

sharidas commented Oct 10, 2020 • edited Loading

sharidas commented Oct 10, 2020

Summary

Description

How was it tested?

sharidas commented Oct 12, 2020

skjnldsv commented Oct 12, 2020

sharidas commented Oct 12, 2020

skjnldsv commented Nov 3, 2020

sharidas commented Nov 4, 2020

skjnldsv commented Nov 4, 2020 • edited Loading

juliusknorr Nov 4, 2020 • edited Loading

Choose a reason for hiding this comment

juliusknorr Nov 4, 2020

Choose a reason for hiding this comment

juliusknorr Nov 4, 2020

Choose a reason for hiding this comment

juliusknorr left a comment

Choose a reason for hiding this comment

J0WI commented Apr 28, 2021

skjnldsv commented Oct 13, 2021

sharidas commented Oct 10, 2020 •

edited

Loading

skjnldsv commented Nov 4, 2020 •

edited

Loading

juliusknorr Nov 4, 2020 •

edited

Loading