Show command transactions in Harbormaster builds

Summary: Create transaction, editor, etc, and move command generation over to editor. Show in a timeline in the buildable page. Also prevent Engine from creating an empty transaction when build starts (Fixes T4885). Fixes T4886. Test Plan: Restart builds and buildables, look at timeline. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley, Korvin Maniphest Tasks: T4885, T4886 Differential Revision: https://secure.phabricator.com/D9110
2024-11-22 14:52:41 +01:00 · 2014-05-15 07:02:30 -07:00 · 2014-05-15 07:02:30 -07:00 · f2c0e94ea8
commit f2c0e94ea8
parent 7f22958a82
14 changed files with 505 additions and 33 deletions
--- a/resources/sql/autopatches/20140514.harbormasterbuildabletransaction.sql
+++ b/resources/sql/autopatches/20140514.harbormasterbuildabletransaction.sql
@ -0,0 +1,43 @@
+CREATE TABLE {$NAMESPACE}_harbormaster.harbormaster_buildabletransaction (
+  id INT UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT,
+  phid VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  authorPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  objectPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  viewPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  editPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  commentPHID VARCHAR(64) COLLATE utf8_bin,
+  commentVersion INT UNSIGNED NOT NULL,
+  transactionType VARCHAR(32) NOT NULL COLLATE utf8_bin,
+  oldValue LONGTEXT NOT NULL COLLATE utf8_bin,
+  newValue LONGTEXT NOT NULL COLLATE utf8_bin,
+  contentSource LONGTEXT NOT NULL COLLATE utf8_bin,
+  metadata LONGTEXT NOT NULL COLLATE utf8_bin,
+  dateCreated INT UNSIGNED NOT NULL,
+  dateModified INT UNSIGNED NOT NULL,
+
+  UNIQUE KEY `key_phid` (phid),
+  KEY `key_object` (objectPHID)
+
+) ENGINE=InnoDB, COLLATE utf8_general_ci;
+
+CREATE TABLE {$NAMESPACE}_harbormaster.harbormaster_buildtransaction (
+  id INT UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT,
+  phid VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  authorPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  objectPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  viewPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  editPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  commentPHID VARCHAR(64) COLLATE utf8_bin,
+  commentVersion INT UNSIGNED NOT NULL,
+  transactionType VARCHAR(32) NOT NULL COLLATE utf8_bin,
+  oldValue LONGTEXT NOT NULL COLLATE utf8_bin,
+  newValue LONGTEXT NOT NULL COLLATE utf8_bin,
+  contentSource LONGTEXT NOT NULL COLLATE utf8_bin,
+  metadata LONGTEXT NOT NULL COLLATE utf8_bin,
+  dateCreated INT UNSIGNED NOT NULL,
+  dateModified INT UNSIGNED NOT NULL,
+
+  UNIQUE KEY `key_phid` (phid),
+  KEY `key_object` (objectPHID)
+
+) ENGINE=InnoDB, COLLATE utf8_general_ci;
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@ -738,6 +738,9 @@ phutil_register_library_map(array(
    'HarbormasterBuildStepTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildStepTransactionQuery.php',
    'HarbormasterBuildTarget' => 'applications/harbormaster/storage/build/HarbormasterBuildTarget.php',
    'HarbormasterBuildTargetQuery' => 'applications/harbormaster/query/HarbormasterBuildTargetQuery.php',
+    'HarbormasterBuildTransaction' => 'applications/harbormaster/storage/HarbormasterBuildTransaction.php',
+    'HarbormasterBuildTransactionEditor' => 'applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php',
+    'HarbormasterBuildTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildTransactionQuery.php',
    'HarbormasterBuildViewController' => 'applications/harbormaster/controller/HarbormasterBuildViewController.php',
    'HarbormasterBuildWorker' => 'applications/harbormaster/worker/HarbormasterBuildWorker.php',
    'HarbormasterBuildable' => 'applications/harbormaster/storage/HarbormasterBuildable.php',
@ -746,6 +749,9 @@ phutil_register_library_map(array(
    'HarbormasterBuildableListController' => 'applications/harbormaster/controller/HarbormasterBuildableListController.php',
    'HarbormasterBuildableQuery' => 'applications/harbormaster/query/HarbormasterBuildableQuery.php',
    'HarbormasterBuildableSearchEngine' => 'applications/harbormaster/query/HarbormasterBuildableSearchEngine.php',
+    'HarbormasterBuildableTransaction' => 'applications/harbormaster/storage/HarbormasterBuildableTransaction.php',
+    'HarbormasterBuildableTransactionEditor' => 'applications/harbormaster/editor/HarbormasterBuildableTransactionEditor.php',
+    'HarbormasterBuildableTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildableTransactionQuery.php',
    'HarbormasterBuildableViewController' => 'applications/harbormaster/controller/HarbormasterBuildableViewController.php',
    'HarbormasterCapabilityManagePlans' => 'applications/harbormaster/capability/HarbormasterCapabilityManagePlans.php',
    'HarbormasterCommandBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php',
@ -3427,6 +3433,9 @@ phutil_register_library_map(array(
      1 => 'PhabricatorPolicyInterface',
    ),
    'HarbormasterBuildTargetQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+    'HarbormasterBuildTransaction' => 'PhabricatorApplicationTransaction',
+    'HarbormasterBuildTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
+    'HarbormasterBuildTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
    'HarbormasterBuildViewController' => 'HarbormasterController',
    'HarbormasterBuildWorker' => 'HarbormasterWorker',
    'HarbormasterBuildable' =>
@ -3439,6 +3448,9 @@ phutil_register_library_map(array(
    'HarbormasterBuildableListController' => 'HarbormasterController',
    'HarbormasterBuildableQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
    'HarbormasterBuildableSearchEngine' => 'PhabricatorApplicationSearchEngine',
+    'HarbormasterBuildableTransaction' => 'PhabricatorApplicationTransaction',
+    'HarbormasterBuildableTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
+    'HarbormasterBuildableTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
    'HarbormasterBuildableViewController' => 'HarbormasterController',
    'HarbormasterCapabilityManagePlans' => 'PhabricatorPolicyCapability',
    'HarbormasterCommandBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
--- a/src/applications/harbormaster/controller/HarbormasterBuildActionController.php
+++ b/src/applications/harbormaster/controller/HarbormasterBuildActionController.php
@ -55,22 +55,17 @@ final class HarbormasterBuildActionController
    }

    if ($request->isDialogFormPost() && $can_issue) {
+      $editor = id(new HarbormasterBuildTransactionEditor())
+        ->setActor($viewer)
+        ->setContentSourceFromRequest($request)
+        ->setContinueOnNoEffect(true)
+        ->setContinueOnMissingFields(true);

-      // Issue the new build command.
-      id(new HarbormasterBuildCommand())
-        ->setAuthorPHID($viewer->getPHID())
-        ->setTargetPHID($build->getPHID())
-        ->setCommand($command)
-        ->save();
+      $xaction = id(new HarbormasterBuildTransaction())
+        ->setTransactionType(HarbormasterBuildTransaction::TYPE_COMMAND)
+        ->setNewValue($command);

-      // Schedule a build update. We may already have stuff in queue (in which
-      // case this will just no-op), but we might also be dealing with a
-      // stopped build, which won't restart unless we deal with this.
-      PhabricatorWorker::scheduleTask(
-        'HarbormasterBuildWorker',
-        array(
-          'buildID' => $build->getID()
-        ));
+      $editor->applyTransactions($build, array($xaction));

      return id(new AphrontRedirectResponse())->setURI($return_uri);
    }
--- a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php
+++ b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php
@ -103,11 +103,21 @@ final class HarbormasterBuildViewController
      $targets[] = $this->buildLog($build, $build_target);
    }

+    $xactions = id(new HarbormasterBuildTransactionQuery())
+      ->setViewer($viewer)
+      ->withObjectPHIDs(array($build->getPHID()))
+      ->execute();
+    $timeline = id(new PhabricatorApplicationTransactionView())
+      ->setUser($viewer)
+      ->setObjectPHID($build->getPHID())
+      ->setTransactions($xactions);
+
    return $this->buildApplicationPage(
      array(
        $crumbs,
        $box,
-        $targets
+        $targets,
+        $timeline,
      ),
      array(
        'title' => $title,
--- a/src/applications/harbormaster/controller/HarbormasterBuildableActionController.php
+++ b/src/applications/harbormaster/controller/HarbormasterBuildableActionController.php
@ -56,18 +56,29 @@ final class HarbormasterBuildableActionController

    $return_uri = $buildable->getMonogram();
    if ($request->isDialogFormPost() && $issuable) {
-      foreach ($issuable as $build) {
-        id(new HarbormasterBuildCommand())
-          ->setAuthorPHID($viewer->getPHID())
-          ->setTargetPHID($build->getPHID())
-          ->setCommand($command)
-          ->save();
+      $editor = id(new HarbormasterBuildableTransactionEditor())
+        ->setActor($viewer)
+        ->setContentSourceFromRequest($request)
+        ->setContinueOnNoEffect(true)
+        ->setContinueOnMissingFields(true);

-        PhabricatorWorker::scheduleTask(
-          'HarbormasterBuildWorker',
-          array(
-            'buildID' => $build->getID()
-          ));
+      $xaction = id(new HarbormasterBuildableTransaction())
+        ->setTransactionType(HarbormasterBuildableTransaction::TYPE_COMMAND)
+        ->setNewValue($command);
+
+      $editor->applyTransactions($buildable, array($xaction));
+
+      $build_editor = id(new HarbormasterBuildTransactionEditor())
+        ->setActor($viewer)
+        ->setContentSourceFromRequest($request)
+        ->setContinueOnNoEffect(true)
+        ->setContinueOnMissingFields(true);
+
+      foreach ($issuable as $build) {
+        $xaction = id(new HarbormasterBuildTransaction())
+          ->setTransactionType(HarbormasterBuildTransaction::TYPE_COMMAND)
+          ->setNewValue($command);
+        $build_editor->applyTransactions($build, array($xaction));
      }

      return id(new AphrontRedirectResponse())->setURI($return_uri);
--- a/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php
+++ b/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php
@ -46,6 +46,15 @@ final class HarbormasterBuildableViewController
    $box = id(new PHUIObjectBoxView())
      ->setHeader($header);

+    $xactions = id(new HarbormasterBuildableTransactionQuery())
+      ->setViewer($viewer)
+      ->withObjectPHIDs(array($buildable->getPHID()))
+      ->execute();
+    $timeline = id(new PhabricatorApplicationTransactionView())
+      ->setUser($viewer)
+      ->setObjectPHID($buildable->getPHID())
+      ->setTransactions($xactions);
+
    $actions = $this->buildActionList($buildable);
    $this->buildPropertyLists($box, $buildable, $actions);

@ -57,6 +66,7 @@ final class HarbormasterBuildableViewController
        $crumbs,
        $box,
        $build_list,
+        $timeline,
      ),
      array(
        'title' => $title,
--- a/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php
+++ b/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php
@ -0,0 +1,106 @@
+<?php
+
+final class HarbormasterBuildTransactionEditor
+  extends PhabricatorApplicationTransactionEditor {
+
+  public function getTransactionTypes() {
+    $types = parent::getTransactionTypes();
+
+    $types[] = HarbormasterBuildTransaction::TYPE_CREATE;
+    $types[] = HarbormasterBuildTransaction::TYPE_COMMAND;
+
+    return $types;
+  }
+
+  protected function getCustomTransactionOldValue(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    switch ($xaction->getTransactionType()) {
+      case HarbormasterBuildTransaction::TYPE_CREATE:
+      case HarbormasterBuildTransaction::TYPE_COMMAND:
+        return null;
+    }
+
+    return parent::getCustomTransactionOldValue($object, $xaction);
+  }
+
+  protected function getCustomTransactionNewValue(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    switch ($xaction->getTransactionType()) {
+      case HarbormasterBuildTransaction::TYPE_CREATE:
+        return true;
+      case HarbormasterBuildTransaction::TYPE_COMMAND:
+        return $xaction->getNewValue();
+    }
+
+    return parent::getCustomTransactionNewValue($object, $xaction);
+  }
+
+  protected function applyCustomInternalTransaction(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    switch ($xaction->getTransactionType()) {
+      case HarbormasterBuildTransaction::TYPE_CREATE:
+        return;
+      case HarbormasterBuildTransaction::TYPE_COMMAND:
+        return $this->executeBuildCommand($object, $xaction);
+    }
+
+    return parent::applyCustomInternalTransaction($object, $xaction);
+  }
+
+  private function executeBuildCommand(
+    HarbormasterBuild $build,
+    HarbormasterBuildTransaction $xaction) {
+
+    $command = $xaction->getNewValue();
+
+    switch ($command) {
+      case HarbormasterBuildCommand::COMMAND_RESTART:
+        $issuable = $build->canRestartBuild();
+        break;
+      case HarbormasterBuildCommand::COMMAND_STOP:
+        $issuable = $build->canStopBuild();
+        break;
+      case HarbormasterBuildCommand::COMMAND_RESUME:
+        $issuable = $build->canResumeBuild();
+        break;
+      default:
+        throw new Exception("Unknown command $command");
+    }
+
+    if (!$issuable) {
+      return;
+    }
+
+    id(new HarbormasterBuildCommand())
+      ->setAuthorPHID($xaction->getAuthorPHID())
+      ->setTargetPHID($build->getPHID())
+      ->setCommand($command)
+      ->save();
+
+    PhabricatorWorker::scheduleTask(
+      'HarbormasterBuildWorker',
+      array(
+        'buildID' => $build->getID()
+      ));
+  }
+
+  protected function applyCustomExternalTransaction(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    switch ($xaction->getTransactionType()) {
+      case HarbormasterBuildTransaction::TYPE_CREATE:
+      case HarbormasterBuildTransaction::TYPE_COMMAND:
+        return;
+    }
+
+    return parent::applyCustomExternalTransaction($object, $xaction);
+  }
+
+}
--- a/src/applications/harbormaster/editor/HarbormasterBuildableTransactionEditor.php
+++ b/src/applications/harbormaster/editor/HarbormasterBuildableTransactionEditor.php
@ -0,0 +1,68 @@
+<?php
+
+final class HarbormasterBuildableTransactionEditor
+  extends PhabricatorApplicationTransactionEditor {
+
+  public function getTransactionTypes() {
+    $types = parent::getTransactionTypes();
+
+    $types[] = HarbormasterBuildableTransaction::TYPE_CREATE;
+    $types[] = HarbormasterBuildableTransaction::TYPE_COMMAND;
+
+    return $types;
+  }
+
+  protected function getCustomTransactionOldValue(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    switch ($xaction->getTransactionType()) {
+      case HarbormasterBuildableTransaction::TYPE_CREATE:
+      case HarbormasterBuildableTransaction::TYPE_COMMAND:
+        return null;
+    }
+
+    return parent::getCustomTransactionOldValue($object, $xaction);
+  }
+
+  protected function getCustomTransactionNewValue(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    switch ($xaction->getTransactionType()) {
+      case HarbormasterBuildableTransaction::TYPE_CREATE:
+        return true;
+      case HarbormasterBuildableTransaction::TYPE_COMMAND:
+        return $xaction->getNewValue();
+    }
+
+    return parent::getCustomTransactionNewValue($object, $xaction);
+  }
+
+  protected function applyCustomInternalTransaction(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    switch ($xaction->getTransactionType()) {
+      case HarbormasterBuildableTransaction::TYPE_CREATE:
+      case HarbormasterBuildableTransaction::TYPE_COMMAND:
+        return;
+    }
+
+    return parent::applyCustomInternalTransaction($object, $xaction);
+  }
+
+  protected function applyCustomExternalTransaction(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    switch ($xaction->getTransactionType()) {
+      case HarbormasterBuildableTransaction::TYPE_CREATE:
+      case HarbormasterBuildableTransaction::TYPE_COMMAND:
+        return;
+    }
+
+    return parent::applyCustomExternalTransaction($object, $xaction);
+  }
+
+}
--- a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php
+++ b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php
@ -408,8 +408,10 @@ final class HarbormasterBuildEngine extends Phobject {
    // can look at the results themselves, and other users generally don't
    // care about the outcome.

-    if ($did_update && !$buildable->getIsManualBuildable()) {
-
+    $should_publish = $did_update &&
+                      $new_status != HarbormasterBuildable::STATUS_BUILDING &&
+                      !$buildable->getIsManualBuildable();
+    if ($should_publish) {
      $object = id(new PhabricatorObjectQuery())
        ->setViewer($viewer)
        ->withPHIDs(array($buildable->getBuildablePHID()))
--- a/src/applications/harbormaster/query/HarbormasterBuildTransactionQuery.php
+++ b/src/applications/harbormaster/query/HarbormasterBuildTransactionQuery.php
@ -0,0 +1,13 @@
+<?php
+
+/**
+ * @group harbormaster
+ */
+final class HarbormasterBuildTransactionQuery
+  extends PhabricatorApplicationTransactionQuery {
+
+  public function getTemplateApplicationTransaction() {
+    return new HarbormasterBuildTransaction();
+  }
+
+}
--- a/src/applications/harbormaster/query/HarbormasterBuildableTransactionQuery.php
+++ b/src/applications/harbormaster/query/HarbormasterBuildableTransactionQuery.php
@ -0,0 +1,13 @@
+<?php
+
+/**
+ * @group harbormaster
+ */
+final class HarbormasterBuildableTransactionQuery
+  extends PhabricatorApplicationTransactionQuery {
+
+  public function getTemplateApplicationTransaction() {
+    return new HarbormasterBuildableTransaction();
+  }
+
+}
--- a/src/applications/harbormaster/storage/HarbormasterBuildTransaction.php
+++ b/src/applications/harbormaster/storage/HarbormasterBuildTransaction.php
@ -0,0 +1,87 @@
+<?php
+
+final class HarbormasterBuildTransaction
+  extends PhabricatorApplicationTransaction {
+
+  const TYPE_CREATE = 'harbormaster:build:create';
+  const TYPE_COMMAND = 'harbormaster:build:command';
+
+  public function getApplicationName() {
+    return 'harbormaster';
+  }
+
+  public function getApplicationTransactionType() {
+    return HarbormasterPHIDTypeBuild::TYPECONST;
+  }
+
+  public function getTitle() {
+    $author_phid = $this->getAuthorPHID();
+
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    switch ($this->getTransactionType()) {
+      case self::TYPE_CREATE:
+        return pht(
+          '%s created this build.',
+          $this->renderHandleLink($author_phid));
+      case self::TYPE_COMMAND:
+        switch ($new) {
+          case HarbormasterBuildCommand::COMMAND_RESTART:
+            return pht(
+              '%s restarted this build.',
+              $this->renderHandleLink($author_phid));
+          case HarbormasterBuildCommand::COMMAND_RESUME:
+            return pht(
+              '%s resumed this build.',
+              $this->renderHandleLink($author_phid));
+          case HarbormasterBuildCommand::COMMAND_STOP:
+            return pht(
+              '%s stopped this build.',
+              $this->renderHandleLink($author_phid));
+        }
+    }
+    return parent::getTitle();
+  }
+
+  public function getIcon() {
+    $author_phid = $this->getAuthorPHID();
+
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    switch ($this->getTransactionType()) {
+      case self::TYPE_CREATE:
+        return 'fa-plus';
+      case self::TYPE_COMMAND:
+        switch ($new) {
+          case HarbormasterBuildCommand::COMMAND_RESTART:
+            return 'fa-backward';
+          case HarbormasterBuildCommand::COMMAND_RESUME:
+            return 'fa-play';
+          case HarbormasterBuildCommand::COMMAND_STOP:
+            return 'fa-stop';
+        }
+    }
+
+    return parent::getIcon();
+  }
+
+  public function getColor() {
+    $author_phid = $this->getAuthorPHID();
+
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    switch ($this->getTransactionType()) {
+      case self::TYPE_CREATE:
+        return 'green';
+      case self::TYPE_COMMAND:
+        switch ($new) {
+          case HarbormasterBuildCommand::COMMAND_STOP:
+            return 'red';
+        }
+    }
+    return parent::getColor();
+  }
+}
--- a/src/applications/harbormaster/storage/HarbormasterBuildableTransaction.php
+++ b/src/applications/harbormaster/storage/HarbormasterBuildableTransaction.php
@ -0,0 +1,87 @@
+<?php
+
+final class HarbormasterBuildableTransaction
+  extends PhabricatorApplicationTransaction {
+
+  const TYPE_CREATE = 'harbormaster:buildable:create';
+  const TYPE_COMMAND = 'harbormaster:buildable:command';
+
+  public function getApplicationName() {
+    return 'harbormaster';
+  }
+
+  public function getApplicationTransactionType() {
+    return HarbormasterPHIDTypeBuildable::TYPECONST;
+  }
+
+  public function getTitle() {
+    $author_phid = $this->getAuthorPHID();
+
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    switch ($this->getTransactionType()) {
+      case self::TYPE_CREATE:
+        return pht(
+          '%s created this buildable.',
+          $this->renderHandleLink($author_phid));
+      case self::TYPE_COMMAND:
+        switch ($new) {
+          case HarbormasterBuildCommand::COMMAND_RESTART:
+            return pht(
+              '%s restarted this buildable.',
+              $this->renderHandleLink($author_phid));
+          case HarbormasterBuildCommand::COMMAND_RESUME:
+            return pht(
+              '%s resumed this buildable.',
+              $this->renderHandleLink($author_phid));
+          case HarbormasterBuildCommand::COMMAND_STOP:
+            return pht(
+              '%s stopped this buildable.',
+              $this->renderHandleLink($author_phid));
+        }
+    }
+    return parent::getTitle();
+  }
+
+  public function getIcon() {
+    $author_phid = $this->getAuthorPHID();
+
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    switch ($this->getTransactionType()) {
+      case self::TYPE_CREATE:
+        return 'fa-plus';
+      case self::TYPE_COMMAND:
+        switch ($new) {
+          case HarbormasterBuildCommand::COMMAND_RESTART:
+            return 'fa-backward';
+          case HarbormasterBuildCommand::COMMAND_RESUME:
+            return 'fa-play';
+          case HarbormasterBuildCommand::COMMAND_STOP:
+            return 'fa-stop';
+        }
+    }
+
+    return parent::getIcon();
+  }
+
+  public function getColor() {
+    $author_phid = $this->getAuthorPHID();
+
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    switch ($this->getTransactionType()) {
+      case self::TYPE_CREATE:
+        return 'green';
+      case self::TYPE_COMMAND:
+        switch ($new) {
+          case HarbormasterBuildCommand::COMMAND_STOP:
+            return 'red';
+        }
+    }
+    return parent::getColor();
+  }
+}
--- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
+++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
@ -448,12 +448,13 @@ abstract class PhabricatorApplicationTransaction
        return true;
      case PhabricatorTransactions::TYPE_BUILDABLE:
        switch ($this->getNewValue()) {
-          case HarbormasterBuildable::STATUS_PASSED:
-            // For now, just never send mail when builds pass. We might let
+          case HarbormasterBuildable::STATUS_FAILED:
+            // For now, only ever send mail when builds fail. We might let
            // you customize this later, but in most cases this is probably
            // completely uninteresting.
-            return true;
+            return false;
        }
+        return true;
    }

    return $this->shouldHide();
@ -465,13 +466,14 @@ abstract class PhabricatorApplicationTransaction
        return true;
      case PhabricatorTransactions::TYPE_BUILDABLE:
        switch ($this->getNewValue()) {
-          case HarbormasterBuildable::STATUS_PASSED:
+          case HarbormasterBuildable::STATUS_FAILED:
            // For now, don't notify on build passes either. These are pretty
            // high volume and annoying, with very little present value. We
            // might want to turn them back on in the specific case of
            // build successes on the current document?
-            return true;
+            return false;
        }
+        return true;
    }

    return $this->shouldHide();
@ -645,6 +647,12 @@ abstract class PhabricatorApplicationTransaction

      case PhabricatorTransactions::TYPE_BUILDABLE:
        switch ($this->getNewValue()) {
+          case HarbormasterBuildable::STATUS_BUILDING:
+            return pht(
+              '%s started building %s.',
+              $this->renderHandleLink($author_phid),
+              $this->renderHandleLink(
+                $this->getMetadataValue('harbormaster:buildablePHID')));
          case HarbormasterBuildable::STATUS_PASSED:
            return pht(
              '%s completed building %s.',
@ -722,6 +730,13 @@ abstract class PhabricatorApplicationTransaction
        }
      case PhabricatorTransactions::TYPE_BUILDABLE:
        switch ($this->getNewValue()) {
+          case HarbormasterBuildable::STATUS_BUILDING:
+            return pht(
+              '%s started building %s for %s.',
+              $this->renderHandleLink($author_phid),
+              $this->renderHandleLink(
+                $this->getMetadataValue('harbormaster:buildablePHID')),
+              $this->renderHandleLink($object_phid));
          case HarbormasterBuildable::STATUS_PASSED:
            return pht(
              '%s completed building %s for %s.',