Log announcement creation and deletion

Closes #381 Signed-off-by: Thomas Citharel <tcit@tcit.fr>
nextcloud · Dec 8, 2022 · 29c8970 · 29c8970
1 parent 9fb1f84
commit 29c8970
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 53 deletions.
diff --git a/lib/Controller/APIController.php b/lib/Controller/APIController.php
@@ -41,29 +41,17 @@
 use OCP\IUserManager;
 use OCP\IUserSession;
 use OCA\AnnouncementCenter\BackgroundJob;
+use Psr\Log\LoggerInterface;
 
 class APIController extends OCSController {
-
-	/** @var IJobList */
-	protected $jobList;
-
-	/** @var IGroupManager */
-	protected $groupManager;
-
-	/** @var IUserManager */
-	protected $userManager;
-
-	/** @var IL10N */
-	protected $l;
-
-	/** @var Manager */
-	protected $manager;
-
-	/** @var ITimeFactory */
-	protected $timeFactory;
-
-	/** @var IUserSession */
-	protected $userSession;
+	protected IJobList $jobList;
+	protected IGroupManager $groupManager;
+	protected IUserManager $userManager;
+	protected IL10N $l;
+	protected Manager $manager;
+	protected ITimeFactory $timeFactory;
+	protected IUserSession $userSession;
+	protected LoggerInterface $logger;
 
 	public function __construct(string $appName,
 		IRequest $request,
@@ -73,7 +61,8 @@ public function __construct(string $appName,
 		IL10N $l,
 		Manager $manager,
 		ITimeFactory $timeFactory,
-		IUserSession $userSession) {
+		IUserSession $userSession,
+		LoggerInterface $logger) {
 		parent::__construct($appName, $request);
 
 		$this->groupManager = $groupManager;
@@ -83,6 +72,7 @@ public function __construct(string $appName,
 		$this->manager = $manager;
 		$this->timeFactory = $timeFactory;
 		$this->userSession = $userSession;
+		$this->logger = $logger;
 	}
 
 	/**
@@ -139,6 +129,8 @@ public function add(string $subject, string $message, string $plainMessage, arra
 			]);
 		}
 
+		$this->logger->info('Admin ' . $userId . ' posted a new announcement: "' . $announcement->getSubject() . '"');
+
 		return new DataResponse($this->renderAnnouncement($announcement));
 	}
 
@@ -198,8 +190,15 @@ public function delete(int $id): DataResponse {
 			);
 		}
 
+		$announcement = $this->manager->getAnnouncement($id);
+
 		$this->manager->delete($id);
 
+		$user = $this->userSession->getUser();
+		$userId = $user instanceof IUser ? $user->getUID() : '';
+
+		$this->logger->info('Admin ' . $userId . ' deleted announcement: "' . $announcement->getSubject() . '"');
+
 		return new DataResponse();
 	}
 

diff --git a/tests/Controller/APIControllerTest.php b/tests/Controller/APIControllerTest.php
@@ -42,6 +42,7 @@
 use OCP\IGroup;
 use PHPUnit\Framework\MockObject\MockBuilder;
 use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
 
 /**
  * @package OCA\AnnouncementCenter\Tests\Controller
@@ -67,6 +68,8 @@ class APIControllerTest extends TestCase {
 	protected $userSession;
 	/** @var IInitialStateService|MockObject */
 	protected $initialStateService;
+	/** @var LoggerInterface|MockObject */
+	protected $logger;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -79,6 +82,7 @@ protected function setUp(): void {
 		$this->manager = $this->createMock(Manager::class);
 		$this->timeFactory = $this->createMock(ITimeFactory::class);
 		$this->userSession = $this->createMock(IUserSession::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
 
 		$this->l
 			->method('t')
@@ -87,6 +91,10 @@ protected function setUp(): void {
 			});
 	}
 
+	/**
+	 * @param array $methods
+	 * @return APIController|MockObject
+	 */
 	protected function getController(array $methods = []): APIController {
 		if (empty($methods)) {
 			return new APIController(
@@ -99,6 +107,7 @@ protected function getController(array $methods = []): APIController {
 				$this->manager,
 				$this->timeFactory,
 				$this->userSession,
+				$this->logger
 			);
 		}
 
@@ -114,6 +123,7 @@ protected function getController(array $methods = []): APIController {
 			$this->manager,
 			$this->timeFactory,
 			$this->userSession,
+			$this->logger
 		])
 			->setMethods($methods)
 			->getMock();
@@ -172,12 +182,8 @@ public function dataGet(): array {
 
 	/**
 	 * @dataProvider dataGet
-	 * @param int $offset
-	 * @param array $announcements
-	 * @param array $userMap
-	 * @param array $expected
 	 */
-	public function legacyTestGet($offset, $announcements, $userMap, $expected) {
+	public function legacyTestGet(int $offset, array $announcements, array $userMap, array $expected) {
 		$this->userManager
 			->method('get')
 			->willReturnMap($userMap);
@@ -224,19 +230,21 @@ public function dataDelete(): array {
 
 	/**
 	 * @dataProvider dataDelete
-	 * @param int $id
-	 * @param bool $isAdmin
-	 * @param int $statusCode
 	 */
-	public function testDelete($id, $isAdmin, $statusCode) {
+	public function testDelete(int $id, bool $isAdmin, int $statusCode) {
 		$this->manager->expects(self::once())
 			->method('checkIsAdmin')
 			->willReturn($isAdmin);
 
 		if ($isAdmin) {
+			$this->manager->expects(self::once())
+				->method('getAnnouncement')
+				->with($id);
 			$this->manager->expects(self::once())
 				->method('delete')
 				->with($id);
+			$this->logger->expects($this->once())
+				->method('info');
 		} else {
 			$this->manager->expects(self::never())
 				->method('delete');
@@ -248,7 +256,7 @@ public function testDelete($id, $isAdmin, $statusCode) {
 		self::assertEquals($statusCode, $response->getStatus());
 	}
 
-	public function dataAddThrows() {
+	public function dataAddThrows(): array {
 		return [
 			['', ['error' => 'The subject is too long or empty']],
 			[str_repeat('a', 513), ['error' => 'The subject is too long or empty']],
@@ -257,10 +265,8 @@ public function dataAddThrows() {
 
 	/**
 	 * @dataProvider dataAddThrows
-	 * @param string $subject
-	 * @param array $expectedData
 	 */
-	public function testAddThrows($subject, array $expectedData) {
+	public function testAddThrows(string $subject, array $expectedData) {
 		$this->manager->expects(self::once())
 			->method('checkIsAdmin')
 			->willReturn(true);
@@ -303,7 +309,7 @@ public function testAddNoAdmin() {
 		self::assertSame(Http::STATUS_FORBIDDEN, $response->getStatus());
 	}
 
-	public function dataAdd() {
+	public function dataAdd(): array {
 		return [
 			['subject1', 'message1', 'message1', ['gid1'], true, true, true, false],
 			['subject2', 'message2', 'message2', ['gid2'], true, false, true, false],
@@ -315,17 +321,8 @@ public function dataAdd() {
 
 	/**
 	 * @dataProvider dataAdd
-	 *
-	 * @param string $subject
-	 * @param string $message
-	 * @param string $plainMessage
-	 * @param array $groups
-	 * @param bool $activities
-	 * @param bool $notifications
-	 * @param bool $emails
-	 * @param bool $comments
 	 */
-	public function legacyTestAdd($subject, $message, $plainMessage, array $groups, $activities, $notifications, $emails, $comments) {
+	public function legacyTestAdd(string $subject, string $message, string $plainMessage, array $groups, bool $activities, bool $notifications, bool $emails, bool $comments) {
 		$this->manager->expects(self::once())
 			->method('checkIsAdmin')
 			->willReturn(true);
@@ -358,14 +355,17 @@ public function legacyTestAdd($subject, $message, $plainMessage, array $groups,
 				'emails' => $emails,
 			]);
 
+		$this->logger->expects($this->once())
+			->method('info');
+
 		$controller = $this->getController();
 
 		$response = $controller->add($subject, $message, $plainMessage, $groups, $activities, $notifications, $emails, $comments);
 
 		self::assertInstanceOf(DataResponse::class, $response);
 		$data = $response->getData();
 		self::assertArrayHasKey('time', $data);
-		self::assertInternalType('int', $data['time']);
+		self::assertIsInt($data['time']);
 		unset($data['time']);
 		self::assertEquals([
 			'author' => 'Author',
@@ -404,13 +404,8 @@ public function dataSearchGroup(): array {
 
 	/**
 	 * @dataProvider dataSearchGroup
-	 * @param bool $isAdmin
-	 * @param string $pattern
-	 * @param array|null $groupSearch
-	 * @param string $expected
-	 * @param int $code
 	 */
-	public function testSearchGroup(bool $isAdmin, string $pattern, $groupSearch, array $expected, int $code) {
+	public function testSearchGroup(bool $isAdmin, string $pattern, ?array $groupSearch, array $expected, int $code) {
 		$this->manager->expects(self::once())
 			->method('checkIsAdmin')
 			->willReturn($isAdmin);