Merge pull request #4890 from nextcloud/unify-settings

Unify settings

10 files changed, 205 insertions, 27 deletions

diff --git a/tests/Settings/Controller/AdminSettingsControllerTest.php b/tests/Settings/Controller/AdminSettingsControllerTest.php
index 6c93bca0d68..51357f67a2d 100644
--- a/tests/Settings/Controller/AdminSettingsControllerTest.php
+++ b/tests/Settings/Controller/AdminSettingsControllerTest.php
@@ -22,7 +22,6 @@
  */
 namespace Tests\Settings\Controller;
 
-
 use OC\Settings\Admin\TipsTricks;
 use OC\Settings\Controller\AdminSettingsController;
 use OCP\AppFramework\Http\TemplateResponse;
@@ -31,6 +30,13 @@ use OCP\IRequest;
 use OCP\Settings\IManager;
 use Test\TestCase;
 
+/**
+ * Class AdminSettingsControllerTest
+ *
+ * @group DB
+ *
+ * @package Tests\Settings\Controller
+ */
 class AdminSettingsControllerTest extends TestCase {
 	/** @var AdminSettingsController */
 	private $adminSettingsController;
@@ -38,8 +44,10 @@ class AdminSettingsControllerTest extends TestCase {
 	private $request;
 	/** @var INavigationManager */
 	private $navigationManager;
-	/** @var IManager */
+	/** @var IManager|\PHPUnit_Framework_MockObject_MockObject */
 	private $settingsManager;
+	/** @var string */
+	private $adminUid = 'lololo';
 
 	public function setUp() {
 		parent::setUp();
@@ -54,6 +62,16 @@ class AdminSettingsControllerTest extends TestCase {
 			$this->navigationManager,
 			$this->settingsManager
 		);
+
+		$user = \OC::$server->getUserManager()->createUser($this->adminUid, 'olo');
+		\OC_User::setUserId($user->getUID());
+		\OC::$server->getGroupManager()->createGroup('admin')->addUser($user);
+	}
+
+	public function tearDown() {
+		\OC::$server->getUserManager()->get($this->adminUid)->delete();
+
+		parent::tearDown();
 	}
 
 	public function testIndex() {
@@ -63,10 +81,15 @@ class AdminSettingsControllerTest extends TestCase {
 			->willReturn([]);
 		$this->settingsManager
 			->expects($this->once())
+			->method('getPersonalSections')
+			->willReturn([]);
+		$this->settingsManager
+			->expects($this->once())
 			->method('getAdminSettings')
 			->with('test')
 			->willReturn([5 => new TipsTricks($this->getMockBuilder('\OCP\IConfig')->getMock())]);
-		$expected = new TemplateResponse('settings', 'admin/frame', ['forms' => [], 'content' => '']);
+
+		$expected = new TemplateResponse('settings', 'settings/frame', ['forms' => ['personal' => [], 'admin' => []], 'content' => '']);
 		$this->assertEquals($expected, $this->adminSettingsController->index('test'));
 	}
 }
diff --git a/tests/acceptance/features/access-levels.feature b/tests/acceptance/features/access-levels.feature
index 57998899a57..80170296675 100644
--- a/tests/acceptance/features/access-levels.feature
+++ b/tests/acceptance/features/access-levels.feature
@@ -1,11 +1,10 @@
 Feature: access-levels
 
-  Scenario: regular users can not see admin-level items in the Settings menu
+  Scenario: regular users cannot see admin-level items in the Settings menu
     Given I am logged in
     When I open the Settings menu
     Then I see that the Settings menu is shown
-    And I see that the "Personal" item in the Settings menu is shown
-    And I see that the "Admin" item in the Settings menu is not shown
+    And I see that the "Settings" item in the Settings menu is shown
     And I see that the "Users" item in the Settings menu is not shown
     And I see that the "Help" item in the Settings menu is shown
     And I see that the "Log out" item in the Settings menu is shown
@@ -14,8 +13,19 @@ Feature: access-levels
     Given I am logged in as the admin
     When I open the Settings menu
     Then I see that the Settings menu is shown
-    And I see that the "Personal" item in the Settings menu is shown
-    And I see that the "Admin" item in the Settings menu is shown
+    And I see that the "Settings" item in the Settings menu is shown
     And I see that the "Users" item in the Settings menu is shown
     And I see that the "Help" item in the Settings menu is shown
     And I see that the "Log out" item in the Settings menu is shown
+
+  Scenario: regular users cannot see admin-level items on the Settings page
+    Given I am logged in
+    When I visit the settings page
+    Then I see that the "Personal" settings panel is shown
+    And I see that the "Administration" settings panel is not shown
+
+  Scenario: admin users can see admin-level items on the Settings page
+    Given I am logged in as the admin
+    When I visit the settings page
+    Then I see that the "Personal" settings panel is shown
+    And I see that the "Administration" settings panel is shown
diff --git a/tests/acceptance/features/bootstrap/SettingsMenuContext.php b/tests/acceptance/features/bootstrap/SettingsMenuContext.php
index 1ff5d94e98f..401575c78f0 100644
--- a/tests/acceptance/features/bootstrap/SettingsMenuContext.php
+++ b/tests/acceptance/features/bootstrap/SettingsMenuContext.php
@@ -67,6 +67,15 @@ class SettingsMenuContext implements Context, ActorAwareInterface {
 	}
 
 	/**
+	 * @param string $itemText
+	 * @return Locator
+	 */
+	private static function settingsPanelFor($itemText) {
+		return Locator::forThe()->xpath("//div[@id = 'app-navigation']//ul//li[@class = 'settings-caption' and normalize-space() = '$itemText']")->
+		describedAs($itemText . " item in Settings panel");
+	}
+
+	/**
 	 * @When I open the Settings menu
 	 */
 	public function iOpenTheSettingsMenu() {
@@ -83,6 +92,14 @@ class SettingsMenuContext implements Context, ActorAwareInterface {
 	}
 
 	/**
+	 * @When I visit the settings page
+	 */
+	public function iVisitTheSettingsPage() {
+		$this->iOpenTheSettingsMenu();
+		$this->actor->find(self::menuItemFor('Settings'), 2)->click();
+	}
+
+	/**
 	 * @When I log out
 	 */
 	public function iLogOut() {
@@ -120,4 +137,25 @@ class SettingsMenuContext implements Context, ActorAwareInterface {
 		}
 	}
 
+	/**
+	 * @Then I see that the :itemText settings panel is shown
+	 */
+	public function iSeeThatTheItemSettingsPanelIsShown($itemText) {
+		PHPUnit_Framework_Assert::assertTrue(
+			$this->actor->find(self::settingsPanelFor($itemText), 10)->isVisible()
+		);
+	}
+
+	/**
+	 * @Then I see that the :itemText settings panel is not shown
+	 */
+	public function iSeeThatTheItemSettingsPanelIsNotShown($itemText) {
+		try {
+			PHPUnit_Framework_Assert::assertFalse(
+				$this->actor->find(self::settingsPanelFor($itemText), 10)->isVisible()
+			);
+		} catch (NoSuchElementException $exception) {
+		}
+	}
+
 }
diff --git a/tests/acceptance/features/core/ElementWrapper.php b/tests/acceptance/features/core/ElementWrapper.php
index 6b730903f6c..f6ce176817b 100644
--- a/tests/acceptance/features/core/ElementWrapper.php
+++ b/tests/acceptance/features/core/ElementWrapper.php
@@ -119,7 +119,7 @@ class ElementWrapper {
 	/**
 	 * Returns whether the wrapped element is visible or not.
 	 *
-	 * @return boolbean true if the wrapped element is visible, false otherwise.
+	 * @return bool true if the wrapped element is visible, false otherwise.
 	 */
 	public function isVisible() {
 		$commandCallback = function() {
diff --git a/tests/lib/Settings/Admin/AdditionalTest.php b/tests/lib/Settings/Admin/AdditionalTest.php
index 420a7110c13..84c63f3aeb1 100644
--- a/tests/lib/Settings/Admin/AdditionalTest.php
+++ b/tests/lib/Settings/Admin/AdditionalTest.php
@@ -97,7 +97,7 @@ class AdditionalTest extends TestCase {
 
 		$expected = new TemplateResponse(
 			'settings',
-			'admin/additional-mail',
+			'settings/admin/additional-mail',
 			[
 				'sendmail_is_available' => (bool) \OC_Helper::findBinaryPath('sendmail'),
 				'mail_domain'           => 'mx.nextcloud.com',
diff --git a/tests/lib/Settings/Admin/EncryptionTest.php b/tests/lib/Settings/Admin/EncryptionTest.php
index a282b059c92..a5f483863e6 100644
--- a/tests/lib/Settings/Admin/EncryptionTest.php
+++ b/tests/lib/Settings/Admin/EncryptionTest.php
@@ -81,7 +81,7 @@ class EncryptionTest extends TestCase {
 			->willReturn(['entry']);
 		$expected = new TemplateResponse(
 			'settings',
-			'admin/encryption',
+			'settings/admin/encryption',
 			[
 				'encryptionEnabled' => $enabled,
 				'encryptionReady' => $enabled,
@@ -116,7 +116,7 @@ class EncryptionTest extends TestCase {
 			->willReturn(['entry', 'entry']);
 		$expected = new TemplateResponse(
 			'settings',
-			'admin/encryption',
+			'settings/admin/encryption',
 			[
 				'encryptionEnabled' => $enabled,
 				'encryptionReady' => $enabled,
diff --git a/tests/lib/Settings/Admin/ServerTest.php b/tests/lib/Settings/Admin/ServerTest.php
index f876ae85136..a71aef0178e 100644
--- a/tests/lib/Settings/Admin/ServerTest.php
+++ b/tests/lib/Settings/Admin/ServerTest.php
@@ -123,7 +123,7 @@ class ServerTest extends TestCase {
 		$envPath = getenv('PATH');
 		$expected = new TemplateResponse(
 			'settings',
-			'admin/server',
+			'settings/admin/server',
 			[
 				// Diagnosis
 				'readOnlyConfigEnabled'            => \OC_Helper::isReadOnlyConfigEnabled(),
diff --git a/tests/lib/Settings/Admin/SharingTest.php b/tests/lib/Settings/Admin/SharingTest.php
index 0bf03559683..d9aa14fecea 100644
--- a/tests/lib/Settings/Admin/SharingTest.php
+++ b/tests/lib/Settings/Admin/SharingTest.php
@@ -112,7 +112,7 @@ class SharingTest extends TestCase {
 
 		$expected = new TemplateResponse(
 			'settings',
-			'admin/sharing',
+			'settings/admin/sharing',
 			[
 				'allowGroupSharing'               => 'yes',
 				'allowLinks'                      => 'yes',
@@ -205,7 +205,7 @@ class SharingTest extends TestCase {
 
 		$expected = new TemplateResponse(
 			'settings',
-			'admin/sharing',
+			'settings/admin/sharing',
 			[
 				'allowGroupSharing'               => 'yes',
 				'allowLinks'                      => 'yes',
diff --git a/tests/lib/Settings/Admin/TipsTricksTest.php b/tests/lib/Settings/Admin/TipsTricksTest.php
index 0e8857b56d0..cbecd51ed55 100644
--- a/tests/lib/Settings/Admin/TipsTricksTest.php
+++ b/tests/lib/Settings/Admin/TipsTricksTest.php
@@ -52,7 +52,7 @@ class TipsTrickTest extends TestCase {
 
 		$expected = new TemplateResponse(
 			'settings',
-			'admin/tipstricks',
+			'settings/admin/tipstricks',
 			[
 				'databaseOverload' => true,
 			],
@@ -71,7 +71,7 @@ class TipsTrickTest extends TestCase {
 
 		$expected = new TemplateResponse(
 			'settings',
-			'admin/tipstricks',
+			'settings/admin/tipstricks',
 			[
 				'databaseOverload' => false,
 			],
diff --git a/tests/lib/Settings/ManagerTest.php b/tests/lib/Settings/ManagerTest.php
index 07f7e71feca..6a13b737c8e 100644
--- a/tests/lib/Settings/ManagerTest.php
+++ b/tests/lib/Settings/ManagerTest.php
@@ -23,18 +23,23 @@
 
 namespace Tests\Settings;
 
+use OC\Accounts\AccountManager;
 use OC\Settings\Admin\Sharing;
 use OC\Settings\Manager;
 use OC\Settings\Mapper;
+use OC\Settings\Personal\Security;
 use OC\Settings\Section;
+use OCP\App\IAppManager;
 use OCP\Encryption\IManager;
 use OCP\IConfig;
 use OCP\IDBConnection;
+use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\ILogger;
 use OCP\IRequest;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
+use OCP\L10N\IFactory;
 use OCP\Lock\ILockingProvider;
 use Test\TestCase;
 
@@ -61,6 +66,16 @@ class ManagerTest extends TestCase {
 	private $mapper;
 	/** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */
 	private $url;
+	/** @var AccountManager|\PHPUnit_Framework_MockObject_MockObject */
+	private $accountManager;
+	/** @var  IGroupManager|\PHPUnit_Framework_MockObject_MockObject */
+	private $groupManager;
+	/** @var  IFactory|\PHPUnit_Framework_MockObject_MockObject */
+	private $l10nFactory;
+	/** @var \OC_Defaults|\PHPUnit_Framework_MockObject_MockObject */
+	private $defaults;
+	/** @var IAppManager */
+	private $appManager;
 
 	public function setUp() {
 		parent::setUp();
@@ -75,6 +90,11 @@ class ManagerTest extends TestCase {
 		$this->request = $this->createMock(IRequest::class);
 		$this->mapper = $this->createMock(Mapper::class);
 		$this->url = $this->createMock(IURLGenerator::class);
+		$this->accountManager = $this->createMock(AccountManager::class);
+		$this->groupManager = $this->createMock(IGroupManager::class);
+		$this->l10nFactory = $this->createMock(IFactory::class);
+		$this->defaults = $this->createMock(\OC_Defaults::class);
+		$this->appManager = $this->createMock(IAppManager::class);
 
 		$this->manager = new Manager(
 			$this->logger,
@@ -86,21 +106,40 @@ class ManagerTest extends TestCase {
 			$this->lockingProvider,
 			$this->request,
 			$this->mapper,
-			$this->url
+			$this->url,
+			$this->accountManager,
+			$this->groupManager,
+			$this->l10nFactory,
+			$this->defaults,
+			$this->appManager
 		);
 	}
 
-	public function testSetupSettingsUpdate() {
+	public function settingsTypeProvider() {
+		return [
+			['admin', 'admin_settings'],
+			['personal', 'personal_settings'],
+		];
+	}
+
+	/**
+	 * @dataProvider settingsTypeProvider
+	 * @param string $type
+	 * @param string $table
+	 */
+	public function testSetupSettingsUpdate($type, $table) {
+		$className = 'OCA\Files\Settings\Admin';
+
 		$this->mapper->expects($this->any())
 			->method('has')
-			->with('admin_settings', 'OCA\Files\Settings\Admin')
+			->with($table, $className)
 			->will($this->returnValue(true));
 
 		$this->mapper->expects($this->once())
 			->method('update')
-			->with('admin_settings',
+			->with($table,
 				'class',
-				'OCA\Files\Settings\Admin', [
+				$className, [
 					'section' => 'additional',
 					'priority' => 5
 				]);
@@ -108,19 +147,24 @@ class ManagerTest extends TestCase {
 			->method('add');
 
 		$this->manager->setupSettings([
-			'admin' => 'OCA\Files\Settings\Admin',
+			$type => $className,
 		]);
 	}
 
-	public function testSetupSettingsAdd() {
+	/**
+	 * @dataProvider settingsTypeProvider
+	 * @param string $type
+	 * @param string $table
+	 */
+	public function testSetupSettingsAdd($type, $table) {
 		$this->mapper->expects($this->any())
 			->method('has')
-			->with('admin_settings', 'OCA\Files\Settings\Admin')
+			->with($table, 'OCA\Files\Settings\Admin')
 			->will($this->returnValue(false));
 
 		$this->mapper->expects($this->once())
 			->method('add')
-			->with('admin_settings', [
+			->with($table, [
 				'class' => 'OCA\Files\Settings\Admin',
 				'section' => 'additional',
 				'priority' => 5
@@ -130,7 +174,7 @@ class ManagerTest extends TestCase {
 			->method('update');
 
 		$this->manager->setupSettings([
-			'admin' => 'OCA\Files\Settings\Admin',
+			$type => 'OCA\Files\Settings\Admin',
 		]);
 	}
 
@@ -167,6 +211,34 @@ class ManagerTest extends TestCase {
 		], $this->manager->getAdminSections());
 	}
 
+	public function testGetPersonalSections() {
+		$this->l10n
+			->expects($this->any())
+			->method('t')
+			->will($this->returnArgument(0));
+
+		$this->mapper->expects($this->once())
+			->method('getPersonalSectionsFromDB')
+			->will($this->returnValue([
+				['class' => \OCA\WorkflowEngine\Settings\Section::class, 'priority' => 90]
+			]));
+
+		$this->url->expects($this->exactly(3))
+			->method('imagePath')
+			->willReturnMap([
+				['core', 'actions/info.svg', '1'],
+				['settings', 'password.svg', '2'],
+				['settings', 'change.svg', '3'],
+			]);
+
+		$this->assertArraySubset([
+			0 => [new Section('personal-info', 'Personal info', 0, '1')],
+			5 => [new Section('security', 'Security', 0, '2')],
+			15 => [new Section('sync-clients', 'Sync clients', 0, '3')],
+			90 => [\OC::$server->query(\OCA\WorkflowEngine\Settings\Section::class)],
+		], $this->manager->getPersonalSections());
+	}
+
 	public function testGetAdminSectionsEmptySection() {
 		$this->l10n
 			->expects($this->any())
@@ -198,6 +270,31 @@ class ManagerTest extends TestCase {
 		], $this->manager->getAdminSections());
 	}
 
+	public function testGetPersonalSectionsEmptySection() {
+		$this->l10n
+			->expects($this->any())
+			->method('t')
+			->will($this->returnArgument(0));
+
+		$this->mapper->expects($this->once())
+			->method('getPersonalSectionsFromDB')
+			->will($this->returnValue([]));
+
+		$this->url->expects($this->exactly(3))
+			->method('imagePath')
+			->willReturnMap([
+				['core', 'actions/info.svg', '1'],
+				['settings', 'password.svg', '2'],
+				['settings', 'change.svg', '3'],
+			]);
+
+		$this->assertArraySubset([
+			0 => [new Section('personal-info', 'Personal info', 0, '1')],
+			5 => [new Section('security', 'Security', 0, '2')],
+			15 => [new Section('sync-clients', 'Sync clients', 0, '3')],
+		], $this->manager->getPersonalSections());
+	}
+
 	public function testGetAdminSettings() {
 		$this->mapper->expects($this->any())
 			->method('getAdminSettingsFromDB')
@@ -207,4 +304,14 @@ class ManagerTest extends TestCase {
 			0 => [new Sharing($this->config)],
 		], $this->manager->getAdminSettings('sharing'));
 	}
+
+	public function testGetPersonalSettings() {
+		$this->mapper->expects($this->any())
+			->method('getPersonalSettingsFromDB')
+			->will($this->returnValue([]));
+
+		$this->assertEquals([
+			10 => [new Security()],
+		], $this->manager->getPersonalSettings('security'));
+	}
 }