Store authors on image macros

Summary:
Currently, the author of an image macro is read from the attached file. This is messy and necessitates a join, and is not always correct. Instead, store the data when the macro is created.

This lays the groundwork for generalizing ApplicationSearch here. Ref T2625.

Test Plan: Migrated existing macros, created a new macro, checked web UI.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2625

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

resources/sql/patches/20130529.macroauthor.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_file.file_imagemacro
+  ADD authorPHID VARCHAR(64) COLLATE utf8_bin AFTER phid;

resources/sql/patches/20130529.macroauthormig.php

@@ -0,0 +1,39 @@
+<?php
+
+echo "Migrating macro authors...\n";
+foreach (new LiskMigrationIterator(new PhabricatorFileImageMacro()) as $macro) {
+  echo "Macro #".$macro->getID()."\n";
+
+  if ($macro->getAuthorPHID()) {
+    // Already have an author; skip it.
+    continue;
+  }
+
+  if (!$macro->getFilePHID()) {
+    // No valid file; skip it.
+    continue;
+  }
+
+  $file = id(new PhabricatorFile())->loadOneWhere(
+    'phid = %s',
+    $macro->getFilePHID());
+
+  if (!$file) {
+    // Couldn't load the file; skip it.
+    continue;
+  }
+
+  if (!$file->getAuthorPHID()) {
+    // File has no author; skip it.
+    continue;
+  }
+
+  queryfx(
+    $macro->establishConnection('w'),
+    'UPDATE %T SET authorPHID = %s WHERE id = %d',
+    $macro->getTableName(),
+    $file->getAuthorPHID(),
+    $macro->getID());
+}
+
+echo "Done.\n";

src/applications/macro/controller/PhabricatorMacroEditController.php

@@ -29,11 +29,12 @@ final class PhabricatorMacroEditController
       }
     } else {
       $macro = new PhabricatorFileImageMacro();
+      $macro->setAuthorPHID($user->getPHID());
     }
 
     $errors = array();
     $e_name = true;
-    $e_file = pht('Provide a URL or a file');
+    $e_file = null;
     $file = null;
     $can_fetch = PhabricatorEnv::getEnvConfig('security.allow-outbound-http');
 
@@ -97,6 +98,7 @@ final class PhabricatorMacroEditController
 
       if (!$macro->getID() && !$file) {
         $errors[] = pht('You must upload an image to create a macro.');
+        $e_file = pht('Required');
       }
 
       if (!$errors) {
@@ -242,12 +244,11 @@ final class PhabricatorMacroEditController
         ->setUser($request->getUser());
 
       if ($can_fetch) {
-        $upload_form
-          ->appendChild(
-            id(new AphrontFormTextControl())
-              ->setLabel(pht('URL'))
-              ->setName('url')
-              ->setValue($request->getStr('url')));
+        $upload_form->appendChild(
+          id(new AphrontFormTextControl())
+            ->setLabel(pht('URL'))
+            ->setName('url')
+            ->setValue($request->getStr('url')));
       }
 
       $upload_form

src/applications/macro/controller/PhabricatorMacroListController.php

@@ -56,10 +56,7 @@ final class PhabricatorMacroListController
       $author_phids = array();
     }
 
-    $files = mpull($macros, 'getFile');
-    if ($files) {
-      $author_phids += mpull($files, 'getAuthorPHID', 'getAuthorPHID');
-    }
+    $author_phids += mpull($macros, 'getAuthorPHID', 'getAuthorPHID');
 
     $this->loadHandles($author_phids);
     $author_handles = array_select_keys($this->getLoadedHandles(), $authors);
@@ -102,18 +99,23 @@ final class PhabricatorMacroListController
       if ($file) {
         $item->setImageURI($file->getThumb280x210URI());
         $item->setImageSize(280, 210);
-        if ($file->getAuthorPHID()) {
-          $author_handle = $this->getHandle($file->getAuthorPHID());
-          $item->appendChild(
-            pht('Created by %s', $author_handle->renderLink()));
-        }
-        $datetime = phabricator_date($file->getDateCreated(), $viewer);
+      }
+
+      if ($macro->getDateCreated()) {
+        $datetime = phabricator_date($macro->getDateCreated(), $viewer);
         $item->appendChild(
           phutil_tag(
             'div',
             array(),
             pht('Created on %s', $datetime)));
       }
+
+      if ($macro->getAuthorPHID()) {
+        $author_handle = $this->getHandle($macro->getAuthorPHID());
+        $item->appendChild(
+          pht('Created by %s', $author_handle->renderLink()));
+      }
+
       $item->setURI($this->getApplicationURI('/view/'.$macro->getID().'/'));
       $item->setHeader($macro->getName());
 

src/applications/macro/query/PhabricatorMacroQuery.php

@@ -52,9 +52,8 @@ final class PhabricatorMacroQuery
 
     $rows = queryfx_all(
       $conn,
-      'SELECT m.* FROM %T m %Q %Q %Q %Q',
+      'SELECT m.* FROM %T m %Q %Q %Q',
       $macro_table->getTableName(),
-      $this->buildJoinClause($conn),
       $this->buildWhereClause($conn),
       $this->buildOrderClause($conn),
       $this->buildLimitClause($conn));
@@ -62,20 +61,6 @@ final class PhabricatorMacroQuery
     return $macro_table->loadAllFromArray($rows);
   }
 
-  protected function buildJoinClause(AphrontDatabaseConnection $conn) {
-    $joins = array();
-
-    if ($this->authors) {
-      $file_table = new PhabricatorFile();
-      $joins[] = qsprintf(
-        $conn,
-        'JOIN %T f ON m.filePHID = f.phid',
-        $file_table->getTableName());
-    }
-
-    return implode(' ', $joins);
-  }
-
   protected function buildWhereClause(AphrontDatabaseConnection $conn) {
     $where = array();
 
@@ -96,7 +81,7 @@ final class PhabricatorMacroQuery
     if ($this->authors) {
       $where[] = qsprintf(
         $conn,
-        'f.authorPHID IN (%Ls)',
+        'm.authorPHID IN (%Ls)',
         $this->authors);
     }
 

src/applications/macro/storage/PhabricatorFileImageMacro.php

@@ -6,6 +6,7 @@ final class PhabricatorFileImageMacro extends PhabricatorFileDAO
     PhabricatorApplicationTransactionInterface,
     PhabricatorPolicyInterface {
 
+  protected $authorPHID;
   protected $filePHID;
   protected $phid;
   protected $name;

src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php

@@ -1318,6 +1318,14 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList {
         'type' => 'sql',
         'name' => $this->getPatchPath('20130524.repoxactions.sql'),
       ),
+      '20130529.macroauthor.sql' => array(
+        'type' => 'sql',
+        'name' => $this->getPatchPath('20130529.macroauthor.sql'),
+      ),
+      '20130529.macroauthormig.php' => array(
+        'type' => 'php',
+        'name' => $this->getPatchPath('20130529.macroauthormig.php'),
+      ),
     );
   }
 }