From 43a11327a69ece55cbda3614c82336a205b02f4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Fri, 16 Nov 2018 14:53:45 +0100 Subject: [PATCH 1/3] Add dedicated setting for description change activities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- appinfo/info.xml | 3 +- lib/Activity/ActivityManager.php | 4 ++- lib/Activity/DescriptionSetting.php | 45 +++++++++++++++++++++++++++++ lib/Activity/Setting.php | 14 ++++++++- 4 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 lib/Activity/DescriptionSetting.php diff --git a/appinfo/info.xml b/appinfo/info.xml index a86a57867..3bcaaa55b 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -17,7 +17,7 @@ - 🚀 Get your project organized - 0.5.0 + 0.5.1-dev1 agpl Julius Härtl Deck @@ -53,6 +53,7 @@ OCA\Deck\Activity\Setting + OCA\Deck\Activity\DescriptionSetting OCA\Deck\Activity\Filter diff --git a/lib/Activity/ActivityManager.php b/lib/Activity/ActivityManager.php index ebecf0df6..f498aa17d 100644 --- a/lib/Activity/ActivityManager.php +++ b/lib/Activity/ActivityManager.php @@ -309,6 +309,7 @@ private function createEvent($objectType, $entity, $subject, $additionalParams = * Automatically fetch related details for subject parameters * depending on the subject */ + $eventType = 'deck'; $subjectParams = []; $message = null; switch ($subject) { @@ -372,6 +373,7 @@ private function createEvent($objectType, $entity, $subject, $additionalParams = if ($subject === self::SUBJECT_CARD_UPDATE_DESCRIPTION){ $subjectParams['diff'] = true; + $eventType = 'deck_card_description'; } if ($subject === self::SUBJECT_CARD_UPDATE_STACKID) { $subjectParams['stackBefore'] = $this->stackMapper->find($additionalParams['before']); @@ -382,7 +384,7 @@ private function createEvent($objectType, $entity, $subject, $additionalParams = $event = $this->manager->generateEvent(); $event->setApp('deck') - ->setType('deck') + ->setType($eventType) ->setAuthor($this->userId) ->setObject($objectType, (int)$object->getId(), $object->getTitle()) ->setSubject($subject, array_merge($subjectParams, $additionalParams)) diff --git a/lib/Activity/DescriptionSetting.php b/lib/Activity/DescriptionSetting.php new file mode 100644 index 000000000..787647ffa --- /dev/null +++ b/lib/Activity/DescriptionSetting.php @@ -0,0 +1,45 @@ + + * + * @author Julius Härtl + * + * @license GNU AGPL version 3 or any later version + * + * 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\Deck\Activity; + + +class DescriptionSetting extends Setting { + + /** + * @return string Lowercase a-z and underscore only identifier + * @since 11.0.0 + */ + public function getIdentifier() { + return 'deck_card_description'; + } + + /** + * @return string A translated string + * @since 11.0.0 + */ + public function getName() { + return $this->l->t('A card description inside the Deck app has been changed'); + } + +} diff --git a/lib/Activity/Setting.php b/lib/Activity/Setting.php index 12080f263..9da38530c 100644 --- a/lib/Activity/Setting.php +++ b/lib/Activity/Setting.php @@ -24,8 +24,20 @@ namespace OCA\Deck\Activity; +use OCP\IL10N; + class Setting implements \OCP\Activity\ISetting { + /** @var IL10N */ + protected $l; + + /** + * @param IL10N $l + */ + public function __construct(IL10N $l) { + $this->l = $l; + } + /** * @return string Lowercase a-z and underscore only identifier * @since 11.0.0 @@ -39,7 +51,7 @@ public function getIdentifier() { * @since 11.0.0 */ public function getName() { - return 'Deck'; + return $this->l->t('Changes in the Deck app'); } /** From dd104466d61e32f59552da183034522e04effe35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 4 Dec 2018 14:44:31 +0100 Subject: [PATCH 2/3] Do not expose activity on every autosave MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- appinfo/database.xml | 11 +++++ appinfo/info.xml | 3 +- lib/Activity/ActivityManager.php | 28 +++++++---- lib/Cron/CardDescriptionActivity.php | 74 ++++++++++++++++++++++++++++ lib/Db/Card.php | 3 ++ lib/Db/CardMapper.php | 5 ++ lib/Db/ChangeHelper.php | 8 +-- lib/Service/CardService.php | 29 +++++++++-- 8 files changed, 144 insertions(+), 17 deletions(-) create mode 100644 lib/Cron/CardDescriptionActivity.php diff --git a/appinfo/database.xml b/appinfo/database.xml index a96137c42..6cbbe952d 100644 --- a/appinfo/database.xml +++ b/appinfo/database.xml @@ -135,6 +135,11 @@ clob false + + description_prev + clob + false + stack_id integer @@ -155,6 +160,12 @@ false true + + last_editor + text + false + 64 + created_at integer diff --git a/appinfo/info.xml b/appinfo/info.xml index 3bcaaa55b..f99ac2178 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -17,7 +17,7 @@ - 🚀 Get your project organized - 0.5.1-dev1 + 0.5.1-dev2 agpl Julius Härtl Deck @@ -41,6 +41,7 @@ OCA\Deck\Cron\DeleteCron OCA\Deck\Cron\ScheduledNotifications + OCA\Deck\Cron\CardDescriptionActivity diff --git a/lib/Activity/ActivityManager.php b/lib/Activity/ActivityManager.php index f498aa17d..a1e4d82fd 100644 --- a/lib/Activity/ActivityManager.php +++ b/lib/Activity/ActivityManager.php @@ -239,10 +239,12 @@ public function getActivityFormat($subjectIdentifier, $subjectParams = [], $ownA return $subject; } - public function triggerEvent($objectType, $entity, $subject, $additionalParams = []) { + public function triggerEvent($objectType, $entity, $subject, $additionalParams = [], $author = null) { try { - $event = $this->createEvent($objectType, $entity, $subject, $additionalParams); - $this->sendToUsers($event); + $event = $this->createEvent($objectType, $entity, $subject, $additionalParams, $author); + if ($event !== null) { + $this->sendToUsers($event); + } } catch (\Exception $e) { // Ignore exception for undefined activities on update events } @@ -262,15 +264,17 @@ public function triggerUpdateEvents($objectType, ChangeSet $changeSet, $subject) if ($previousEntity !== null) { foreach ($entity->getUpdatedFields() as $field => $value) { $getter = 'get' . ucfirst($field); - $subject = $subject . '_' . $field; + $subjectComplete = $subject . '_' . $field; $changes = [ 'before' => $previousEntity->$getter(), 'after' => $entity->$getter() ]; if ($changes['before'] !== $changes['after']) { try { - $event = $this->createEvent($objectType, $entity, $subject, $changes); - $events[] = $event; + $event = $this->createEvent($objectType, $entity, $subjectComplete, $changes); + if ($event !== null) { + $events[] = $event; + } } catch (\Exception $e) { // Ignore exception for undefined activities on update events } @@ -278,7 +282,7 @@ public function triggerUpdateEvents($objectType, ChangeSet $changeSet, $subject) } } else { try { - $events = [$this->createEvent($objectType, $entity, $subject)]; + $events = [$this->createEvent($objectType, $entity, $subject, $author)]; } catch (\Exception $e) { // Ignore exception for undefined activities on update events } @@ -293,10 +297,10 @@ public function triggerUpdateEvents($objectType, ChangeSet $changeSet, $subject) * @param $entity * @param $subject * @param array $additionalParams - * @return IEvent + * @return IEvent|null * @throws \Exception */ - private function createEvent($objectType, $entity, $subject, $additionalParams = []) { + private function createEvent($objectType, $entity, $subject, $additionalParams = [], $author = null) { try { $object = $this->findObjectForEntity($objectType, $entity); } catch (DoesNotExistException $e) { @@ -372,6 +376,10 @@ private function createEvent($objectType, $entity, $subject, $additionalParams = } if ($subject === self::SUBJECT_CARD_UPDATE_DESCRIPTION){ + $card = $subjectParams['card']; + if ($card->getLastEditor() === $this->userId) { + return null; + } $subjectParams['diff'] = true; $eventType = 'deck_card_description'; } @@ -385,7 +393,7 @@ private function createEvent($objectType, $entity, $subject, $additionalParams = $event = $this->manager->generateEvent(); $event->setApp('deck') ->setType($eventType) - ->setAuthor($this->userId) + ->setAuthor($author === null ? $this->userId : $author) ->setObject($objectType, (int)$object->getId(), $object->getTitle()) ->setSubject($subject, array_merge($subjectParams, $additionalParams)) ->setTimestamp(time()); diff --git a/lib/Cron/CardDescriptionActivity.php b/lib/Cron/CardDescriptionActivity.php new file mode 100644 index 000000000..bdd4226f1 --- /dev/null +++ b/lib/Cron/CardDescriptionActivity.php @@ -0,0 +1,74 @@ + + * + * @author Julius Härtl + * + * @license GNU AGPL version 3 or any later version + * + * 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\Deck\Cron; + +use OC\BackgroundJob\Job; +use OCA\Deck\Activity\ActivityManager; +use OCA\Deck\Activity\ChangeSet; +use OCA\Deck\Db\AttachmentMapper; +use OCA\Deck\Db\BoardMapper; +use OCA\Deck\Db\Card; +use OCA\Deck\Db\CardMapper; +use OCA\Deck\InvalidAttachmentType; +use OCA\Deck\Service\AttachmentService; +use OCA\Deck\Service\CardService; + +class CardDescriptionActivity extends Job { + + /** @var ActivityManager */ + private $activityManager; + /** @var CardMapper */ + private $cardMapper; + + public function __construct(ActivityManager $activityManager, CardMapper $cardMapper) { + $this->activityManager = $activityManager; + $this->cardMapper = $cardMapper; + } + + /** + * @param $argument + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ + public function run($argument) { + $cards = $this->cardMapper->findUnexposedDescriptionChances(); + foreach ($cards as $card) { + $this->activityManager->triggerEvent( + ActivityManager::DECK_OBJECT_CARD, + $card, + ActivityManager::SUBJECT_CARD_UPDATE_DESCRIPTION, + [ + 'before' => $card->getDescriptionPrev(), + 'after' => $card->getDescription() + ], + $card->getLastEditor() + ); + + $card->setDescriptionPrev(null); + $card->setLastEditor(null); + $this->cardMapper->update($card, false); + } + } + +} diff --git a/lib/Db/Card.php b/lib/Db/Card.php index 91b580ada..7dbf22738 100644 --- a/lib/Db/Card.php +++ b/lib/Db/Card.php @@ -29,9 +29,11 @@ class Card extends RelationalEntity { protected $title; protected $description; + protected $descriptionPrev; protected $stackId; protected $type; protected $lastModified; + protected $lastEditor; protected $createdAt; protected $labels; protected $assignedUsers; @@ -113,6 +115,7 @@ public function jsonSerialize() { } $json['duedate'] = $this->getDuedate(true); unset($json['notified']); + unset($json['descriptionPrev']); return $json; } diff --git a/lib/Db/CardMapper.php b/lib/Db/CardMapper.php index 9cbcc49b6..554dee465 100644 --- a/lib/Db/CardMapper.php +++ b/lib/Db/CardMapper.php @@ -147,6 +147,11 @@ public function findOverdue() { return $this->findEntities($sql); } + public function findUnexposedDescriptionChances() { + $sql = 'SELECT id,title,duedate,notified,description_prev,last_editor,description from `*PREFIX*deck_cards` WHERE last_editor IS NOT NULL AND description_prev IS NOT NULL'; + return $this->findEntities($sql); + } + public function delete(Entity $entity) { // delete assigned labels $this->labelMapper->deleteLabelAssignmentsForCard($entity->getId()); diff --git a/lib/Db/ChangeHelper.php b/lib/Db/ChangeHelper.php index f90545ebe..4bcc6b4b1 100644 --- a/lib/Db/ChangeHelper.php +++ b/lib/Db/ChangeHelper.php @@ -41,11 +41,13 @@ class ChangeHelper { public function __construct( IDBConnection $db, ICacheFactory $cacheFactory, - IRequest $request + IRequest $request, + $userId ) { $this->db = $db; $this->cache = $cacheFactory->createDistributed('deck_changes'); $this->request = $request; + $this->userId = $userId; } public function boardChanged($boardId) { @@ -61,8 +63,8 @@ public function cardChanged($cardId, $updateCard = true) { $etag = md5($time . microtime()); $this->cache->set(self::TYPE_CARD . '-' .$cardId, $etag); if ($updateCard) { - $sql = 'UPDATE `*PREFIX*deck_cards` SET `last_modified` = ? WHERE `id` = ?'; - $this->db->executeUpdate($sql, [time(), $cardId]); + $sql = 'UPDATE `*PREFIX*deck_cards` SET `last_modified` = ?, `last_editor` = ? WHERE `id` = ?'; + $this->db->executeUpdate($sql, [time(), $this->userId, $cardId]); } $sql = 'SELECT s.board_id as id, c.stack_id as stack_id FROM `*PREFIX*deck_stacks` as s inner join `*PREFIX*deck_cards` as c ON c.stack_id = s.id WHERE c.id = ?'; diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index 0d7e7928e..94d773524 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -259,18 +259,41 @@ public function update($id, $title, $stackId, $type, $order = 0, $description = throw new StatusException('Operation not allowed. This card is archived.'); } $changes = new ChangeSet($card); + if ($card->getLastEditor() !== $this->currentUser && $card->getLastEditor() !== null) { + $this->activityManager->triggerEvent( + ActivityManager::DECK_OBJECT_CARD, + $card, + ActivityManager::SUBJECT_CARD_UPDATE_DESCRIPTION, + [ + 'before' => $card->getDescriptionPrev(), + 'after' => $card->getDescription() + ], + $card->getLastEditor() + ); + + $card->setDescriptionPrev($card->getDescription()); + $card->setLastEditor($this->currentUser); + } $card->setTitle($title); $card->setStackId($stackId); $card->setType($type); $card->setOrder($order); $card->setOwner($owner); - $card->setDescription($description); $card->setDuedate($duedate); $card->setDeletedAt($deletedAt); + + // Trigger update events before setting description as it is handled separately $changes->setAfter($card); - $card = $this->cardMapper->update($card); $this->activityManager->triggerUpdateEvents(ActivityManager::DECK_OBJECT_CARD, $changes, ActivityManager::SUBJECT_CARD_UPDATE); - $this->changeHelper->cardChanged($card->getId(), false); + + if ($card->getDescriptionPrev() === null) { + $card->setDescriptionPrev($card->getDescription()); + } + $card->setDescription($description); + + + $card = $this->cardMapper->update($card); + $this->changeHelper->cardChanged($card->getId(), true); return $card; } From 4279e09cc2eff4caf77cbb8eb15c74b6c39e5ad0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 5 Dec 2018 13:22:17 +0100 Subject: [PATCH 3/3] Fix tests for new setting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- tests/unit/Activity/SettingTest.php | 9 +++++++-- tests/unit/Db/CardTest.php | 3 +++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/unit/Activity/SettingTest.php b/tests/unit/Activity/SettingTest.php index 956d5c1d5..5b64df442 100644 --- a/tests/unit/Activity/SettingTest.php +++ b/tests/unit/Activity/SettingTest.php @@ -23,15 +23,20 @@ namespace OCA\Deck\Activity; +use OCP\IL10N; use PHPUnit\Framework\TestCase; class SettingTest extends TestCase { + /** @var IL10N */ + private $l10n; /** @var Setting */ private $setting; public function setUp() { - $this->setting = new Setting(); + $this->l10n = $this->createMock(IL10N::class); + $this->l10n->expects($this->any())->method('t')->will($this->returnCallback(function ($s) { return $s; })); + $this->setting = new Setting($this->l10n); } public function testGetIdentifier() { @@ -39,7 +44,7 @@ public function testGetIdentifier() { } public function testGetName() { - $this->assertEquals('Deck', $this->setting->getName()); + $this->assertEquals('Changes in the Deck app', $this->setting->getName()); } public function testGetPriority() { diff --git a/tests/unit/Db/CardTest.php b/tests/unit/Db/CardTest.php index 0fc3c4b6d..ff8aeab39 100644 --- a/tests/unit/Db/CardTest.php +++ b/tests/unit/Db/CardTest.php @@ -83,6 +83,7 @@ public function testJsonSerialize() { 'assignedUsers' => null, 'deletedAt' => 0, 'commentsUnread' => 0, + 'lastEditor' => null, ], $card->jsonSerialize()); } public function testJsonSerializeLabels() { @@ -107,6 +108,7 @@ public function testJsonSerializeLabels() { 'assignedUsers' => null, 'deletedAt' => 0, 'commentsUnread' => 0, + 'lastEditor' => null, ], $card->jsonSerialize()); } @@ -141,6 +143,7 @@ public function testJsonSerializeAsignedUsers() { 'assignedUsers' => ['user1'], 'deletedAt' => 0, 'commentsUnread' => 0, + 'lastEditor' => null, ], $card->jsonSerialize()); }