From 99e97f135de73de85550e167feb7ccc80f1fc13d Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Wed, 26 Apr 2017 12:45:23 +0200
Subject: [PATCH] consolidate setEnabled method

and fix a unit test

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 settings/Controller/UsersController.php       | 88 +++----------------
 .../Controller/UsersControllerTest.php        | 28 +++---
 2 files changed, 28 insertions(+), 88 deletions(-)

diff --git a/settings/Controller/UsersController.php b/settings/Controller/UsersController.php
index 277263b3753..4fed2655940 100644
--- a/settings/Controller/UsersController.php
+++ b/settings/Controller/UsersController.php
@@ -519,66 +519,17 @@ class UsersController extends Controller {
 	 * @NoAdminRequired
 	 *
 	 * @param string $id
+	 * @param int $enabled
 	 * @return DataResponse
 	 */
-	public function disable($id) {
-		$userId = $this->userSession->getUser()->getUID();
-		$user = $this->userManager->get($id);
-
-		if ($userId === $id) {
-			return new DataResponse(
-				[
-					'status' => 'error',
-					'data' => [
-						'message' => (string) $this->l10n->t('Error while disabling user.')
-					]
-				], Http::STATUS_FORBIDDEN
-			);
-		}
-
-		if ($user) {
-			if(!$this->isAdmin && !$this->groupManager->getSubAdmin()->isUserAccessible($this->userSession->getUser(), $user)) {
-				return new DataResponse(
-					[
-						'status' => 'error',
-						'data' => [
-							'message' => (string) $this->l10n->t('Authentication error')
-						]
-					],
-					Http::STATUS_FORBIDDEN
-				);
-			}
-
-			$user->setEnabled(false);
-			return new DataResponse(
-				[
-					'status' => 'success',
-					'data' => [
-						'username' => $id,
-						'enabled' => 0
-					]
-				]
-			);
+	public function setEnabled($id, $enabled) {
+		$enabled = (bool)$enabled;
+		if($enabled) {
+			$errorMsgGeneral = (string) $this->l10n->t('Error while enabling user.');
 		} else {
-			return new DataResponse(
-				[
-					'status' => 'error',
-					'data' => [
-						'message' => (string) $this->l10n->t('Error while disabling user.')
-					]
-				],
-				Http::STATUS_FORBIDDEN
-			);
+			$errorMsgGeneral = (string) $this->l10n->t('Error while disabling user.');
 		}
-	}
 
-	/**
-	 * @NoAdminRequired
-	 *
-	 * @param string $id
-	 * @return DataResponse
-	 */
-	public function enable($id) {
 		$userId = $this->userSession->getUser()->getUID();
 		$user = $this->userManager->get($id);
 
@@ -587,10 +538,9 @@ class UsersController extends Controller {
 				[
 					'status' => 'error',
 					'data' => [
-						'message' => (string) $this->l10n->t('Error while enabling user.')
-				]
-				],
-				Http::STATUS_FORBIDDEN
+						'message' => $errorMsgGeneral
+					]
+				], Http::STATUS_FORBIDDEN
 			);
 		}
 
@@ -607,13 +557,13 @@ class UsersController extends Controller {
 				);
 			}
 
-			$user->setEnabled(true);
+			$user->setEnabled($enabled);
 			return new DataResponse(
 				[
 					'status' => 'success',
 					'data' => [
 						'username' => $id,
-						'enabled' => 1
+						'enabled' => $enabled
 					]
 				]
 			);
@@ -622,27 +572,13 @@ class UsersController extends Controller {
 				[
 					'status' => 'error',
 					'data' => [
-						'message' => (string) $this->l10n->t('Error while enabling user.')
+						'message' => $errorMsgGeneral
 					]
 				],
 				Http::STATUS_FORBIDDEN
 			);
 		}
-	}
 
-	/**
-	 * @NoAdminRequired
-	 *
-	 * @param string $id
-	 * @param int $enabled
-	 * @return DataResponse
-	 */
-	public function setEnabled($id, $enabled) {
-		if ((bool) $enabled) {
-			return $this->enable($id);
-		} else {
-			return $this->disable($id);
-		}
 	}
 
 	/**
diff --git a/tests/Settings/Controller/UsersControllerTest.php b/tests/Settings/Controller/UsersControllerTest.php
index 24019800490..5905023e960 100644
--- a/tests/Settings/Controller/UsersControllerTest.php
+++ b/tests/Settings/Controller/UsersControllerTest.php
@@ -2299,6 +2299,9 @@ class UsersControllerTest extends \Test\TestCase {
 			->expects($this->once())
 			->method('getBackendClassName')
 			->willReturn('bar');
+		$user
+			->method('isEnabled')
+			->willReturn(true);
 
 		$this->userManager
 			->expects($this->once())
@@ -2348,6 +2351,7 @@ class UsersControllerTest extends \Test\TestCase {
 				'email' => 'abc@example.org',
 				'isRestoreDisabled' => false,
 				'isAvatarAvailable' => true,
+				'isEnabled' => true,
 			],
 			Http::STATUS_CREATED
 		);
@@ -2460,7 +2464,7 @@ class UsersControllerTest extends \Test\TestCase {
 			],
 			Http::STATUS_FORBIDDEN
 		);
-		$response = $this->getController(true)->disable('abc');
+		$response = $this->getController(true)->setEnabled('abc', false);
 		$this->assertEquals($expectedResponse, $response);
 	}
 
@@ -2502,7 +2506,7 @@ class UsersControllerTest extends \Test\TestCase {
 			],
 			Http::STATUS_FORBIDDEN
 		);
-		$response = $this->getController(false)->disable('abc');
+		$response = $this->getController(false)->setEnabled('abc', false);
 		$this->assertEquals($expectedResponse, $response);
 	}
 
@@ -2535,7 +2539,7 @@ class UsersControllerTest extends \Test\TestCase {
 			],
 			Http::STATUS_FORBIDDEN
 		);
-		$response = $this->getController(true)->disable('abc');
+		$response = $this->getController(true)->setEnabled('abc', false);
 		$this->assertEquals($expectedResponse, $response);
 	}
 
@@ -2568,7 +2572,7 @@ class UsersControllerTest extends \Test\TestCase {
 			],
 			Http::STATUS_FORBIDDEN
 		);
-		$response = $this->getController(false)->disable('abc');
+		$response = $this->getController(false)->setEnabled('abc', false);
 		$this->assertEquals($expectedResponse, $response);
 	}
 
@@ -2606,7 +2610,7 @@ class UsersControllerTest extends \Test\TestCase {
 				],
 			]
 		);
-		$response = $this->getController(true)->disable('abc');
+		$response = $this->getController(true)->setEnabled('abc', false);
 		$this->assertEquals($expectedResponse, $response);
 	}
 
@@ -2648,7 +2652,7 @@ class UsersControllerTest extends \Test\TestCase {
 				],
 			]
 		);
-		$response = $this->getController(false)->disable('abc');
+		$response = $this->getController(false)->setEnabled('abc', false);
 		$this->assertEquals($expectedResponse, $response);
 	}
 
@@ -2671,7 +2675,7 @@ class UsersControllerTest extends \Test\TestCase {
 			],
 			Http::STATUS_FORBIDDEN
 		);
-		$response = $this->getController(true)->enable('abc');
+		$response = $this->getController(true)->setEnabled('abc', true);
 		$this->assertEquals($expectedResponse, $response);
 	}
 
@@ -2713,7 +2717,7 @@ class UsersControllerTest extends \Test\TestCase {
 			],
 			Http::STATUS_FORBIDDEN
 		);
-		$response = $this->getController(false)->enable('abc');
+		$response = $this->getController(false)->setEnabled('abc', true);
 		$this->assertEquals($expectedResponse, $response);
 	}
 
@@ -2746,7 +2750,7 @@ class UsersControllerTest extends \Test\TestCase {
 			],
 			Http::STATUS_FORBIDDEN
 		);
-		$response = $this->getController(true)->enable('abc');
+		$response = $this->getController(true)->setEnabled('abc', true);
 		$this->assertEquals($expectedResponse, $response);
 	}
 
@@ -2779,7 +2783,7 @@ class UsersControllerTest extends \Test\TestCase {
 			],
 			Http::STATUS_FORBIDDEN
 		);
-		$response = $this->getController(false)->enable('abc');
+		$response = $this->getController(false)->setEnabled('abc', true);
 		$this->assertEquals($expectedResponse, $response);
 	}
 
@@ -2816,7 +2820,7 @@ class UsersControllerTest extends \Test\TestCase {
 				],
 			]
 		);
-		$response = $this->getController(true)->enable('abc');
+		$response = $this->getController(true)->setEnabled('abc', true);
 		$this->assertEquals($expectedResponse, $response);
 	}
 
@@ -2859,7 +2863,7 @@ class UsersControllerTest extends \Test\TestCase {
 				],
 			]
 		);
-		$response = $this->getController(false)->enable('abc');
+		$response = $this->getController(false)->setEnabled('abc', true);
 		$this->assertEquals($expectedResponse, $response);
 	}
 }