From 3d6a3e28fa3fc99cd97f6b6401fa01ae8e5bbc25 Mon Sep 17 00:00:00 2001
From: vrana <jakubv@fb.com>
Date: Mon, 1 Oct 2012 15:50:47 -0700
Subject: [PATCH] Don't store empty drafts

Summary: We have lots of empty drafts in DB.

Test Plan: Wrote revision comment, deleted it, checked db.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3591
---
 .../PhabricatorAuditPreviewController.php     |  2 +-
 .../DifferentialCommentPreviewController.php  | 19 +++++++++++--------
 .../draft/storage/PhabricatorDraft.php        | 13 +++++++++++++
 .../ManiphestTransactionPreviewController.php | 17 +++++------------
 .../PhrictionDocumentPreviewController.php    | 14 +++++---------
 5 files changed, 35 insertions(+), 30 deletions(-)

diff --git a/src/applications/audit/controller/PhabricatorAuditPreviewController.php b/src/applications/audit/controller/PhabricatorAuditPreviewController.php
index 4d023df8fb..4e8f3c688b 100644
--- a/src/applications/audit/controller/PhabricatorAuditPreviewController.php
+++ b/src/applications/audit/controller/PhabricatorAuditPreviewController.php
@@ -75,7 +75,7 @@ final class PhabricatorAuditPreviewController
       ->setAuthorPHID($comment->getActorPHID())
       ->setDraftKey('diffusion-audit-'.$this->id)
       ->setDraft($comment->getContent())
-      ->replace();
+      ->replaceOrDelete();
 
     return id(new AphrontAjaxResponse())
       ->setContent($view->render());
diff --git a/src/applications/differential/controller/DifferentialCommentPreviewController.php b/src/applications/differential/controller/DifferentialCommentPreviewController.php
index c6b7063f6e..2710297be8 100644
--- a/src/applications/differential/controller/DifferentialCommentPreviewController.php
+++ b/src/applications/differential/controller/DifferentialCommentPreviewController.php
@@ -66,17 +66,20 @@ final class DifferentialCommentPreviewController
     $view->setPreview(true);
     $view->setTargetDiff(null);
 
-    $draft = new PhabricatorDraft();
-    $draft
+    $metadata = array(
+      'reviewers' => $reviewers,
+      'ccs' => $ccs,
+    );
+    if ($action != DifferentialAction::ACTION_COMMENT) {
+      $metadata['action'] = $action;
+    }
+
+    id(new PhabricatorDraft())
       ->setAuthorPHID($author_phid)
       ->setDraftKey('differential-comment-'.$this->id)
       ->setDraft($comment->getContent())
-      ->setMetadata(array(
-        'action' => $action,
-        'reviewers' => $reviewers,
-        'ccs' => $ccs,
-      ))
-      ->replace();
+      ->setMetadata($metadata)
+      ->replaceOrDelete();
 
     return id(new AphrontAjaxResponse())
       ->setContent($view->render());
diff --git a/src/applications/draft/storage/PhabricatorDraft.php b/src/applications/draft/storage/PhabricatorDraft.php
index 102f8946d7..cce1008831 100644
--- a/src/applications/draft/storage/PhabricatorDraft.php
+++ b/src/applications/draft/storage/PhabricatorDraft.php
@@ -31,4 +31,17 @@ final class PhabricatorDraft extends PhabricatorDraftDAO {
     ) + parent::getConfiguration();
   }
 
+  public function replaceOrDelete() {
+    if ($this->draft == '' && !array_filter($this->metadata)) {
+      queryfx(
+        $this->establishConnection('w'),
+        'DELETE FROM %T WHERE authorPHID = %s AND draftKey = %s',
+        $this->getTableName(),
+        $this->authorPHID,
+        $this->draftKey);
+      return $this;
+    }
+    return parent::replace();
+  }
+
 }
diff --git a/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php b/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php
index 2206933e78..e8e26de6af 100644
--- a/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php
+++ b/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php
@@ -39,18 +39,11 @@ final class ManiphestTransactionPreviewController extends ManiphestController {
       return new Aphront404Response();
     }
 
-    $draft = id(new PhabricatorDraft())->loadOneWhere(
-      'authorPHID = %s AND draftKey = %s',
-      $user->getPHID(),
-      $task->getPHID());
-    if (!$draft) {
-      $draft = new PhabricatorDraft();
-      $draft->setAuthorPHID($user->getPHID());
-      $draft->setDraftKey($task->getPHID());
-    }
-    $draft->setDraft($comments);
-    $draft->save();
-
+    id(new PhabricatorDraft())
+      ->setAuthorPHID($user->getPHID())
+      ->setDraftKey($task->getPHID())
+      ->setDraft($comments)
+      ->replaceOrDelete();
 
     $action = $request->getStr('action');
 
diff --git a/src/applications/phriction/controller/PhrictionDocumentPreviewController.php b/src/applications/phriction/controller/PhrictionDocumentPreviewController.php
index e5583cc890..51f5f314e0 100644
--- a/src/applications/phriction/controller/PhrictionDocumentPreviewController.php
+++ b/src/applications/phriction/controller/PhrictionDocumentPreviewController.php
@@ -29,15 +29,11 @@ final class PhrictionDocumentPreviewController
 
     $draft_key = $request->getStr('draftkey');
     if ($draft_key) {
-      $table = new PhabricatorDraft();
-      queryfx(
-        $table->establishConnection('w'),
-        'INSERT INTO %T (authorPHID, draftKey, draft) VALUES (%s, %s, %s)
-          ON DUPLICATE KEY UPDATE draft = VALUES(draft)',
-        $table->getTableName(),
-        $request->getUser()->getPHID(),
-        $draft_key,
-        $document);
+      id(new PhabricatorDraft())
+        ->setAuthorPHID($request->getUser()->getPHID())
+        ->setDraftKey($draft_key)
+        ->setDraft($document)
+        ->replaceOrDelete();
     }
 
     $content_obj = new PhrictionContent();