Use ApplicationTransactions for all non-path edits to Owners packages

Summary: Ref T8320.

Test Plan: {F437431}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8320

Differential Revision: https://secure.phabricator.com/D13028

resources/sql/autopatches/20150526.owners.mailkey.1.sql

···
+ALTER TABLE {$NAMESPACE}_owners.owners_package
+  ADD mailKey binary(20) NOT NULL;

resources/sql/autopatches/20150526.owners.mailkey.2.php

···
+<?php
+
+$table = new PhabricatorOwnersPackage();
+$conn_w = $table->establishConnection('w');
+$iterator = new LiskMigrationIterator($table);
+foreach ($iterator as $package) {
+  $id = $package->getID();
+
+  echo pht('Adding mail key for package %d...', $id);
+  echo "\n";
+
+  queryfx(
+    $conn_w,
+    'UPDATE %T SET mailKey = %s WHERE id = %d',
+    $table->getTableName(),
+    Filesystem::readRandomCharacters(20),
+    $id);
+}

resources/sql/autopatches/20150526.owners.xaction.sql

···
+CREATE TABLE {$NAMESPACE}_owners.owners_packagetransaction (
+  id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
+  phid VARBINARY(64) NOT NULL,
+  authorPHID VARBINARY(64) NOT NULL,
+  objectPHID VARBINARY(64) NOT NULL,
+  viewPolicy VARBINARY(64) NOT NULL,
+  editPolicy VARBINARY(64) NOT NULL,
+  commentPHID VARBINARY(64) DEFAULT NULL,
+  commentVersion INT UNSIGNED NOT NULL,
+  transactionType VARCHAR(32) COLLATE {$COLLATE_TEXT} NOT NULL,
+  oldValue LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL,
+  newValue LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL,
+  contentSource LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL,
+  metadata LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL,
+  dateCreated INT UNSIGNED NOT NULL,
+  dateModified INT UNSIGNED NOT NULL,
+  UNIQUE KEY `key_phid` (`phid`),
+  KEY `key_object` (`objectPHID`)
+) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT};

src/__phutil_library_map__.php

···
     'PhabricatorOwnersPackageQuery' => 'applications/owners/query/PhabricatorOwnersPackageQuery.php',
     'PhabricatorOwnersPackageSearchEngine' => 'applications/owners/query/PhabricatorOwnersPackageSearchEngine.php',
     'PhabricatorOwnersPackageTestCase' => 'applications/owners/storage/__tests__/PhabricatorOwnersPackageTestCase.php',
+    'PhabricatorOwnersPackageTransaction' => 'applications/owners/storage/PhabricatorOwnersPackageTransaction.php',
+    'PhabricatorOwnersPackageTransactionEditor' => 'applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php',
+    'PhabricatorOwnersPackageTransactionQuery' => 'applications/owners/query/PhabricatorOwnersPackageTransactionQuery.php',
     'PhabricatorOwnersPath' => 'applications/owners/storage/PhabricatorOwnersPath.php',
     'PhabricatorOwnersPathsController' => 'applications/owners/controller/PhabricatorOwnersPathsController.php',
     'PhabricatorPHDConfigOptions' => 'applications/config/option/PhabricatorPHDConfigOptions.php',
···
     'PhabricatorOwnersPackage' => array(
       'PhabricatorOwnersDAO',
       'PhabricatorPolicyInterface',
+      'PhabricatorApplicationTransactionInterface',
     ),
     'PhabricatorOwnersPackageDatasource' => 'PhabricatorTypeaheadDatasource',
     'PhabricatorOwnersPackageEditor' => 'PhabricatorEditor',
···
     'PhabricatorOwnersPackageQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorOwnersPackageSearchEngine' => 'PhabricatorApplicationSearchEngine',
     'PhabricatorOwnersPackageTestCase' => 'PhabricatorTestCase',
+    'PhabricatorOwnersPackageTransaction' => 'PhabricatorApplicationTransaction',
+    'PhabricatorOwnersPackageTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
+    'PhabricatorOwnersPackageTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
     'PhabricatorOwnersPath' => 'PhabricatorOwnersDAO',
     'PhabricatorOwnersPathsController' => 'PhabricatorOwnersController',
     'PhabricatorPHDConfigOptions' => 'PhabricatorApplicationConfigOptions',

src/applications/owners/controller/PhabricatorOwnersDetailController.php

···
     $crumbs = $this->buildApplicationCrumbs();
     $crumbs->addTextCrumb($package->getName());
 
+    $timeline = $this->buildTransactionTimeline(
+      $package,
+      new PhabricatorOwnersPackageTransactionQuery());
+    $timeline->setShouldTerminate(true);
+
     return $this->buildApplicationPage(
       array(
         $crumbs,
         $panel,
         $this->renderPathsTable($paths, $repositories),
         $commit_panels,
+        $timeline,
       ),
       array(
         'title' => $package->getName(),

src/applications/owners/controller/PhabricatorOwnersEditController.php

···
       if (!$package) {
         return new Aphront404Response();
       }
-
       $is_new = false;
     } else {
-      $package = new PhabricatorOwnersPackage();
-      $package->setPrimaryOwnerPHID($viewer->getPHID());
-
+      $package = PhabricatorOwnersPackage::initializeNewPackage($viewer);
       $is_new = true;
     }
 
     $e_name = true;
     $e_primary = true;
 
+    $v_name = $package->getName();
+    $v_primary = $package->getPrimaryOwnerPHID();
+    // TODO: Pull these off needOwners() on the Query.
+    $v_owners = mpull($package->loadOwners(), 'getUserPHID');
+    $v_auditing = $package->getAuditingEnabled();
+    $v_description = $package->getDescription();
+
+
     $errors = array();
-
     if ($request->isFormPost()) {
-      $package->setName($request->getStr('name'));
-      $package->setDescription($request->getStr('description'));
-      $old_auditing_enabled = $package->getAuditingEnabled();
-      $package->setAuditingEnabled(
-        ($request->getStr('auditing') === 'enabled')
-          ? 1
-          : 0);
+      $xactions = array();
 
-      $primary = $request->getArr('primary');
-      $primary = reset($primary);
-      $old_primary = $package->getPrimaryOwnerPHID();
-      $package->setPrimaryOwnerPHID($primary);
+      $v_name = $request->getStr('name');
+      $v_primary = head($request->getArr('primary'));
+      $v_owners = $request->getArr('owners');
+      $v_auditing = ($request->getStr('auditing') == 'enabled');
+      $v_description = $request->getStr('description');
 
-      $owners = $request->getArr('owners');
-      if ($primary) {
-        array_unshift($owners, $primary);
+      if ($v_primary) {
+        $v_owners[] = $v_primary;
+        $v_owners = array_unique($v_owners);
       }
-      $owners = array_unique($owners);
 
-      if (!strlen($package->getName())) {
-        $e_name = pht('Required');
-        $errors[] = pht('Package name is required.');
-      } else {
-        $e_name = null;
-      }
+      $type_name = PhabricatorOwnersPackageTransaction::TYPE_NAME;
+      $type_primary = PhabricatorOwnersPackageTransaction::TYPE_PRIMARY;
+      $type_owners = PhabricatorOwnersPackageTransaction::TYPE_OWNERS;
+      $type_auditing = PhabricatorOwnersPackageTransaction::TYPE_AUDITING;
+      $type_description = PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION;
 
-      if (!$package->getPrimaryOwnerPHID()) {
-        $e_primary = pht('Required');
-        $errors[] = pht('Package must have a primary owner.');
-      } else {
-        $e_primary = null;
-      }
+      $xactions[] = id(new PhabricatorOwnersPackageTransaction())
+        ->setTransactionType($type_name)
+        ->setNewValue($v_name);
 
-      if (!$errors) {
-        $package->attachUnsavedOwners($owners);
-        $package->attachUnsavedPaths(array());
-        $package->attachOldAuditingEnabled($old_auditing_enabled);
-        $package->attachOldPrimaryOwnerPHID($old_primary);
-        try {
-          id(new PhabricatorOwnersPackageEditor())
-            ->setActor($viewer)
-            ->setPackage($package)
-            ->save();
+      $xactions[] = id(new PhabricatorOwnersPackageTransaction())
+        ->setTransactionType($type_primary)
+        ->setNewValue($v_primary);
 
-          $id = $package->getID();
-          if ($is_new) {
-            $next_uri = '/owners/paths/'.$id.'/';
-          } else {
-            $next_uri = '/owners/package/'.$id.'/';
-          }
+      $xactions[] = id(new PhabricatorOwnersPackageTransaction())
+        ->setTransactionType($type_owners)
+        ->setNewValue($v_owners);
+
+      $xactions[] = id(new PhabricatorOwnersPackageTransaction())
+        ->setTransactionType($type_auditing)
+        ->setNewValue($v_auditing);
+
+      $xactions[] = id(new PhabricatorOwnersPackageTransaction())
+        ->setTransactionType($type_description)
+        ->setNewValue($v_description);
+
+      $editor = id(new PhabricatorOwnersPackageTransactionEditor())
+        ->setActor($viewer)
+        ->setContentSourceFromRequest($request)
+        ->setContinueOnNoEffect(true);
+
+      try {
+        $editor->applyTransactions($package, $xactions);
 
-          return id(new AphrontRedirectResponse())->setURI($next_uri);
-        } catch (AphrontDuplicateKeyQueryException $ex) {
-          $e_name = pht('Duplicate');
-          $errors[] = pht('Package name must be unique.');
+        $id = $package->getID();
+        if ($is_new) {
+          $next_uri = '/owners/paths/'.$id.'/';
+        } else {
+          $next_uri = '/owners/package/'.$id.'/';
         }
+
+        return id(new AphrontRedirectResponse())->setURI($next_uri);
+      } catch (AphrontDuplicateKeyQueryException $ex) {
+        $e_name = pht('Duplicate');
+        $errors[] = pht('Package name must be unique.');
+      } catch (PhabricatorApplicationTransactionValidationException $ex) {
+        $validation_exception = $ex;
+
+        $e_name = $ex->getShortMessage($type_name);
+        $e_primary = $ex->getShortMessage($type_primary);
       }
-    } else {
-      $owners = $package->loadOwners();
-      $owners = mpull($owners, 'getUserPHID');
     }
 
-    $primary = $package->getPrimaryOwnerPHID();
-    if ($primary) {
-      $value_primary_owner = array($primary);
+    if ($v_primary) {
+      $value_primary_owner = array($v_primary);
     } else {
       $value_primary_owner = array();
     }
···
         id(new AphrontFormTextControl())
           ->setLabel(pht('Name'))
           ->setName('name')
-          ->setValue($package->getName())
+          ->setValue($v_name)
           ->setError($e_name))
       ->appendControl(
         id(new AphrontFormTokenizerControl())
···
           ->setDatasource(new PhabricatorProjectOrUserDatasource())
           ->setLabel(pht('Owners'))
           ->setName('owners')
-          ->setValue($owners))
+          ->setValue($v_owners))
       ->appendChild(
         id(new AphrontFormSelectControl())
           ->setName('auditing')
···
               'this package will be reviewed to make sure an owner '.
               'of the package is involved and the commit message has '.
               'a valid revision, reviewed by, and author.'))
-          ->setOptions(array(
-            'disabled'  => pht('Disabled'),
-            'enabled'   => pht('Enabled'),
-          ))
-          ->setValue(
-            $package->getAuditingEnabled()
-              ? 'enabled'
-              : 'disabled'))
+          ->setOptions(
+            array(
+              'disabled'  => pht('Disabled'),
+              'enabled'   => pht('Enabled'),
+            ))
+          ->setValue(($v_auditing ? 'enabled' : 'disabled')))
       ->appendChild(
-        id(new AphrontFormTextAreaControl())
+        id(new PhabricatorRemarkupControl())
+          ->setUser($viewer)
           ->setLabel(pht('Description'))
           ->setName('description')
-          ->setValue($package->getDescription()))
+          ->setValue($v_description))
       ->appendChild(
         id(new AphrontFormSubmitControl())
           ->addCancelButton($cancel_uri)

src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php

···
+<?php
+
+final class PhabricatorOwnersPackageTransactionEditor
+  extends PhabricatorApplicationTransactionEditor {
+
+  public function getEditorApplicationClass() {
+    return 'PhabricatorOwnersApplication';
+  }
+
+  public function getEditorObjectsDescription() {
+    return pht('Owners Packages');
+  }
+
+  public function getTransactionTypes() {
+    $types = parent::getTransactionTypes();
+
+    $types[] = PhabricatorOwnersPackageTransaction::TYPE_NAME;
+    $types[] = PhabricatorOwnersPackageTransaction::TYPE_PRIMARY;
+    $types[] = PhabricatorOwnersPackageTransaction::TYPE_OWNERS;
+    $types[] = PhabricatorOwnersPackageTransaction::TYPE_AUDITING;
+    $types[] = PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION;
+
+    return $types;
+  }
+
+  protected function getCustomTransactionOldValue(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    switch ($xaction->getTransactionType()) {
+      case PhabricatorOwnersPackageTransaction::TYPE_NAME:
+        return $object->getName();
+      case PhabricatorOwnersPackageTransaction::TYPE_PRIMARY:
+        return $object->getPrimaryOwnerPHID();
+      case PhabricatorOwnersPackageTransaction::TYPE_OWNERS:
+        // TODO: needOwners() this on the Query.
+        $phids = mpull($object->loadOwners(), 'getUserPHID');
+        $phids = array_values($phids);
+        return $phids;
+      case PhabricatorOwnersPackageTransaction::TYPE_AUDITING:
+        return (int)$object->getAuditingEnabled();
+      case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION:
+        return $object->getDescription();
+    }
+  }
+
+  protected function getCustomTransactionNewValue(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    switch ($xaction->getTransactionType()) {
+      case PhabricatorOwnersPackageTransaction::TYPE_NAME:
+      case PhabricatorOwnersPackageTransaction::TYPE_PRIMARY:
+      case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION:
+        return $xaction->getNewValue();
+      case PhabricatorOwnersPackageTransaction::TYPE_AUDITING:
+        return (int)$xaction->getNewValue();
+      case PhabricatorOwnersPackageTransaction::TYPE_OWNERS:
+        $phids = $xaction->getNewValue();
+        $phids = array_unique($phids);
+        $phids = array_values($phids);
+        return $phids;
+    }
+  }
+
+  protected function applyCustomInternalTransaction(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    switch ($xaction->getTransactionType()) {
+      case PhabricatorOwnersPackageTransaction::TYPE_NAME:
+        $object->setName($xaction->getNewValue());
+        return;
+      case PhabricatorOwnersPackageTransaction::TYPE_PRIMARY:
+        $object->setPrimaryOwnerPHID($xaction->getNewValue());
+        return;
+      case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION:
+        $object->setDescription($xaction->getNewValue());
+        return;
+      case PhabricatorOwnersPackageTransaction::TYPE_AUDITING:
+        $object->setAuditingEnabled($xaction->getNewValue());
+        return;
+      case PhabricatorOwnersPackageTransaction::TYPE_OWNERS:
+        return;
+    }
+
+    return parent::applyCustomInternalTransaction($object, $xaction);
+  }
+
+  protected function applyCustomExternalTransaction(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    switch ($xaction->getTransactionType()) {
+      case PhabricatorOwnersPackageTransaction::TYPE_NAME:
+      case PhabricatorOwnersPackageTransaction::TYPE_PRIMARY:
+      case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION:
+      case PhabricatorOwnersPackageTransaction::TYPE_AUDITING:
+        return;
+      case PhabricatorOwnersPackageTransaction::TYPE_OWNERS:
+        $old = $xaction->getOldValue();
+        $new = $xaction->getNewValue();
+
+        // TODO: needOwners this
+        $owners = $object->loadOwners();
+        $owners = mpull($owners, null, 'getUserPHID');
+
+        $rem = array_diff($old, $new);
+        foreach ($rem as $phid) {
+          if (isset($owners[$phid])) {
+            $owners[$phid]->delete();
+            unset($owners[$phid]);
+          }
+        }
+
+        $add = array_diff($new, $old);
+        foreach ($add as $phid) {
+          $owners[$phid] = id(new PhabricatorOwnersOwner())
+            ->setPackageID($object->getID())
+            ->setUserPHID($phid)
+            ->save();
+        }
+
+        // TODO: Attach owners here
+        return;
+    }
+
+    return parent::applyCustomExternalTransaction($object, $xaction);
+  }
+
+  protected function validateTransaction(
+    PhabricatorLiskDAO $object,
+    $type,
+    array $xactions) {
+
+    $errors = parent::validateTransaction($object, $type, $xactions);
+
+    switch ($type) {
+      case PhabricatorOwnersPackageTransaction::TYPE_NAME:
+        $missing = $this->validateIsEmptyTextField(
+          $object->getName(),
+          $xactions);
+
+        if ($missing) {
+          $error = new PhabricatorApplicationTransactionValidationError(
+            $type,
+            pht('Required'),
+            pht('Package name is required.'),
+            nonempty(last($xactions), null));
+
+          $error->setIsMissingFieldError(true);
+          $errors[] = $error;
+        }
+        break;
+      case PhabricatorOwnersPackageTransaction::TYPE_PRIMARY:
+        $missing = $this->validateIsEmptyTextField(
+          $object->getPrimaryOwnerPHID(),
+          $xactions);
+
+        if ($missing) {
+          $error = new PhabricatorApplicationTransactionValidationError(
+            $type,
+            pht('Required'),
+            pht('Packages must have a primary owner.'),
+            nonempty(last($xactions), null));
+
+          $error->setIsMissingFieldError(true);
+          $errors[] = $error;
+        }
+        break;
+    }
+
+    return $errors;
+  }
+
+  protected function extractFilePHIDsFromCustomTransaction(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    switch ($xaction->getTransactionType()) {
+      case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION:
+        return array($xaction->getNewValue());
+    }
+
+    return parent::extractFilePHIDsFromCustomTransaction($object, $xaction);
+  }
+
+  protected function shouldSendMail(
+    PhabricatorLiskDAO $object,
+    array $xactions) {
+    return true;
+  }
+
+  protected function getMailSubjectPrefix() {
+    return PhabricatorEnv::getEnvConfig('metamta.package.subject-prefix');
+  }
+
+  protected function getMailTo(PhabricatorLiskDAO $object) {
+    return array(
+      $object->getPrimaryOwnerPHID(),
+      $this->requireActor()->getPHID(),
+    );
+  }
+
+  protected function getMailCC(PhabricatorLiskDAO $object) {
+    // TODO: needOwners() this
+    return mpull($object->loadOwners(), 'getUserPHID');
+  }
+
+  protected function buildReplyHandler(PhabricatorLiskDAO $object) {
+    return id(new OwnersPackageReplyHandler())
+      ->setMailReceiver($object);
+  }
+
+  protected function buildMailTemplate(PhabricatorLiskDAO $object) {
+    $id = $object->getID();
+    $name = $object->getName();
+
+    return id(new PhabricatorMetaMTAMail())
+      ->setSubject($name)
+      ->addHeader('Thread-Topic', $object->getPHID());
+  }
+
+  protected function buildMailBody(
+    PhabricatorLiskDAO $object,
+    array $xactions) {
+
+    $body = parent::buildMailBody($object, $xactions);
+
+    $detail_uri = PhabricatorEnv::getProductionURI(
+      '/owners/package/'.$object->getID().'/');
+
+    $body->addLinkSection(
+      pht('PACKAGE DETAIL'),
+      $detail_uri);
+
+    return $body;
+  }
+
+}

src/applications/owners/query/PhabricatorOwnersPackageTransactionQuery.php

···
+<?php
+
+final class PhabricatorOwnersPackageTransactionQuery
+  extends PhabricatorApplicationTransactionQuery {
+
+  public function getTemplateApplicationTransaction() {
+    return new PhabricatorOwnersPackageTransaction();
+  }
+
+}

src/applications/owners/storage/PhabricatorOwnersPackage.php

···
 <?php
 
-final class PhabricatorOwnersPackage extends PhabricatorOwnersDAO
-  implements PhabricatorPolicyInterface {
+final class PhabricatorOwnersPackage
+  extends PhabricatorOwnersDAO
+  implements
+    PhabricatorPolicyInterface,
+    PhabricatorApplicationTransactionInterface {
 
   protected $name;
   protected $originalName;
   protected $auditingEnabled;
   protected $description;
   protected $primaryOwnerPHID;
+  protected $mailKey;
 
   private $unsavedOwners = self::ATTACHABLE;
   private $unsavedPaths = self::ATTACHABLE;
···
   private $oldPrimaryOwnerPHID;
   private $oldAuditingEnabled;
 
+  public static function initializeNewPackage(PhabricatorUser $actor) {
+    return id(new PhabricatorOwnersPackage())
+      ->setAuditingEnabled(0)
+      ->setPrimaryOwnerPHID($actor->getPHID());
+  }
+
   public function getCapabilities() {
     return array(
       PhabricatorPolicyCapability::CAN_VIEW,
···
         'description' => 'text',
         'primaryOwnerPHID' => 'phid?',
         'auditingEnabled' => 'bool',
+        'mailKey' => 'bytes20',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'key_phid' => null,
···
   public function generatePHID() {
     return PhabricatorPHID::generateNewPHID(
       PhabricatorOwnersPackagePHIDType::TYPECONST);
+  }
+
+  public function save() {
+    if (!$this->getMailKey()) {
+      $this->setMailKey(Filesystem::readRandomCharacters(20));
+    }
+
+    return parent::save();
   }
 
   public function attachUnsavedOwners(array $owners) {
···
     }
     return $result;
   }
+
+
+/* -(  PhabricatorApplicationTransactionInterface  )------------------------- */
+
+
+  public function getApplicationTransactionEditor() {
+    return new PhabricatorOwnersPackageTransactionEditor();
+  }
+
+  public function getApplicationTransactionObject() {
+    return $this;
+  }
+
+  public function getApplicationTransactionTemplate() {
+    return new PhabricatorOwnersPackageTransaction();
+  }
+
+  public function willRenderTimeline(
+    PhabricatorApplicationTransactionView $timeline,
+    AphrontRequest $request) {
+    return $timeline;
+  }
+
 }

src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php

···
+<?php
+
+final class PhabricatorOwnersPackageTransaction
+  extends PhabricatorApplicationTransaction {
+
+  const TYPE_NAME = 'owners.name';
+  const TYPE_PRIMARY = 'owners.primary';
+  const TYPE_OWNERS = 'owners.owners';
+  const TYPE_AUDITING = 'owners.auditing';
+  const TYPE_DESCRIPTION = 'owners.description';
+
+  public function getApplicationName() {
+    return 'owners';
+  }
+
+  public function getApplicationTransactionType() {
+    return PhabricatorOwnersPackagePHIDType::TYPECONST;
+  }
+
+  public function getRequiredHandlePHIDs() {
+    $phids = parent::getRequiredHandlePHIDs();
+
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    switch ($this->getTransactionType()) {
+      case self::TYPE_PRIMARY:
+        if ($old) {
+          $phids[] = $old;
+        }
+        if ($new) {
+          $phids[] = $new;
+        }
+        break;
+      case self::TYPE_OWNERS:
+        $add = array_diff($new, $old);
+        foreach ($add as $phid) {
+          $phids[] = $phid;
+        }
+        $rem = array_diff($old, $new);
+        foreach ($rem as $phid) {
+          $phids[] = $phid;
+        }
+        break;
+    }
+
+    return $phids;
+  }
+
+  public function shouldHide() {
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    switch ($this->getTransactionType()) {
+      case self::TYPE_DESCRIPTION:
+        return ($old === null);
+    }
+  }
+
+  public function getTitle() {
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+    $author_phid = $this->getAuthorPHID();
+
+    switch ($this->getTransactionType()) {
+      case self::TYPE_NAME:
+        if ($old === null) {
+          return pht(
+            '%s created this package.',
+            $this->renderHandleLink($author_phid));
+        } else {
+          return pht(
+            '%s renamed this package from "%s" to "%s".',
+            $this->renderHandleLink($author_phid),
+            $old,
+            $new);
+        }
+      case self::TYPE_PRIMARY:
+        return pht(
+          '%s changed the primary owner for this package from %s to %s.',
+          $this->renderHandleLink($author_phid),
+          $this->renderHandleLink($old),
+          $this->renderHandleLink($new));
+      case self::TYPE_OWNERS:
+        $add = array_diff($new, $old);
+        $rem = array_diff($old, $new);
+        if ($add && !$rem) {
+          return pht(
+            '%s added %s owner(s): %s.',
+            $this->renderHandleLink($author_phid),
+            count($add),
+            $this->renderHandleList($add));
+        } else if ($rem && !$add) {
+          return pht(
+            '%s removed %s owner(s): %s.',
+            $this->renderHandleLink($author_phid),
+            count($rem),
+            $this->renderHandleList($rem));
+        } else {
+          return pht(
+            '%s changed %s package owner(s), added %s: %s; removed %s: %s.',
+            $this->renderHandleLink($author_phid),
+            count($add) + count($rem),
+            count($add),
+            $this->renderHandleList($add),
+            count($rem),
+            $this->renderHandleList($rem));
+        }
+      case self::TYPE_AUDITING:
+        if ($new) {
+          return pht(
+            '%s enabled auditing for this package.',
+            $this->renderHandleLink($author_phid));
+        } else {
+          return pht(
+            '%s disabled auditing for this package.',
+            $this->renderHandleLink($author_phid));
+        }
+      case self::TYPE_DESCRIPTION:
+        return pht(
+          '%s updated the description for this package.',
+          $this->renderHandleLink($author_phid));
+    }
+
+    return parent::getTitle();
+  }
+
+  public function hasChangeDetails() {
+    switch ($this->getTransactionType()) {
+      case self::TYPE_DESCRIPTION:
+        return ($this->getOldValue() !== null);
+    }
+
+    return parent::hasChangeDetails();
+  }
+
+  public function renderChangeDetails(PhabricatorUser $viewer) {
+    switch ($this->getTransactionType()) {
+      case self::TYPE_DESCRIPTION:
+        $old = $this->getOldValue();
+        $new = $this->getNewValue();
+
+        return $this->renderTextCorpusChangeDetails(
+          $viewer,
+          $old,
+          $new);
+    }
+
+    return parent::renderChangeDetails($viewer);
+  }
+
+}

src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php

···
         'Are you absolutely certain you want to destroy these objects?',
       ),
 
+      '%s added %s owner(s): %s.' => array(
+        array(
+          '%s added an owner: %3$s.',
+          '%s added owners: %3$s.',
+        ),
+      ),
+
+      '%s removed %s owner(s): %s.' => array(
+        array(
+          '%s removed an owner: %3$s.',
+          '%s removed owners: %3$s.',
+        ),
+      ),
+
+      '%s changed %s package owner(s), added %s: %s; removed %s: %s.' => array(
+        '%s changed package owners, added: %4$s; removed: %6$s.',
+      ),
+
     );
   }