Do not expose activity on every autosave

Signed-off-by: Julius Härtl <jus@bitgrid.net>

appinfo/database.xml

@@ -135,6 +135,11 @@
                 <type>clob</type>
                 <notnull>false</notnull>
             </field>
+            <field>
+                <name>description_prev</name>
+                <type>clob</type>
+                <notnull>false</notnull>
+            </field>
             <field>
                 <name>stack_id</name>
                 <type>integer</type>
@@ -155,6 +160,12 @@
                 <notnull>false</notnull>
                 <unsigned>true</unsigned>
             </field>
+            <field>
+                <name>last_editor</name>
+                <type>text</type>
+                <notnull>false</notnull>
+                <length>64</length>
+            </field>
             <field>
                 <name>created_at</name>
                 <type>integer</type>

appinfo/info.xml

@@ -17,7 +17,7 @@
 - 🚀 Get your project organized
 
 </description>
-    <version>0.5.1-dev1</version>
+    <version>0.5.1-dev2</version>
     <licence>agpl</licence>
     <author>Julius Härtl</author>
     <namespace>Deck</namespace>
@@ -41,6 +41,7 @@
     <background-jobs>
         <job>OCA\Deck\Cron\DeleteCron</job>
         <job>OCA\Deck\Cron\ScheduledNotifications</job>
+        <job>OCA\Deck\Cron\CardDescriptionActivity</job>
     </background-jobs>
     <repair-steps>
         <post-migration>

lib/Activity/ActivityManager.php

@@ -239,10 +239,12 @@ class ActivityManager {
 		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 @@ class ActivityManager {
 		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 @@ class ActivityManager {
 			}
 		} 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 @@ class ActivityManager {
 	 * @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 @@ class ActivityManager {
 		}
 
 		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 @@ class ActivityManager {
 		$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());

lib/Cron/CardDescriptionActivity.php

@@ -0,0 +1,74 @@
+<?php
+/**
+ * @copyright Copyright (c) 2018 Julius Härtl <jus@bitgrid.net>
+ *
+ * @author Julius Härtl <jus@bitgrid.net>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+
+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);
+		}
+	}
+
+}

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 @@ class Card extends RelationalEntity {
 		}
 		$json['duedate'] = $this->getDuedate(true);
 		unset($json['notified']);
+		unset($json['descriptionPrev']);
 		return $json;
 	}
 

lib/Db/CardMapper.php

@@ -147,6 +147,11 @@ class CardMapper extends DeckMapper implements IPermissionMapper {
 		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());

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 @@ class ChangeHelper {
 		$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 = ?';

lib/Service/CardService.php

@@ -259,18 +259,41 @@ class CardService {
 			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;
 	}