From 9d1688c87b4fdf77d66d97c49458dd8334f0a9b7 Mon Sep 17 00:00:00 2001
From: Anna Larch <anna@nextcloud.com>
Date: Mon, 26 Aug 2024 11:15:13 +0200
Subject: [PATCH] fix(teams): resolve undefined variable error and add logging

Signed-off-by: Anna Larch <anna@nextcloud.com>
---
 lib/Controller/ContactController.php          | 27 ++++++++++++-------
 .../unit/Controller/ContactControllerTest.php |  9 ++++++-
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/lib/Controller/ContactController.php b/lib/Controller/ContactController.php
index af1ee7518..01ce60b02 100644
--- a/lib/Controller/ContactController.php
+++ b/lib/Controller/ContactController.php
@@ -7,7 +7,9 @@
  */
 namespace OCA\Calendar\Controller;
 
+use Exception;
 use OCA\Calendar\Service\ServiceException;
+use OCA\Circles\Api\v1\Circles;
 use OCA\Circles\Exceptions\CircleNotFoundException;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Controller;
@@ -17,6 +19,7 @@
 use OCP\Contacts\IManager;
 use OCP\IRequest;
 use OCP\IUserManager;
+use Psr\Log\LoggerInterface;
 
 /**
  * Class ContactController
@@ -40,11 +43,14 @@ class ContactController extends Controller {
 	 * @param IRequest $request
 	 * @param IManager $contacts
 	 */
-	public function __construct(string $appName,
+	public function __construct(
+		string $appName,
 		IRequest $request,
 		IManager $contacts,
 		IAppManager $appManager,
-		IUserManager $userManager) {
+		IUserManager $userManager,
+		private LoggerInterface $logger,
+	) {
 		parent::__construct($appName, $request);
 		$this->contactsManager = $contacts;
 		$this->appManager = $appManager;
@@ -173,32 +179,32 @@ public function searchAttendee(string $search):JSONResponse {
 	 * @param string $circleId CircleId to query for members
 	 * @return JSONResponse
 	 * @throws Exception
-	 * @throws \OCP\AppFramework\QueryException
 	 *
 	 * @NoAdminRequired
 	 */
 	public function getCircleMembers(string $circleId):JSONResponse {
-		if (!$this->appManager->isEnabledForUser('circles') || !class_exists('\OCA\Circles\Api\v1\Circles')) {
+		if (!class_exists('\OCA\Circles\Api\v1\Circles') || !$this->appManager->isEnabledForUser('circles')) {
+			$this->logger->debug('Circles not enabled');
 			return new JSONResponse();
 		}
 		if (!$this->contactsManager->isEnabled()) {
+			$this->logger->debug('Contacts not enabled');
 			return new JSONResponse();
 		}
 
 		try {
-			$circle = \OCA\Circles\Api\v1\Circles::detailsCircle($circleId, true);
+			$circle = Circles::detailsCircle($circleId, true);
 		} catch (QueryException $ex) {
+			$this->logger->error('Could not resolve circle details', ['exception' => $ex]);
 			return new JSONResponse();
 		} catch (CircleNotFoundException $ex) {
-			return new JSONResponse();
-		}
-
-		if (!$circle) {
+			$this->logger->error('Could not find circle', ['exception' => $ex]);
 			return new JSONResponse();
 		}
 
 		$circleMembers = $circle->getInheritedMembers();
 
+		$contacts = [];
 		foreach ($circleMembers as $circleMember) {
 			if ($circleMember->isLocal()) {
 
@@ -207,7 +213,8 @@ public function getCircleMembers(string $circleId):JSONResponse {
 				$user = $this->userManager->get($circleMemberUserId);
 
 				if ($user === null) {
-					throw new ServiceException('Could not find organizer');
+					$this->logger->error('Could not find user with user id' . $circleMemberUserId);
+					throw new ServiceException('Could not find circle member');
 				}
 
 				$contacts[] = [
diff --git a/tests/php/unit/Controller/ContactControllerTest.php b/tests/php/unit/Controller/ContactControllerTest.php
index 3b176ee3c..57e88f998 100644
--- a/tests/php/unit/Controller/ContactControllerTest.php
+++ b/tests/php/unit/Controller/ContactControllerTest.php
@@ -14,6 +14,7 @@
 use OCP\IRequest;
 use OCP\IUserManager;
 use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
 
 class ContactControllerTest extends TestCase {
 	/** @var string */
@@ -42,8 +43,14 @@ protected function setUp():void {
 		$this->manager = $this->createMock(IManager::class);
 		$this->appManager = $this->createMock(IAppManager::class);
 		$this->userManager = $this->createMock(IUserManager::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->controller = new ContactController($this->appName,
-			$this->request, $this->manager, $this->appManager, $this->userManager);
+			$this->request,
+			$this->manager,
+			$this->appManager,
+			$this->userManager,
+			$this->logger,
+		);
 	}
 
 	public function testSearchLocationDisabled():void {