diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 76801ed2..493a8aab 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -11,6 +11,7 @@ use OCA\Approval\Dav\ApprovalPlugin; use OCA\Approval\Listener\LoadAdditionalScriptsListener; use OCA\Approval\Listener\LoadSidebarScripts; +use OCA\Approval\Listener\UpdateFilesListener; use OCA\Approval\Notification\Notifier; use OCA\Approval\Service\ApprovalService; @@ -22,6 +23,7 @@ use OCP\AppFramework\Bootstrap\IRegistrationContext; use OCP\EventDispatcher\IEventDispatcher; +use OCP\FilesMetadata\Event\MetadataBackgroundEvent; use OCP\SabrePluginEvent; use OCP\SystemTag\MapperEvent; @@ -63,6 +65,7 @@ public function register(IRegistrationContext $context): void { $context->registerEventListener(LoadSidebar::class, LoadSidebarScripts::class); $context->registerNotifierService(Notifier::class); $context->registerDashboardWidget(ApprovalPendingWidget::class); + $context->registerEventListener(MetadataBackgroundEvent::class, UpdateFilesListener::class); } public function boot(IBootContext $context): void { diff --git a/lib/Controller/ConfigController.php b/lib/Controller/ConfigController.php index 397041fc..97c0be14 100644 --- a/lib/Controller/ConfigController.php +++ b/lib/Controller/ConfigController.php @@ -110,11 +110,12 @@ public function getRules(): DataResponse { * @param array $approvers * @param array $requesters * @param string $description + * @param bool $unapproveWhenModified * @return DataResponse */ public function createRule(int $tagPending, int $tagApproved, int $tagRejected, - array $approvers, array $requesters, string $description): DataResponse { - $result = $this->ruleService->createRule($tagPending, $tagApproved, $tagRejected, $approvers, $requesters, $description); + array $approvers, array $requesters, string $description, bool $unapproveWhenModified): DataResponse { + $result = $this->ruleService->createRule($tagPending, $tagApproved, $tagRejected, $approvers, $requesters, $description, $unapproveWhenModified); return isset($result['error']) ? new DataResponse($result, 400) : new DataResponse($result['id']); @@ -128,11 +129,12 @@ public function createRule(int $tagPending, int $tagApproved, int $tagRejected, * @param array $approvers * @param array $requesters * @param string $description + * @param bool $unapproveWhenModified * @return DataResponse */ public function saveRule(int $id, int $tagPending, int $tagApproved, int $tagRejected, - array $approvers, array $requesters, string $description): DataResponse { - $result = $this->ruleService->saveRule($id, $tagPending, $tagApproved, $tagRejected, $approvers, $requesters, $description); + array $approvers, array $requesters, string $description, bool $unapproveWhenModified): DataResponse { + $result = $this->ruleService->saveRule($id, $tagPending, $tagApproved, $tagRejected, $approvers, $requesters, $description, $unapproveWhenModified); return isset($result['error']) ? new DataResponse($result, 400) : new DataResponse($result['id']); diff --git a/lib/Listener/UpdateFilesListener.php b/lib/Listener/UpdateFilesListener.php new file mode 100644 index 00000000..af8ea6aa --- /dev/null +++ b/lib/Listener/UpdateFilesListener.php @@ -0,0 +1,32 @@ + */ +class UpdateFilesListener implements IEventListener { + + public function __construct( + private ApprovalService $approvalService, + ) { + } + + /** + * @inheritDoc + */ + public function handle(Event $event): void { + if (!($event instanceof MetadataBackgroundEvent)) { + return; + } + $fileNode = $event->getNode(); + $this->approvalService->removeApprovalTags($fileNode); + } +} diff --git a/lib/Migration/Version020301Date20250618110518.php b/lib/Migration/Version020301Date20250618110518.php new file mode 100644 index 00000000..09cf493b --- /dev/null +++ b/lib/Migration/Version020301Date20250618110518.php @@ -0,0 +1,71 @@ +hasTable('approval_rules')) { + $table = $schema->getTable('approval_rules'); + if (!$table->hasColumn('unapprove_when_modified')) { + $table->addColumn('unapprove_when_modified', Types::SMALLINT, [ + 'default' => 0, + 'notnull' => true + ]); + } + } + + return $schema; + } + + /** + * @param IOutput $output + * @param Closure(): ISchemaWrapper $schemaClosure + * @param array $options + */ + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + // Not sure if this is needed, but it can't hurt + $qbUpdate = $this->connection->getQueryBuilder(); + $qbUpdate->update('approval_rules') + ->set('unapprove_when_modified', $qbUpdate->expr()->literal(0)) + ->executeStatement(); + } +} diff --git a/lib/Service/ApprovalService.php b/lib/Service/ApprovalService.php index 6f56a86f..9be7374b 100644 --- a/lib/Service/ApprovalService.php +++ b/lib/Service/ApprovalService.php @@ -14,6 +14,7 @@ use OCP\App\IAppManager; use OCP\Files\FileInfo; use OCP\Files\IRootFolder; +use OCP\Files\Node; use OCP\IGroupManager; use OCP\IL10N; use OCP\IUser; @@ -815,4 +816,24 @@ public function propFind(PropFind $propFind, INode $node): void { } ); } + + /** + * Remove approval tag from a file + * + * @param Node $file + */ + public function removeApprovalTags(Node $file): void { + $fileId = $file->getId(); + $fileTags = $this->tagObjectMapper->getTagIdsForObjects([$fileId], 'files'); + $fileTags = $fileTags[$fileId] ?? []; + if (count($fileTags) > 0) { + $tags = $this->ruleService->filterApprovalTags($fileTags); + foreach ($tags as $tag) { + $mTime = $file->getMTime(); + if ($this->ruleService->wasApprovedAfter($fileId, $mTime)) { + $this->tagObjectMapper->unassignTags((string)$fileId, 'files', $tag); + } + } + } + } } diff --git a/lib/Service/RuleService.php b/lib/Service/RuleService.php index d312cb53..f7e391d5 100644 --- a/lib/Service/RuleService.php +++ b/lib/Service/RuleService.php @@ -11,6 +11,7 @@ use OCA\Approval\AppInfo\Application; use OCP\App\IAppManager; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\ICacheFactory; use OCP\IDBConnection; use OCP\IUserManager; @@ -27,6 +28,7 @@ public function __construct( private IDBConnection $db, private IUserManager $userManager, private IAppManager $appManager, + private ICacheFactory $cacheFactory, ) { $this->strTypeToInt = [ 'user' => Application::TYPE_USER, @@ -102,11 +104,12 @@ private function hasConflict(?int $id, int $tagPending): bool { * @param array $approvers * @param array $requesters * @param string $description + * @param bool $unapproveWhenModified * @return array Error string or id of saved rule * @throws \OCP\DB\Exception */ public function saveRule(int $id, int $tagPending, int $tagApproved, int $tagRejected, - array $approvers, array $requesters, string $description): array { + array $approvers, array $requesters, string $description, bool $unapproveWhenModified): array { $this->cachedRules = null; if (!$this->isValid($tagPending, $tagApproved, $tagRejected)) { return ['error' => 'Invalid rule']; @@ -122,11 +125,13 @@ public function saveRule(int $id, int $tagPending, int $tagApproved, int $tagRej $qb->set('tag_approved', $qb->createNamedParameter($tagApproved, IQueryBuilder::PARAM_INT)); $qb->set('tag_rejected', $qb->createNamedParameter($tagRejected, IQueryBuilder::PARAM_INT)); $qb->set('description', $qb->createNamedParameter($description, IQueryBuilder::PARAM_STR)); + $qb->set('unapprove_when_modified', $qb->createNamedParameter($unapproveWhenModified ? 1 : 0, IQueryBuilder::PARAM_INT)); $qb->where( $qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT)) ); $qb->executeStatement(); $qb = $qb->resetQueryParts(); + $this->clearRuleCaches(); $rule = $this->getRule($id); @@ -211,10 +216,11 @@ public function saveRule(int $id, int $tagPending, int $tagApproved, int $tagRej * @param array $approvers * @param array $requesters * @param string $description + * @param bool $unapproveWhenModified * @return array id of created rule or error string */ public function createRule(int $tagPending, int $tagApproved, int $tagRejected, - array $approvers, array $requesters, string $description): array { + array $approvers, array $requesters, string $description, bool $unapproveWhenModified): array { $this->cachedRules = null; if (!$this->isValid($tagPending, $tagApproved, $tagRejected)) { return ['error' => 'Rule is invalid']; @@ -231,9 +237,11 @@ public function createRule(int $tagPending, int $tagApproved, int $tagRejected, 'tag_approved' => $qb->createNamedParameter($tagApproved, IQueryBuilder::PARAM_INT), 'tag_rejected' => $qb->createNamedParameter($tagRejected, IQueryBuilder::PARAM_INT), 'description' => $qb->createNamedParameter($description, IQueryBuilder::PARAM_STR), + 'unapprove_when_modified' => $qb->createNamedParameter($unapproveWhenModified ? 1 : 0, IQueryBuilder::PARAM_INT), ]); $qb->executeStatement(); $qb = $qb->resetQueryParts(); + $this->clearRuleCaches(); $insertedRuleId = $qb->getLastInsertId(); @@ -302,6 +310,7 @@ public function deleteRule(int $id): array { ); $qb->executeStatement(); $qb->resetQueryParts(); + $this->clearRuleCaches(); return []; } @@ -377,6 +386,7 @@ public function getRules(): array { 'description' => $description, 'approvers' => [], 'requesters' => [], + 'unapproveWhenModified' => (int)$row['unapprove_when_modified'] === 1, ]; } $req->closeCursor(); @@ -500,4 +510,78 @@ public function getLastAction(int $fileId, int $ruleId, int $newState): ?array { } return $activity; } + + /** + * Gets all approval tags that should be unapproved when a file is modified + * + * @param array $tags + * @return array of filtered approval tags + */ + public function getApprovalTags(): array { + $cache = $this->cacheFactory->createDistributed(Application::APP_ID); + if ($cached = $cache->get('approval_tags')) { + return $cached; + } + $qb = $this->db->getQueryBuilder(); + $qb->selectDistinct('tag_approved')->from('approval_rules') + ->where($qb->expr() + ->eq('unapprove_when_modified', $qb->expr()->literal(1)) + ); + $req = $qb->executeQuery(); + $approvalTags = $req->fetchAll(); + $approvalTags = array_map(function ($tag) { + return $tag['tag_approved']; + }, $approvalTags); + $req->closeCursor(); + $cache->set('approval_tags', $approvalTags, 3600); + return $approvalTags; + } + + /** + * Check if a list of tags contains an approval tags that should be unapproved + * when the file is modified + * + * @param array $tags + * @return array of filtered approval tags + */ + public function filterApprovalTags(array $tags): array { + $approvalTags = $this->getApprovalTags(); + return array_filter($tags, function ($tag) use ($approvalTags) { + return in_array($tag, $approvalTags); + }); + } + /** + * Clear caches based on rule changes + */ + public function clearRuleCaches(): void { + $cache = $this->cacheFactory->createDistributed(Application::APP_ID); + $cache->remove('approval_tags'); + } + + /** + * Checks that the approval of the file was after the time given. + * This does not verify that the file was actually approved. + * + * @param int $fileId + * @param int $time + * @return bool + */ + public function wasApprovedAfter(int $fileId, int $time): bool { + $qb = $this->db->getQueryBuilder(); + $qb->select('timestamp') + ->from('approval_activity') + ->where( + $qb->expr()->eq('file_id', $qb->createNamedParameter($fileId, IQueryBuilder::PARAM_INT)) + ) + ->andWhere( + $qb->expr()->eq('new_state', $qb->createNamedParameter(Application::STATE_APPROVED, IQueryBuilder::PARAM_INT)) + ); + $req = $qb->executeQuery(); + $timestamp = $req->fetchOne(); + $req->closeCursor(); + if (!$timestamp) { + return true; + } + return $timestamp < $time; + } } diff --git a/src/components/AdminSettings.vue b/src/components/AdminSettings.vue index 1dabf84a..231544b3 100644 --- a/src/components/AdminSettings.vue +++ b/src/components/AdminSettings.vue @@ -254,6 +254,7 @@ export default { entityId: u.entityId, } }), + unapproveWhenModified: rule.unapproveWhenModified, } const url = generateUrl('/apps/approval/rule/' + id) axios.put(url, req).then((response) => { @@ -279,6 +280,7 @@ export default { description: '', approvers: [], requesters: [], + unapproveWhenModified: 'false', } }, onNewRuleDelete() { @@ -306,6 +308,7 @@ export default { entityId: u.entityId, } }), + unapproveWhenModified: rule.unapproveWhenModified, } const url = generateUrl('/apps/approval/rule') axios.post(url, req).then((response) => { diff --git a/src/components/ApprovalRule.vue b/src/components/ApprovalRule.vue index 7f0e7651..72a3abb0 100644 --- a/src/components/ApprovalRule.vue +++ b/src/components/ApprovalRule.vue @@ -108,6 +108,13 @@ :limit="null" @input="update('tagRejected', $event)" /> +
+ + {{ checkboxLabel }} + +
@@ -120,6 +127,7 @@ import TagIcon from 'vue-material-design-icons/Tag.vue' import NcSelectTags from '@nextcloud/vue/dist/Components/NcSelectTags.js' +import NcCheckboxRadioSwitch from '@nextcloud/vue/dist/Components/NcCheckboxRadioSwitch.js' import { delay } from '../utils.js' import MultiselectWho from './MultiselectWho.vue' @@ -133,6 +141,7 @@ export default { MultiselectWho, NcSelectTags, TagIcon, + NcCheckboxRadioSwitch, }, props: { @@ -159,6 +168,7 @@ export default { rejectedLabel: t('approval', 'Tag set on rejection'), descriptionLabel: t('approval', 'Workflow title'), descriptionPlaceholder: t('approval', 'What is the purpose of this workflow?'), + checkboxLabel: t('approval', 'Remove file approval when file is modified'), } }, @@ -185,7 +195,7 @@ export default { }, update(key, value) { console.debug('update', key, value) - if (value) { + if (value || value === false) { const backupRule = { ...this.value, approvers: this.value.approvers.map(e => e), @@ -228,6 +238,7 @@ export default { .tag, .text, + .checkbox, .users { display: flex; flex-direction: column; diff --git a/tests/unit/Service/ApprovalServiceTest.php b/tests/unit/Service/ApprovalServiceTest.php index 03506ed7..93ae911d 100644 --- a/tests/unit/Service/ApprovalServiceTest.php +++ b/tests/unit/Service/ApprovalServiceTest.php @@ -13,6 +13,7 @@ use OCA\Approval\AppInfo\Application; use OCP\App\IAppManager; use OCP\Files\IRootFolder; +use OCP\ICacheFactory; use OCP\IConfig; use OCP\IDBConnection; use OCP\IGroupManager; @@ -114,7 +115,8 @@ protected function setUp(): void { 'approval', $c->get(IDBConnection::class), $c->get(IUserManager::class), - $c->get(IAppManager::class) + $c->get(IAppManager::class), + $c->get(ICacheFactory::class) ); $this->approvalService = new ApprovalService( 'approval', @@ -157,7 +159,7 @@ protected function setUp(): void { $this->description1 = 'desc1'; $r = $this->ruleService->createRule( $this->idTagPending1, $this->idTagApproved1, $this->idTagRejected1, - $this->approvers1, $this->requesters1, $this->description1 + $this->approvers1, $this->requesters1, $this->description1, false ); $this->idRule1 = $r['id']; } @@ -229,7 +231,7 @@ public function testCreateAndDeleteRule() { $description2 = 'desc2'; $r = $this->ruleService->createRule( $idTagPending2, $idTagApproved2, $idTagRejected2, - $approvers2, $requesters2, $description2 + $approvers2, $requesters2, $description2, false ); $idRule2 = $r['id']; @@ -297,10 +299,10 @@ public function testGetApprovalState() { $this->assertEquals(Application::STATE_REJECTED, $state['state']); // failure because tag does not exist - $this->ruleService->saveRule($this->idRule1, -1, -2, -3, $this->approvers1, $this->requesters1, $this->description1); + $this->ruleService->saveRule($this->idRule1, -1, -2, -3, $this->approvers1, $this->requesters1, $this->description1, false); $state = $this->approvalService->getApprovalState($file1->getId(), 'user1'); $this->assertEquals(Application::STATE_NOTHING, $state['state']); - $this->ruleService->saveRule($this->idRule1, $this->idTagPending1, $this->idTagApproved1, $this->idTagRejected1, $this->approvers1, $this->requesters1, $this->description1); + $this->ruleService->saveRule($this->idRule1, $this->idTagPending1, $this->idTagApproved1, $this->idTagRejected1, $this->approvers1, $this->requesters1, $this->description1, false); } // test request/approve/reject @@ -337,9 +339,10 @@ public function testApproval() { ], ]; $description = 'desc3'; + $unapproveWhenModified = false; $r = $this->ruleService->createRule( $idTagPending3, $idTagApproved3, $idTagRejected3, - $approvers, $requesters, $description + $approvers, $requesters, $description, $unapproveWhenModified ); $idRule3 = $r['id']; @@ -371,10 +374,10 @@ public function testApproval() { 'type' => 'user', ], ]; - $this->ruleService->saveRule($idRule3, $idTagPending3, $idTagApproved3, $idTagRejected3, $approversFailure, $requesters, $description); + $this->ruleService->saveRule($idRule3, $idTagPending3, $idTagApproved3, $idTagRejected3, $approversFailure, $requesters, $description, $unapproveWhenModified); $result = $this->approvalService->request($otherFile->getId(), $idRule3, 'user1', false); $this->assertTrue(isset($result['warning'])); - $this->ruleService->saveRule($idRule3, $idTagPending3, $idTagApproved3, $idTagRejected3, $approvers, $requesters, $description); + $this->ruleService->saveRule($idRule3, $idTagPending3, $idTagApproved3, $idTagRejected3, $approvers, $requesters, $description, $unapproveWhenModified); // get state $stateForUser1 = $this->approvalService->getApprovalState($fileToApprove->getId(), 'user1'); @@ -391,10 +394,10 @@ public function testApproval() { // approve failures // tag does not exist - $this->ruleService->saveRule($idRule3, $idTagPending3, -1, $idTagRejected3, $approvers, $requesters, $description); + $this->ruleService->saveRule($idRule3, $idTagPending3, -1, $idTagRejected3, $approvers, $requesters, $description, $unapproveWhenModified); $result = $this->approvalService->approve($fileToReject->getId(), 'user1'); $this->assertFalse($result); - $this->ruleService->saveRule($idRule3, $idTagPending3, $idTagApproved3, $idTagRejected3, $approvers, $requesters, $description); + $this->ruleService->saveRule($idRule3, $idTagPending3, $idTagApproved3, $idTagRejected3, $approvers, $requesters, $description, $unapproveWhenModified); // approve $this->approvalService->approve($fileToApprove->getId(), 'user1'); @@ -403,10 +406,10 @@ public function testApproval() { // reject failures // tag does not exist - $this->ruleService->saveRule($idRule3, $idTagPending3, $idTagApproved3, -1, $approvers, $requesters, $description); + $this->ruleService->saveRule($idRule3, $idTagPending3, $idTagApproved3, -1, $approvers, $requesters, $description, $unapproveWhenModified); $result = $this->approvalService->reject($fileToReject->getId(), 'user1'); $this->assertFalse($result); - $this->ruleService->saveRule($idRule3, $idTagPending3, $idTagApproved3, $idTagRejected3, $approvers, $requesters, $description); + $this->ruleService->saveRule($idRule3, $idTagPending3, $idTagApproved3, $idTagRejected3, $approvers, $requesters, $description, $unapproveWhenModified); // reject $this->approvalService->reject($fileToReject->getId(), 'user1');