From 2afddc0eafcd2c9cad6635174c50ab8eaf91f87d Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Sat, 11 Feb 2023 16:34:59 +0100 Subject: [PATCH] feat: Add new activity for submissions on shared forms Also move activity creation from controller into service. Signed-off-by: Ferdinand Thiessen --- appinfo/info.xml | 1 + lib/Activity/ActivityConstants.php | 1 + lib/Activity/ActivityManager.php | 38 ++++++++ lib/Activity/Settings/NewSharedSubmission.php | 54 +++++++++++ lib/Controller/ApiController.php | 91 ++++--------------- lib/Service/FormsService.php | 19 ++++ tests/Unit/Activity/ActivityManagerTest.php | 66 ++++++++++++++ .../Settings/NewSharedSubmissionTest.php | 75 +++++++++++++++ tests/Unit/Controller/ApiControllerTest.php | 25 ++--- tests/Unit/Service/FormsServiceTest.php | 83 +++++++++++++++++ 10 files changed, 363 insertions(+), 90 deletions(-) create mode 100644 lib/Activity/Settings/NewSharedSubmission.php create mode 100644 tests/Unit/Activity/Settings/NewSharedSubmissionTest.php diff --git a/appinfo/info.xml b/appinfo/info.xml index 3e3a3c296..f639ec2a9 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -75,6 +75,7 @@ OCA\Forms\Activity\Settings\NewShare OCA\Forms\Activity\Settings\NewSubmission + OCA\Forms\Activity\Settings\NewSharedSubmission diff --git a/lib/Activity/ActivityConstants.php b/lib/Activity/ActivityConstants.php index e93a5afe3..181b17dd9 100644 --- a/lib/Activity/ActivityConstants.php +++ b/lib/Activity/ActivityConstants.php @@ -29,6 +29,7 @@ class ActivityConstants { */ public const TYPE_NEWSHARE = 'forms_newshare'; public const TYPE_NEWSUBMISSION = 'forms_newsubmission'; + public const TYPE_NEWSHAREDSUBMISSION = 'forms_newsharedsubmission'; /***** * Subjects are internal 'types', that get interpreted by our own Provider. diff --git a/lib/Activity/ActivityManager.php b/lib/Activity/ActivityManager.php index e2e27f85a..8534c254a 100644 --- a/lib/Activity/ActivityManager.php +++ b/lib/Activity/ActivityManager.php @@ -29,6 +29,7 @@ use OCP\IGroupManager; use OCP\IUser; use OCP\IUserSession; +use OCP\Share\IShare; use Psr\Log\LoggerInterface; @@ -126,4 +127,41 @@ public function publishNewSubmission(Form $form, string $submitterID) { $this->manager->publish($event); } + + /** + * Publish a new-Submission Activity for shared forms + * + * @param Form $form The affected Form + * @param string $submitterID ID of the User who submitted the form. Can also be our 'anon-user-'-ID + */ + public function publishNewSharedSubmission(Form $form, int $shareType, string $shareWith, string $submitterID) { + $users = []; + switch ($shareType) { + case IShare::TYPE_USER: + $users[] = $shareWith; + break; + case IShare::TYPE_GROUP: + $group = $this->groupManager->get($shareWith); + if ($group !== null) { + $users = array_map(fn (IUser $user) => $user->getUID(), $group->getUsers()); + } + break; + } + + foreach ($users as $userId) { + $event = $this->manager->generateEvent(); + $event->setApp($this->appName) + ->setType(ActivityConstants::TYPE_NEWSHAREDSUBMISSION) + ->setAffectedUser($userId) + ->setAuthor($submitterID) + ->setSubject(ActivityConstants::SUBJECT_NEWSUBMISSION, [ + 'userId' => $submitterID, + 'formTitle' => $form->getTitle(), + 'formHash' => $form->getHash() + ]) + ->setObject('form', $form->getId()); + + $this->manager->publish($event); + } + } } diff --git a/lib/Activity/Settings/NewSharedSubmission.php b/lib/Activity/Settings/NewSharedSubmission.php new file mode 100644 index 000000000..a2f00a4ee --- /dev/null +++ b/lib/Activity/Settings/NewSharedSubmission.php @@ -0,0 +1,54 @@ + + * + * @author Ferdinand Thiessen + * + * @license AGPL-3.0-or-later + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Forms\Activity\Settings; + +use OCA\Forms\Activity\ActivityConstants; + +class NewSharedSubmission extends FormsActivitySettings { + /** + * Event-Type this setting applies to + * Only lowercase letters and underscore allowed + * @return string + */ + public function getIdentifier(): string { + return ActivityConstants::TYPE_NEWSHAREDSUBMISSION; + } + + /** + * Text of the setting + * @return string Translated String + */ + public function getName(): string { + return $this->l10n->t('Someone answered a shared form'); + } + + /** + * Priority of the Setting + * Using Forms Base-Priority (parent) + * @return int + */ + public function getPriority(): int { + return parent::getPriority() + 2; + } +} diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index e24d5b7a5..ae0de7564 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -27,7 +27,6 @@ namespace OCA\Forms\Controller; -use OCA\Forms\Activity\ActivityManager; use OCA\Forms\Constants; use OCA\Forms\Db\Answer; use OCA\Forms\Db\AnswerMapper; @@ -62,83 +61,27 @@ use Psr\Log\LoggerInterface; class ApiController extends OCSController { - protected $appName; - - /** @var ActivityManager */ - private $activityManager; - - /** @var AnswerMapper */ - private $answerMapper; - - /** @var FormMapper */ - private $formMapper; - - /** @var OptionMapper */ - private $optionMapper; - - /** @var QuestionMapper */ - private $questionMapper; - - /** @var ShareMapper */ - private $shareMapper; - - /** @var SubmissionMapper */ - private $submissionMapper; - - /** @var ConfigService */ - private $configService; - - /** @var FormsService */ - private $formsService; - - /** @var SubmissionService */ - private $submissionService; - - /** @var IL10N */ - private $l10n; - - /** @var LoggerInterface */ - private $logger; - /** @var IUser */ private $currentUser; - /** @var IUserManager */ - private $userManager; - - public function __construct(string $appName, - ActivityManager $activityManager, - AnswerMapper $answerMapper, - FormMapper $formMapper, - OptionMapper $optionMapper, - QuestionMapper $questionMapper, - ShareMapper $shareMapper, - SubmissionMapper $submissionMapper, - ConfigService $configService, - FormsService $formsService, - SubmissionService $submissionService, - IL10N $l10n, - LoggerInterface $logger, + public function __construct( + string $appName, IRequest $request, - IUserManager $userManager, - IUserSession $userSession) { + IUserSession $userSession, + private AnswerMapper $answerMapper, + private FormMapper $formMapper, + private OptionMapper $optionMapper, + private QuestionMapper $questionMapper, + private ShareMapper $shareMapper, + private SubmissionMapper $submissionMapper, + private ConfigService $configService, + private FormsService $formsService, + private SubmissionService $submissionService, + private IL10N $l10n, + private LoggerInterface $logger, + private IUserManager $userManager, + ) { parent::__construct($appName, $request); - $this->appName = $appName; - $this->activityManager = $activityManager; - $this->answerMapper = $answerMapper; - $this->formMapper = $formMapper; - $this->optionMapper = $optionMapper; - $this->questionMapper = $questionMapper; - $this->shareMapper = $shareMapper; - $this->submissionMapper = $submissionMapper; - $this->configService = $configService; - $this->formsService = $formsService; - $this->submissionService = $submissionService; - - $this->l10n = $l10n; - $this->logger = $logger; - $this->userManager = $userManager; - $this->currentUser = $userSession->getUser(); } @@ -1038,7 +981,7 @@ public function insertSubmission(int $formId, array $answers, string $shareHash $this->formsService->setLastUpdatedTimestamp($formId); //Create Activity - $this->activityManager->publishNewSubmission($form, $submission->getUserId()); + $this->formsService->notifyNewSubmission($form, $submission->getUserId()); return new DataResponse(); } diff --git a/lib/Service/FormsService.php b/lib/Service/FormsService.php index 444807f35..e4f2991db 100644 --- a/lib/Service/FormsService.php +++ b/lib/Service/FormsService.php @@ -487,6 +487,25 @@ public function notifyNewShares(Form $form, Share $share): void { } } + /** + * Creates activities for new submissions on a form + * + * @param Form $form Related Form + * @param string $submitter The ID of the user who submitted the form. Can also be our 'anon-user-'-ID + */ + public function notifyNewSubmission(Form $form, string $submitter): void { + $shares = $this->getShares($form->getId()); + $this->activityManager->publishNewSubmission($form, $submitter); + + foreach ($shares as $share) { + if (!in_array(Constants::PERMISSION_RESULTS, $share['permissions'])) { + continue; + } + + $this->activityManager->publishNewSharedSubmission($form, $share['shareType'], $share['shareWith'], $submitter); + } + } + /** * Return shares of a form shared with given user * diff --git a/tests/Unit/Activity/ActivityManagerTest.php b/tests/Unit/Activity/ActivityManagerTest.php index fc4794efd..150addfad 100644 --- a/tests/Unit/Activity/ActivityManagerTest.php +++ b/tests/Unit/Activity/ActivityManagerTest.php @@ -33,6 +33,7 @@ use OCP\IGroupManager; use OCP\IUser; use OCP\IUserSession; +use OCP\Share\IShare; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -175,4 +176,69 @@ public function testPublishNewSubmission() { $this->activityManager->publishNewSubmission($form, $submittorId); } + + public function dataPublichNewSharedSubmission() { + return [ + 'user-share' => [ + 'shareType' => IShare::TYPE_USER, + 'shareWith' => 'sharedUser', + 'expected' => [['sharedUser']] + ], + 'group-share' => [ + IShare::TYPE_GROUP, + 'sharedGroup', + [['user1'], ['user2']], + ['user1', 'user2'], + ] + ]; + } + + /** + * Test notify shared results + * + * @dataProvider dataPublichNewSharedSubmission + */ + public function testPublishNewSharedSubmission(int $shareType, string $shareWith, array $expected, ?array $groupUsers = null) { + // Can't mock the DB-Classes, as their Property-Methods are not explicitely defined. + $form = new Form(); + $form->setId(5); + $form->setTitle('TestForm-Title'); + $form->setHash('abcdefg12345'); + $form->setOwnerId('formOwner'); + $submitterId = 'submittingUser'; + + if ($groupUsers === null) { + $this->groupManager->expects($this->never())->method('get'); + } else { + $users = array_map(function ($name) { + $user = $this->createMock(IUser::class); + $user->expects($this->once())->method('getUID')->willReturn($name); + return $user; + }, $groupUsers); + $group = $this->createMock(IGroup::class); + $group->expects($this->once())->method('getUsers')->willReturn($users); + $this->groupManager->expects($this->once())->method('get')->willReturn($group); + } + + $event = $this->createMock(IEvent::class); + $this->manager->expects($this->exactly(count($expected))) + ->method('generateEvent') + ->willReturn($event); + $event->expects($this->exactly(count($expected)))->method('setApp')->with('forms')->willReturn($event); + $event->expects($this->exactly(count($expected)))->method('setType')->with('forms_newsharedsubmission')->willReturn($event); + $event->expects($this->exactly(count($expected)))->method('setAffectedUser')->withConsecutive(...[...$expected])->willReturn($event); + $event->expects($this->exactly(count($expected)))->method('setAuthor')->with('submittingUser')->willReturn($event); + $event->expects($this->exactly(count($expected)))->method('setObject')->with('form', 5)->willReturn($event); + $event->expects($this->exactly(count($expected)))->method('setSubject')->with('newsubmission', [ + 'userId' => 'submittingUser', + 'formTitle' => 'TestForm-Title', + 'formHash' => 'abcdefg12345' + ])->willReturn($event); + + $this->manager->expects($this->exactly(count($expected))) + ->method('publish') + ->with($event); + + $this->activityManager->publishNewSharedSubmission($form, $shareType, $shareWith, $submitterId); + } } diff --git a/tests/Unit/Activity/Settings/NewSharedSubmissionTest.php b/tests/Unit/Activity/Settings/NewSharedSubmissionTest.php new file mode 100644 index 000000000..17d3da105 --- /dev/null +++ b/tests/Unit/Activity/Settings/NewSharedSubmissionTest.php @@ -0,0 +1,75 @@ + + * + * @author Jonas Rittershofer + * + * @license AGPL-3.0-or-later + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OCA\Forms\Tests\Unit\Activity\Settings; + +use OCA\Forms\Activity\Settings\NewSharedSubmission; +use OCP\IL10N; + +use PHPUnit\Framework\MockObject\MockClass; +use Test\TestCase; + +class NewSharedSubmissionTest extends TestCase { + /** @var IL10N|MockObject */ + private $l10n; + + /** @var NewSharedSubmission|MockClass */ + private $newShare; + + public function setUp(): void { + parent::setUp(); + $this->l10n = $this->createMock(IL10N::class); + $this->newShare = new NewSharedSubmission('forms', $this->l10n); + } + + public function testGetGroupIdentifier() { + $this->assertEquals('forms', $this->newShare->getGroupIdentifier()); + } + + public function testGetGroupName() { + $this->l10n->expects($this->once()) + ->method('t') + ->will($this->returnCallback(function ($identity) { + return $identity; + })); + $this->assertEquals('Forms', $this->newShare->getGroupName()); + } + + public function testGetIdentifier() { + $this->assertEquals('forms_newsharedsubmission', $this->newShare->getIdentifier()); + } + + public function testGetName() { + $this->l10n->expects($this->once()) + ->method('t') + ->will($this->returnCallback(function ($identity) { + return $identity; + })); + $this->assertEquals('Someone answered a shared form', $this->newShare->getName()); + } + + public function testGetPriority() { + $this->assertEquals(62, $this->newShare->getPriority()); + } +} diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php index 1c0d30de9..7eb117e6e 100644 --- a/tests/Unit/Controller/ApiControllerTest.php +++ b/tests/Unit/Controller/ApiControllerTest.php @@ -45,7 +45,6 @@ function time($expected = null) { namespace OCA\Forms\Tests\Unit\Controller; -use OCA\Forms\Activity\ActivityManager; use OCA\Forms\Constants; use OCA\Forms\Controller\ApiController; use OCA\Forms\Db\AnswerMapper; @@ -76,8 +75,6 @@ function time($expected = null) { class ApiControllerTest extends TestCase { private ApiController $apiController; - /** @var ActivityManager|MockObject */ - private $activityManager; /** @var AnswerMapper|MockObject */ private $answerMapper; /** @var FormMapper|MockObject */ @@ -106,7 +103,6 @@ class ApiControllerTest extends TestCase { private $l10n; public function setUp(): void { - $this->activityManager = $this->createMock(ActivityManager::class); $this->answerMapper = $this->createMock(AnswerMapper::class); $this->formMapper = $this->createMock(FormMapper::class); $this->optionMapper = $this->createMock(OptionMapper::class); @@ -125,10 +121,10 @@ public function setUp(): void { ->willReturnCallback(function ($v) { return $v; }); - $this->apiController = new ApiController( 'forms', - $this->activityManager, + $this->request, + $this->createUserSession(), $this->answerMapper, $this->formMapper, $this->optionMapper, @@ -140,9 +136,7 @@ public function setUp(): void { $this->submissionService, $this->l10n, $this->logger, - $this->request, $this->userManager, - $this->createUserSession() ); } @@ -364,6 +358,7 @@ public function dataTestCreateNewForm() { ]] ]; } + /** * @dataProvider dataTestCreateNewForm() */ @@ -373,7 +368,8 @@ public function testCreateNewForm($expectedForm) { $apiController = $this->getMockBuilder(ApiController::class) ->onlyMethods(['getForm']) ->setConstructorArgs(['forms', - $this->activityManager, + $this->request, + $this->createUserSession(), $this->answerMapper, $this->formMapper, $this->optionMapper, @@ -385,9 +381,7 @@ public function testCreateNewForm($expectedForm) { $this->submissionService, $this->l10n, $this->logger, - $this->request, $this->userManager, - $this->createUserSession() ])->getMock(); // Set the time that should be set for `lastUpdated` \OCA\Forms\Controller\time(123456789); @@ -531,7 +525,8 @@ public function testCloneForm($old, $new) { $apiController = $this->getMockBuilder(ApiController::class) ->onlyMethods(['getForm']) ->setConstructorArgs(['forms', - $this->activityManager, + $this->request, + $this->createUserSession(), $this->answerMapper, $this->formMapper, $this->optionMapper, @@ -543,9 +538,7 @@ public function testCloneForm($old, $new) { $this->submissionService, $this->l10n, $this->logger, - $this->request, $this->userManager, - $this->createUserSession() ]) ->getMock(); @@ -660,8 +653,8 @@ public function testInsertSubmission_answers() { ->method('setLastUpdatedTimestamp') ->with(1); - $this->activityManager->expects($this->once()) - ->method('publishNewSubmission') + $this->formsService->expects($this->once()) + ->method('notifyNewSubmission') ->with($form, 'currentUser'); $this->apiController->insertSubmission(1, $answers, ''); diff --git a/tests/Unit/Service/FormsServiceTest.php b/tests/Unit/Service/FormsServiceTest.php index 1f5139314..6deb7c57c 100644 --- a/tests/Unit/Service/FormsServiceTest.php +++ b/tests/Unit/Service/FormsServiceTest.php @@ -1192,4 +1192,87 @@ public function testNotifyNewShares(int $shareType, string $shareWith, string $e $this->formsService->notifyNewShares($form, $share); } + + public function dataNotifyNewSubmission() { + return [ + 'no-shares' => [ + [], + 0 + ], + 'one-share' => [ + [[ + 'shareWith' => 'user', + 'shareType' => IShare::TYPE_USER, + 'permissions' => [ Constants::PERMISSION_RESULTS ] + ]], + 1 + ], + 'one-invalid-share' => [ + [[ + 'shareWith' => 'user', + 'shareType' => IShare::TYPE_USER, + 'permissions' => [ Constants::PERMISSION_SUBMIT ] + ]], + 0 + ], + 'mixed-shares' => [ + [ + [ + 'shareWith' => 'user', + 'shareType' => IShare::TYPE_USER, + 'permissions' => [ Constants::PERMISSION_SUBMIT ] + ], + [ + 'shareWith' => 'user2', + 'shareType' => IShare::TYPE_USER, + 'permissions' => [ Constants::PERMISSION_RESULTS ] + ]], + 1 + ], + ]; + } + + /** + * Test creating notifications for new submissions + * + * @dataProvider dataNotifyNewSubmission + */ + public function testNotifyNewSubmission($shares, $shareNotifications) { + $owner = 'ownerUser'; + $submitter = 'someUser'; + + $userSession = $this->createMock(IUserSession::class); + $userSession->method('getUser')->willReturn(null); + + $formsService = $this->getMockBuilder(FormsService::class) + ->onlyMethods(['getShares']) + ->setConstructorArgs([ + $this->activityManager, + $this->formMapper, + $this->optionMapper, + $this->questionMapper, + $this->shareMapper, + $this->submissionMapper, + $this->configService, + $this->groupManager, + $this->logger, + $this->userManager, + $userSession, + $this->secureRandom + ]) + ->getMock(); + + $form = Form::fromParams(['id' => 42, 'ownerId' => $owner]); + + $formsService->method('getShares')->willReturn($shares); + + $this->activityManager->expects($this->once()) + ->method('publishNewSubmission') + ->with($form, $submitter); + + $this->activityManager->expects($this->exactly($shareNotifications)) + ->method('publishNewSharedSubmission'); + + $formsService->notifyNewSubmission($form, $submitter); + } }