make Settings of Personal and Admin into own views

[privatemaker]

This is my first PHP contribution to Nextcloud, as far as I can tell no tests were required, but if so please advise further. If accepted, this resolves #40490

Summary

I created the following new methods in CommonSettingsTrait.php

• getPersonalResponse()
• getAdminResponse()

I think this is better than conditionals in getIndexResponse as it is more locked down (no trailing elseif) and makes room for future adjustments respective to only Personal or Admin areas. This comes at the expense of being slightly less DRY of repeated lines.

Checklist

• Screenshots in issue Only show context relevant links (Personal or Administration settings) in sidebar #40490
• Backports requested where applicable (ex: critical bugfixes)

[szaimen]

Thanks a lot for the contribution! Tested and works great! :)

[skjnldsv]

Please fix the indents issues. I see spaces and wrong indents ;)

[kesselb]

Admin right privilege / Administration privileges is broken.

[kesselb]

Looks wrong, please revert.

[nickvergessen]

happens by bash build/autoloader…
It's okay

[kesselb]

Looks wrong, please revert.

[kesselb]

I find it weird that the underlying logic to render the setting sections is still executed for admin and personal, even if you visit the personal section and vice versa.

[kesselb]

Admin right privilege / Administration privileges is broken.

It's possible to make some admin sections visible to groups without granting the users full admin rights. That's called Administration privileges (in Nextcloud) / Admin right privilege (in our Documentation).

1. Create a group subadmin
2. Add bob to the subadmin group
3. Visit Admin -> Administration privileges
4. Assign a group to a section
5. Login as bob

Main:

Bob see the administration section but only one item.

This PR:

Bob does not see the administration section.

It's still possible the admin the section if you know the right url, but there is no link somewhere. The menu item "Administration settings" is only added for users with admin permission, but not administration privileges.

Adding a link below "Settings" is a bit tricky for Administration privileges because you need to figure out the internal url for the given section.

[kesselb]

This looks weird ;)

Adding @group DB says a test needs a database connection.
Afaict the test only works with mocked data and therefore does not require a db.

Index: apps/settings/tests/Controller/PersonalSettingsControllerTest.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/apps/settings/tests/Controller/PersonalSettingsControllerTest.php b/apps/settings/tests/Controller/PersonalSettingsControllerTest.php
--- a/apps/settings/tests/Controller/PersonalSettingsControllerTest.php	(revision bb2353ae4a18e95dc4ca6346ad696c0e371453e5)
+++ b/apps/settings/tests/Controller/PersonalSettingsControllerTest.php	(date 1695568072588)
@@ -40,13 +40,6 @@
 use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
-/**
- * Class PersonalSettingsControllerTest
- *
- * @group DB
- *
- * @package Tests\Settings\Controller
- */
 class PersonalSettingsControllerTest extends TestCase {
 
 	/** @var PersonalSettingsController */
@@ -76,16 +69,6 @@
 			$this->settingsManager,
 			$this->userSession,
 		);
-
-		$user = \OC::$server->getUserManager()->createUser($this->adminUid, 'mylongrandompassword');
-		\OC_User::setUserId($user->getUID());
-		// \OC::$server->getGroupManager()->createGroup('admin')->addUser($user);
-	}
-
-	protected function tearDown(): void {
-		// \OC::$server->getUserManager()->get($this->adminUid)->delete();
-
-		parent::tearDown();
 	}
 
 	public function testIndex() {
@@ -102,7 +85,10 @@
 		$idx = $this->personalSettingsController->index('test');
 
 		$expected = new TemplateResponse('settings', 'settings/personal', [
-			'forms' => ['personal' => []],
+			'forms' => [
+				'personal' => [],
+				'admin' => [],
+			],
 			'content' => ''
 		]);
 		$this->assertEquals($expected, $idx);
Index: apps/settings/tests/Controller/AdminSettingsControllerTest.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/apps/settings/tests/Controller/AdminSettingsControllerTest.php b/apps/settings/tests/Controller/AdminSettingsControllerTest.php
--- a/apps/settings/tests/Controller/AdminSettingsControllerTest.php	(revision bb2353ae4a18e95dc4ca6346ad696c0e371453e5)
+++ b/apps/settings/tests/Controller/AdminSettingsControllerTest.php	(date 1695568055144)
@@ -27,6 +27,7 @@
 namespace OCA\Settings\Tests\Controller;
 
 use OCA\Settings\Controller\AdminSettingsController;
+use OCA\Settings\Settings\Personal\ServerDevNotice;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\Group\ISubAdmin;
 use OCP\IGroupManager;
@@ -38,13 +39,6 @@
 use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
-/**
- * Class AdminSettingsControllerTest
- *
- * @group DB
- *
- * @package Tests\Settings\Controller
- */
 class AdminSettingsControllerTest extends TestCase {
 
 	/** @var AdminSettingsController */
@@ -62,7 +56,6 @@
 	/** @var ISubAdmin|MockObject */
 	private $subAdmin;
 	/** @var string */
-	private $adminUid = 'lololo';
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -83,16 +76,6 @@
 			$this->groupManager,
 			$this->subAdmin
 		);
-
-		$user = \OC::$server->getUserManager()->createUser($this->adminUid, 'mylongrandompassword');
-		\OC_User::setUserId($user->getUID());
-		\OC::$server->getGroupManager()->createGroup('admin')->addUser($user);
-	}
-
-	protected function tearDown(): void {
-		\OC::$server->getUserManager()->get($this->adminUid)->delete();
-
-		parent::tearDown();
 	}
 
 	public function testIndex() {
@@ -115,10 +98,6 @@
 			->willReturn([]);
 		$this->settingsManager
 			->expects($this->once())
-			->method('getPersonalSections')
-			->willReturn([]);
-		$this->settingsManager
-			->expects($this->once())
 			->method('getAllowedAdminSettings')
 			->with('test')
 			->willReturn([5 => $this->createMock(ServerDevNotice::class)]);
@@ -126,7 +105,10 @@
 		$idx = $this->adminSettingsController->index('test');
 
 		$expected = new TemplateResponse('settings', 'settings/admin', [
-			'forms' => ['admin' => []],
+			'forms' => [
+				'personal' => [],
+				'admin' => [],
+			],
 			'content' => ''
 		]);
 		$this->assertEquals($expected, $idx);

[JuliaKirschenheuter]

Please adapt your changes to two independent nav's https://github.com/nextcloud/server/pull/40576/files
Thank you!