Update Spaces for modular transactions

Summary: Updates the Spaces application for modular transactions, seemed easy to bang out.

Test Plan: Create a space, edit a space, archive a space. Verify default space works as intended.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

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

src/__phutil_library_map__.php

@@ -3937,13 +3937,18 @@ phutil_register_library_map(array(
     'PhabricatorSpacesInterface' => 'applications/spaces/interface/PhabricatorSpacesInterface.php',
     'PhabricatorSpacesListController' => 'applications/spaces/controller/PhabricatorSpacesListController.php',
     'PhabricatorSpacesNamespace' => 'applications/spaces/storage/PhabricatorSpacesNamespace.php',
+    'PhabricatorSpacesNamespaceArchiveTransaction' => 'applications/spaces/xaction/PhabricatorSpacesNamespaceArchiveTransaction.php',
     'PhabricatorSpacesNamespaceDatasource' => 'applications/spaces/typeahead/PhabricatorSpacesNamespaceDatasource.php',
+    'PhabricatorSpacesNamespaceDefaultTransaction' => 'applications/spaces/xaction/PhabricatorSpacesNamespaceDefaultTransaction.php',
+    'PhabricatorSpacesNamespaceDescriptionTransaction' => 'applications/spaces/xaction/PhabricatorSpacesNamespaceDescriptionTransaction.php',
     'PhabricatorSpacesNamespaceEditor' => 'applications/spaces/editor/PhabricatorSpacesNamespaceEditor.php',
+    'PhabricatorSpacesNamespaceNameTransaction' => 'applications/spaces/xaction/PhabricatorSpacesNamespaceNameTransaction.php',
     'PhabricatorSpacesNamespacePHIDType' => 'applications/spaces/phid/PhabricatorSpacesNamespacePHIDType.php',
     'PhabricatorSpacesNamespaceQuery' => 'applications/spaces/query/PhabricatorSpacesNamespaceQuery.php',
     'PhabricatorSpacesNamespaceSearchEngine' => 'applications/spaces/query/PhabricatorSpacesNamespaceSearchEngine.php',
     'PhabricatorSpacesNamespaceTransaction' => 'applications/spaces/storage/PhabricatorSpacesNamespaceTransaction.php',
     'PhabricatorSpacesNamespaceTransactionQuery' => 'applications/spaces/query/PhabricatorSpacesNamespaceTransactionQuery.php',
+    'PhabricatorSpacesNamespaceTransactionType' => 'applications/spaces/xaction/PhabricatorSpacesNamespaceTransactionType.php',
     'PhabricatorSpacesNoAccessController' => 'applications/spaces/controller/PhabricatorSpacesNoAccessController.php',
     'PhabricatorSpacesRemarkupRule' => 'applications/spaces/remarkup/PhabricatorSpacesRemarkupRule.php',
     'PhabricatorSpacesSchemaSpec' => 'applications/spaces/storage/PhabricatorSpacesSchemaSpec.php',
@@ -9373,13 +9378,18 @@ phutil_register_library_map(array(
       'PhabricatorApplicationTransactionInterface',
       'PhabricatorDestructibleInterface',
     ),
+    'PhabricatorSpacesNamespaceArchiveTransaction' => 'PhabricatorSpacesNamespaceTransactionType',
     'PhabricatorSpacesNamespaceDatasource' => 'PhabricatorTypeaheadDatasource',
+    'PhabricatorSpacesNamespaceDefaultTransaction' => 'PhabricatorSpacesNamespaceTransactionType',
+    'PhabricatorSpacesNamespaceDescriptionTransaction' => 'PhabricatorSpacesNamespaceTransactionType',
     'PhabricatorSpacesNamespaceEditor' => 'PhabricatorApplicationTransactionEditor',
+    'PhabricatorSpacesNamespaceNameTransaction' => 'PhabricatorSpacesNamespaceTransactionType',
     'PhabricatorSpacesNamespacePHIDType' => 'PhabricatorPHIDType',
     'PhabricatorSpacesNamespaceQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorSpacesNamespaceSearchEngine' => 'PhabricatorApplicationSearchEngine',
-    'PhabricatorSpacesNamespaceTransaction' => 'PhabricatorApplicationTransaction',
+    'PhabricatorSpacesNamespaceTransaction' => 'PhabricatorModularTransaction',
     'PhabricatorSpacesNamespaceTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
+    'PhabricatorSpacesNamespaceTransactionType' => 'PhabricatorModularTransactionType',
     'PhabricatorSpacesNoAccessController' => 'PhabricatorSpacesController',
     'PhabricatorSpacesRemarkupRule' => 'PhabricatorObjectRemarkupRule',
     'PhabricatorSpacesSchemaSpec' => 'PhabricatorConfigSchemaSpec',

src/applications/spaces/__tests__/PhabricatorSpacesTestCase.php

@@ -190,8 +190,10 @@ final class PhabricatorSpacesTestCase extends PhabricatorTestCase {
 
     $space = PhabricatorSpacesNamespace::initializeNewNamespace($actor);
 
-    $type_name = PhabricatorSpacesNamespaceTransaction::TYPE_NAME;
+    $type_name =
-    $type_default = PhabricatorSpacesNamespaceTransaction::TYPE_DEFAULT;
+      PhabricatorSpacesNamespaceNameTransaction::TRANSACTIONTYPE;
+    $type_default =
+      PhabricatorSpacesNamespaceDefaultTransaction::TRANSACTIONTYPE;
     $type_view = PhabricatorTransactions::TYPE_VIEW_POLICY;
     $type_edit = PhabricatorTransactions::TYPE_EDIT_POLICY;
 

src/applications/spaces/controller/PhabricatorSpacesArchiveController.php

@@ -23,7 +23,8 @@ final class PhabricatorSpacesArchiveController
     $cancel_uri = '/'.$space->getMonogram();
 
     if ($request->isFormPost()) {
-      $type_archive = PhabricatorSpacesNamespaceTransaction::TYPE_ARCHIVE;
+      $type_archive =
+        PhabricatorSpacesNamespaceArchiveTransaction::TRANSACTIONTYPE;
 
       $xactions = array();
       $xactions[] = id(new PhabricatorSpacesNamespaceTransaction())

src/applications/spaces/controller/PhabricatorSpacesEditController.php

@@ -67,9 +67,12 @@ final class PhabricatorSpacesEditController
       $v_view = $request->getStr('viewPolicy');
       $v_edit = $request->getStr('editPolicy');
 
-      $type_name = PhabricatorSpacesNamespaceTransaction::TYPE_NAME;
+      $type_name =
-      $type_desc = PhabricatorSpacesNamespaceTransaction::TYPE_DESCRIPTION;
+        PhabricatorSpacesNamespaceNameTransaction::TRANSACTIONTYPE;
-      $type_default = PhabricatorSpacesNamespaceTransaction::TYPE_DEFAULT;
+      $type_desc =
+        PhabricatorSpacesNamespaceDescriptionTransaction::TRANSACTIONTYPE;
+      $type_default =
+        PhabricatorSpacesNamespaceDefaultTransaction::TRANSACTIONTYPE;
       $type_view = PhabricatorTransactions::TYPE_VIEW_POLICY;
       $type_edit = PhabricatorTransactions::TYPE_EDIT_POLICY;
 

src/applications/spaces/controller/PhabricatorSpacesViewController.php

@@ -39,7 +39,7 @@ final class PhabricatorSpacesViewController
       ->setHeaderIcon('fa-th-large');
 
     if ($space->getIsArchived()) {
-      $header->setStatus('fa-ban', 'red', pht('Archived'));
+      $header->setStatus('fa-ban', 'indigo', pht('Archived'));
     } else {
       $header->setStatus('fa-check', 'bluegrey', pht('Active'));
     }

src/applications/spaces/editor/PhabricatorSpacesNamespaceEditor.php

@@ -14,153 +14,18 @@ final class PhabricatorSpacesNamespaceEditor
   public function getTransactionTypes() {
     $types = parent::getTransactionTypes();
 
-    $types[] = PhabricatorSpacesNamespaceTransaction::TYPE_NAME;
-    $types[] = PhabricatorSpacesNamespaceTransaction::TYPE_DESCRIPTION;
-    $types[] = PhabricatorSpacesNamespaceTransaction::TYPE_DEFAULT;
-    $types[] = PhabricatorSpacesNamespaceTransaction::TYPE_ARCHIVE;
-
     $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
     $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
 
     return $types;
   }
 
-  protected function getCustomTransactionOldValue(
+  public function getCreateObjectTitle($author, $object) {
-    PhabricatorLiskDAO $object,
+    return pht('%s created this space.', $author);
-    PhabricatorApplicationTransaction $xaction) {
-
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorSpacesNamespaceTransaction::TYPE_NAME:
-        $name = $object->getNamespaceName();
-        if (!strlen($name)) {
-          return null;
-        }
-        return $name;
-      case PhabricatorSpacesNamespaceTransaction::TYPE_DESCRIPTION:
-        if ($this->getIsNewObject()) {
-          return null;
-        }
-        return $object->getDescription();
-      case PhabricatorSpacesNamespaceTransaction::TYPE_ARCHIVE:
-        return $object->getIsArchived();
-      case PhabricatorSpacesNamespaceTransaction::TYPE_DEFAULT:
-        return $object->getIsDefaultNamespace() ? 1 : null;
-      case PhabricatorTransactions::TYPE_VIEW_POLICY:
-        return $object->getViewPolicy();
-      case PhabricatorTransactions::TYPE_EDIT_POLICY:
-        return $object->getEditPolicy();
-    }
-
-    return parent::getCustomTransactionOldValue($object, $xaction);
   }
 
-  protected function getCustomTransactionNewValue(
+  public function getCreateObjectTitleForFeed($author, $object) {
-    PhabricatorLiskDAO $object,
+    return pht('%s created space %s.', $author, $object);
-    PhabricatorApplicationTransaction $xaction) {
-
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorSpacesNamespaceTransaction::TYPE_NAME:
-      case PhabricatorSpacesNamespaceTransaction::TYPE_DESCRIPTION:
-      case PhabricatorTransactions::TYPE_VIEW_POLICY:
-      case PhabricatorTransactions::TYPE_EDIT_POLICY:
-        return $xaction->getNewValue();
-      case PhabricatorSpacesNamespaceTransaction::TYPE_ARCHIVE:
-        return $xaction->getNewValue() ? 1 : 0;
-      case PhabricatorSpacesNamespaceTransaction::TYPE_DEFAULT:
-        return $xaction->getNewValue() ? 1 : null;
-    }
-
-    return parent::getCustomTransactionNewValue($object, $xaction);
-  }
-
-  protected function applyCustomInternalTransaction(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    $new = $xaction->getNewValue();
-
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorSpacesNamespaceTransaction::TYPE_NAME:
-        $object->setNamespaceName($new);
-        return;
-      case PhabricatorSpacesNamespaceTransaction::TYPE_DESCRIPTION:
-        $object->setDescription($new);
-        return;
-      case PhabricatorSpacesNamespaceTransaction::TYPE_DEFAULT:
-        $object->setIsDefaultNamespace($new ? 1 : null);
-        return;
-      case PhabricatorSpacesNamespaceTransaction::TYPE_ARCHIVE:
-        $object->setIsArchived($new ? 1 : 0);
-        return;
-      case PhabricatorTransactions::TYPE_VIEW_POLICY:
-        $object->setViewPolicy($new);
-        return;
-      case PhabricatorTransactions::TYPE_EDIT_POLICY:
-        $object->setEditPolicy($new);
-        return;
-    }
-
-    return parent::applyCustomInternalTransaction($object, $xaction);
-  }
-
-  protected function applyCustomExternalTransaction(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorSpacesNamespaceTransaction::TYPE_NAME:
-      case PhabricatorSpacesNamespaceTransaction::TYPE_DESCRIPTION:
-      case PhabricatorSpacesNamespaceTransaction::TYPE_DEFAULT:
-      case PhabricatorSpacesNamespaceTransaction::TYPE_ARCHIVE:
-      case PhabricatorTransactions::TYPE_VIEW_POLICY:
-      case PhabricatorTransactions::TYPE_EDIT_POLICY:
-        return;
-    }
-
-    return parent::applyCustomExternalTransaction($object, $xaction);
-  }
-
-  protected function validateTransaction(
-    PhabricatorLiskDAO $object,
-    $type,
-    array $xactions) {
-
-    $errors = parent::validateTransaction($object, $type, $xactions);
-
-    switch ($type) {
-      case PhabricatorSpacesNamespaceTransaction::TYPE_NAME:
-        $missing = $this->validateIsEmptyTextField(
-          $object->getNamespaceName(),
-          $xactions);
-
-        if ($missing) {
-          $error = new PhabricatorApplicationTransactionValidationError(
-            $type,
-            pht('Required'),
-            pht('Spaces must have a name.'),
-            nonempty(last($xactions), null));
-
-          $error->setIsMissingFieldError(true);
-          $errors[] = $error;
-        }
-        break;
-      case PhabricatorSpacesNamespaceTransaction::TYPE_DEFAULT:
-        if (!$this->getIsNewObject()) {
-          foreach ($xactions as $xaction) {
-            $errors[] = new PhabricatorApplicationTransactionValidationError(
-              $type,
-              pht('Invalid'),
-              pht(
-                'Only the first space created can be the default space, and '.
-                'it must remain the default space evermore.'),
-              $xaction);
-          }
-        }
-        break;
-
-    }
-
-    return $errors;
   }
 
 }

src/applications/spaces/storage/PhabricatorSpacesNamespaceTransaction.php

@@ -1,12 +1,7 @@
 <?php
 
 final class PhabricatorSpacesNamespaceTransaction
-  extends PhabricatorApplicationTransaction {
+  extends PhabricatorModularTransaction {
-
-  const TYPE_NAME = 'spaces:name';
-  const TYPE_DEFAULT = 'spaces:default';
-  const TYPE_DESCRIPTION = 'spaces:description';
-  const TYPE_ARCHIVE = 'spaces:archive';
 
   public function getApplicationName() {
     return 'spaces';
@@ -20,78 +15,8 @@ final class PhabricatorSpacesNamespaceTransaction
     return null;
   }
 
-  public function shouldHide() {
+  public function getBaseTransactionClass() {
-    $old = $this->getOldValue();
+    return 'PhabricatorSpacesNamespaceTransactionType';
-
-    switch ($this->getTransactionType()) {
-      case self::TYPE_DESCRIPTION:
-        return ($old === null);
-    }
-
-    return parent::shouldHide();
-  }
-
-  public function hasChangeDetails() {
-    switch ($this->getTransactionType()) {
-      case self::TYPE_DESCRIPTION:
-        return true;
-    }
-
-    return parent::hasChangeDetails();
-  }
-
-  public function getRemarkupBlocks() {
-    $blocks = parent::getRemarkupBlocks();
-
-    switch ($this->getTransactionType()) {
-      case self::TYPE_DESCRIPTION:
-        $blocks[] = $this->getNewValue();
-        break;
-    }
-
-    return $blocks;
-  }
-
-  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 space.',
-            $this->renderHandleLink($author_phid));
-        } else {
-          return pht(
-            '%s renamed this space from "%s" to "%s".',
-            $this->renderHandleLink($author_phid),
-            $old,
-            $new);
-        }
-      case self::TYPE_DESCRIPTION:
-        return pht(
-          '%s updated the description for this space.',
-            $this->renderHandleLink($author_phid));
-      case self::TYPE_DEFAULT:
-        return pht(
-          '%s made this the default space.',
-          $this->renderHandleLink($author_phid));
-      case self::TYPE_ARCHIVE:
-        if ($new) {
-          return pht(
-            '%s archived this space.',
-            $this->renderHandleLink($author_phid));
-        } else {
-          return pht(
-            '%s activated this space.',
-            $this->renderHandleLink($author_phid));
-        }
-    }
-
-    return parent::getTitle();
   }
 
 }

src/applications/spaces/xaction/PhabricatorSpacesNamespaceArchiveTransaction.php

@@ -0,0 +1,60 @@
+<?php
+
+final class PhabricatorSpacesNamespaceArchiveTransaction
+  extends PhabricatorSpacesNamespaceTransactionType {
+
+  const TRANSACTIONTYPE = 'spaces:archive';
+
+  public function generateOldValue($object) {
+    return $object->getIsArchived();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setIsArchived((int)$value);
+  }
+
+  public function getTitle() {
+    $new = $this->getNewValue();
+    if ($new) {
+      return pht(
+        '%s archived this space.',
+        $this->renderAuthor());
+    } else {
+      return pht(
+        '%s activated this space.',
+        $this->renderAuthor());
+    }
+  }
+
+  public function getTitleForFeed() {
+    $new = $this->getNewValue();
+    if ($new) {
+      return pht(
+        '%s archived space %s.',
+        $this->renderAuthor(),
+        $this->renderObject());
+    } else {
+      return pht(
+        '%s activated space %s.',
+        $this->renderAuthor(),
+        $this->renderObject());
+    }
+  }
+
+  public function getIcon() {
+    $new = $this->getNewValue();
+    if ($new) {
+      return 'fa-ban';
+    } else {
+      return 'fa-check';
+    }
+  }
+
+  public function getColor() {
+    $new = $this->getNewValue();
+    if ($new) {
+      return 'indigo';
+    }
+  }
+
+}

src/applications/spaces/xaction/PhabricatorSpacesNamespaceDefaultTransaction.php

@@ -0,0 +1,44 @@
+<?php
+
+final class PhabricatorSpacesNamespaceDefaultTransaction
+  extends PhabricatorSpacesNamespaceTransactionType {
+
+  const TRANSACTIONTYPE = 'spaces:default';
+
+  public function generateOldValue($object) {
+    return $object->getIsDefaultNamespace();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setIsDefaultNamespace($value);
+  }
+
+  public function getTitle() {
+    return pht(
+      '%s made this the default space.',
+      $this->renderAuthor());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s made space %s the default space.',
+      $this->renderAuthor(),
+      $this->renderObject());
+
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    if (!$this->isNewObject()) {
+      foreach ($xactions as $xaction) {
+        $errors[] = $this->newInvalidError(
+          pht('Only the first space created can be the default space, and '.
+              'it must remain the default space evermore.'));
+      }
+    }
+
+    return $errors;
+  }
+
+}

src/applications/spaces/xaction/PhabricatorSpacesNamespaceDescriptionTransaction.php

@@ -0,0 +1,57 @@
+<?php
+
+final class PhabricatorSpacesNamespaceDescriptionTransaction
+  extends PhabricatorSpacesNamespaceTransactionType {
+
+  const TRANSACTIONTYPE = 'spaces:description';
+
+  public function generateOldValue($object) {
+    return $object->getDescription();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setDescription($value);
+  }
+
+  public function getTitle() {
+    return pht(
+      '%s updated the space description.',
+      $this->renderAuthor());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s updated the space description for %s.',
+      $this->renderAuthor(),
+      $this->renderObject());
+  }
+
+  public function hasChangeDetailView() {
+    return true;
+  }
+
+  public function getMailDiffSectionHeader() {
+    return pht('CHANGES TO SPACE DESCRIPTION');
+  }
+
+  public function newChangeDetailView() {
+    $viewer = $this->getViewer();
+
+    return id(new PhabricatorApplicationTransactionTextDiffDetailView())
+      ->setViewer($viewer)
+      ->setOldText($this->getOldValue())
+      ->setNewText($this->getNewValue());
+  }
+
+  public function newRemarkupChanges() {
+    $changes = array();
+
+    $changes[] = $this->newRemarkupChange()
+      ->setOldValue($this->getOldValue())
+      ->setNewValue($this->getNewValue());
+
+    return $changes;
+  }
+
+
+}

src/applications/spaces/xaction/PhabricatorSpacesNamespaceNameTransaction.php

@@ -0,0 +1,62 @@
+<?php
+
+final class PhabricatorSpacesNamespaceNameTransaction
+  extends PhabricatorSpacesNamespaceTransactionType {
+
+  const TRANSACTIONTYPE = 'spaces:name';
+
+  public function generateOldValue($object) {
+    return $object->getNamespaceName();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setNamespaceName($value);
+  }
+
+  public function getTitle() {
+    $old = $this->getOldValue();
+    if (!strlen($old)) {
+      return pht(
+        '%s created this space.',
+        $this->renderAuthor());
+    } else {
+      return pht(
+        '%s renamed this space from %s to %s.',
+        $this->renderAuthor(),
+        $this->renderOldValue(),
+        $this->renderNewValue());
+    }
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s renamed space %s from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderObject(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    if ($this->isEmptyTextTransaction($object->getNamespaceName(), $xactions)) {
+      $errors[] = $this->newRequiredError(
+        pht('Spaces must have a name.'));
+    }
+
+    $max_length = $object->getColumnMaximumByteLength('namespaceName');
+    foreach ($xactions as $xaction) {
+      $new_value = $xaction->getNewValue();
+      $new_length = strlen($new_value);
+      if ($new_length > $max_length) {
+        $errors[] = $this->newInvalidError(
+          pht('The name can be no longer than %s characters.',
+          new PhutilNumber($max_length)));
+      }
+    }
+
+    return $errors;
+  }
+
+}

src/applications/spaces/xaction/PhabricatorSpacesNamespaceTransactionType.php

@@ -0,0 +1,4 @@
+<?php
+
+abstract class PhabricatorSpacesNamespaceTransactionType
+  extends PhabricatorModularTransactionType {}