Pholio phixes

Summary: Fixes T3573 and T3541. (Note T3573 is only fixed moving forward as I changed how transactions are converted in the editor.)

Test Plan: created mocks with combinations of set and empty image title and descriptions. verified when i updated title / description transactions showed up. called up the email via command line tools and saw nice 'created' text.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T3541, T3573

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

src/applications/pholio/controller/PholioMockEditController.php

@@ -81,6 +81,7 @@ final class PholioMockEditController extends PholioController {
 
       if (!strlen($request->getStr('name'))) {
         $e_name = 'Required';
+        $errors[] = pht('You must give the mock a name.');
       }
 
       $file_phids = $request->getArr('file_phids');

src/applications/pholio/editor/PholioMockEditor.php

@@ -49,7 +49,7 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
         $name = null;
         $phid = null;
         $image = $this->getImageForXaction($object, $xaction);
-        if ($image && $image->getName()) {
+        if ($image) {
           $name = $image->getName();
           $phid = $image->getPHID();
         }
@@ -58,7 +58,7 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
         $description = null;
         $phid = null;
         $image = $this->getImageForXaction($object, $xaction);
-        if ($image && $image->getDescription()) {
+        if ($image) {
           $description = $image->getDescription();
           $phid = $image->getPHID();
         }

src/applications/pholio/storage/PholioTransaction.php

@@ -49,13 +49,11 @@ final class PholioTransaction extends PhabricatorApplicationTransaction {
     $old = $this->getOldValue();
 
     switch ($this->getTransactionType()) {
-      case PholioTransactionType::TYPE_NAME:
       case PholioTransactionType::TYPE_DESCRIPTION:
         return ($old === null);
       case PholioTransactionType::TYPE_IMAGE_NAME:
       case PholioTransactionType::TYPE_IMAGE_DESCRIPTION:
-        $old_value = reset($old);
+        return ($old === array(null => null));
-        return ($old_value === null);
     }
 
     return parent::shouldHide();
@@ -86,11 +84,18 @@ final class PholioTransaction extends PhabricatorApplicationTransaction {
     $type = $this->getTransactionType();
     switch ($type) {
       case PholioTransactionType::TYPE_NAME:
-        return pht(
+        if ($old === null) {
-          '%s renamed this mock from "%s" to "%s".',
+          return pht(
-          $this->renderHandleLink($author_phid),
+            '%s created "%s".',
-          $old,
+            $this->renderHandleLink($author_phid),
-          $new);
+            $new);
+        } else {
+          return pht(
+            '%s renamed this mock from "%s" to "%s".',
+            $this->renderHandleLink($author_phid),
+            $old,
+            $new);
+        }
         break;
       case PholioTransactionType::TYPE_DESCRIPTION:
         return pht(
@@ -247,6 +252,9 @@ final class PholioTransaction extends PhabricatorApplicationTransaction {
 
     switch ($this->getTransactionType()) {
       case PholioTransactionType::TYPE_NAME:
+        if ($old === null) {
+          return PhabricatorTransactions::COLOR_GREEN;
+        }
       case PholioTransactionType::TYPE_DESCRIPTION:
       case PholioTransactionType::TYPE_IMAGE_NAME:
       case PholioTransactionType::TYPE_IMAGE_DESCRIPTION:

src/applications/pholio/view/PholioMockImagesView.php

@@ -72,9 +72,8 @@ final class PholioMockImagesView extends AphrontView {
         'pageURI' => '/M'.$mock->getID().'/'.$image->getID().'/',
         'width'   => $x,
         'height'  => $y,
-        'title'   => $file->getName(),
+        'title'   => $image->getName(),
-        'desc'    => 'Lorem ipsum dolor sit amet: there is no way to set any '.
+        'desc'    => $image->getDescription(),
-                     'descriptive text yet; were there, it would appear here.',
       );
     }