Project revamp part 2: Edit

Summary:
Taking a pass at revamping the edit pages in Projects. Specifically:

 - Remove EditMainController
 - Move actions from EditMain to Profile
 - Move properties from EditMain to Profile
 - Move timeline from EditMain to Profile
 - Move Open Tasks from Profile to sidenavicon
 - Add custom icons and colors to timeline

Feel free to bang on this a bit and give feedback, feels generally correct to me.

Test Plan: Edit everything I could on various projects. Check links, timelines, actions.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

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

src/__phutil_library_map__.php

@@ -2204,7 +2204,6 @@ phutil_register_library_map(array(
     'PhabricatorProjectDescriptionField' => 'applications/project/customfield/PhabricatorProjectDescriptionField.php',
     'PhabricatorProjectEditDetailsController' => 'applications/project/controller/PhabricatorProjectEditDetailsController.php',
     'PhabricatorProjectEditIconController' => 'applications/project/controller/PhabricatorProjectEditIconController.php',
-    'PhabricatorProjectEditMainController' => 'applications/project/controller/PhabricatorProjectEditMainController.php',
     'PhabricatorProjectEditPictureController' => 'applications/project/controller/PhabricatorProjectEditPictureController.php',
     'PhabricatorProjectEditorTestCase' => 'applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php',
     'PhabricatorProjectFeedController' => 'applications/project/controller/PhabricatorProjectFeedController.php',
@@ -5444,7 +5443,6 @@ phutil_register_library_map(array(
     'PhabricatorProjectDescriptionField' => 'PhabricatorProjectStandardCustomField',
     'PhabricatorProjectEditDetailsController' => 'PhabricatorProjectController',
     'PhabricatorProjectEditIconController' => 'PhabricatorProjectController',
-    'PhabricatorProjectEditMainController' => 'PhabricatorProjectController',
     'PhabricatorProjectEditPictureController' => 'PhabricatorProjectController',
     'PhabricatorProjectEditorTestCase' => 'PhabricatorTestCase',
     'PhabricatorProjectFeedController' => 'PhabricatorProjectController',

src/applications/files/controller/PhabricatorFileComposeController.php

@@ -72,7 +72,7 @@ final class PhabricatorFileComposeController
         ));
 
       if ($project_phid) {
-        $edit_uri = '/project/edit/'.$project->getID().'/';
+        $edit_uri = '/project/profile/'.$project->getID().'/';
 
         $xactions = array();
         $xactions[] = id(new PhabricatorProjectTransaction())

src/applications/project/application/PhabricatorProjectApplication.php

@@ -43,7 +43,6 @@ final class PhabricatorProjectApplication extends PhabricatorApplication {
       '/project/' => array(
         '(?:query/(?P<queryKey>[^/]+)/)?' => 'PhabricatorProjectListController',
         'filter/(?P<filter>[^/]+)/' => 'PhabricatorProjectListController',
-        'edit/(?P<id>[1-9]\d*)/' => 'PhabricatorProjectEditMainController',
         'details/(?P<id>[1-9]\d*)/'
           => 'PhabricatorProjectEditDetailsController',
         'archive/(?P<id>[1-9]\d*)/'

src/applications/project/controller/PhabricatorProjectArchiveController.php

@@ -26,7 +26,7 @@ final class PhabricatorProjectArchiveController
       return new Aphront404Response();
     }
 
-    $edit_uri = $this->getApplicationURI('edit/'.$project->getID().'/');
+    $edit_uri = $this->getApplicationURI('profile/'.$project->getID().'/');
 
     if ($request->isFormPost()) {
       if ($project->isArchived()) {

src/applications/project/controller/PhabricatorProjectController.php

@@ -7,28 +7,27 @@ abstract class PhabricatorProjectController extends PhabricatorController {
   }
 
   public function buildSideNavView($for_app = false) {
-    $user = $this->getRequest()->getUser();
+    $viewer = $this->getViewer();
 
     $nav = new AphrontSideNavFilterView();
     $nav->setBaseURI(new PhutilURI($this->getApplicationURI()));
 
     $id = null;
     if ($for_app) {
-      $user = $this->getRequest()->getUser();
       $id = $this->getRequest()->getURIData('id');
       if ($id) {
         $nav->addFilter("profile/{$id}/", pht('Profile'));
         $nav->addFilter("board/{$id}/", pht('Workboard'));
         $nav->addFilter("members/{$id}/", pht('Members'));
         $nav->addFilter("feed/{$id}/", pht('Feed'));
-        $nav->addFilter("edit/{$id}/", pht('Edit'));
+        $nav->addFilter("details/{$id}/", pht('Edit Details'));
       }
       $nav->addFilter('create', pht('Create Project'));
     }
 
     if (!$id) {
       id(new PhabricatorProjectSearchEngine())
-        ->setViewer($user)
+        ->setViewer($viewer)
         ->addNavigationItems($nav->getMenu());
     }
 
@@ -38,13 +37,13 @@ abstract class PhabricatorProjectController extends PhabricatorController {
   }
 
   public function buildIconNavView(PhabricatorProject $project) {
-    $user = $this->getRequest()->getUser();
+    $viewer = $this->getViewer();
     $id = $project->getID();
     $picture = $project->getProfileImageURI();
     $name = $project->getName();
 
     $columns = id(new PhabricatorProjectColumnQuery())
-      ->setViewer($user)
+      ->setViewer($viewer)
       ->withProjectPHIDs(array($project->getPHID()))
       ->execute();
     if ($columns) {
@@ -58,9 +57,20 @@ abstract class PhabricatorProjectController extends PhabricatorController {
     $nav->setBaseURI(new PhutilURI($this->getApplicationURI()));
     $nav->addIcon("profile/{$id}/", $name, null, $picture);
     $nav->addIcon("board/{$id}/", pht('Workboard'), $board_icon);
+
+    $class = 'PhabricatorManiphestApplication';
+    if (PhabricatorApplication::isClassInstalledForViewer($class, $viewer)) {
+      $phid = $project->getPHID();
+      $query_uri = urisprintf(
+        '/maniphest/?statuses=%s&allProjects=%s#R',
+        implode(',', ManiphestTaskStatus::getOpenStatusConstants()),
+      $phid);
+      $nav->addIcon(null, pht('Open Tasks'), 'fa-anchor', null, $query_uri);
+    }
+
     $nav->addIcon("feed/{$id}/", pht('Feed'), 'fa-newspaper-o');
     $nav->addIcon("members/{$id}/", pht('Members'), 'fa-group');
-    $nav->addIcon("edit/{$id}/", pht('Edit'), 'fa-pencil');
+    $nav->addIcon("details/{$id}/", pht('Edit Details'), 'fa-pencil');
 
     return $nav;
   }

src/applications/project/controller/PhabricatorProjectEditDetailsController.php

@@ -149,13 +149,8 @@ final class PhabricatorProjectEditDetailsController
             ));
         }
 
-        if ($is_new) {
-          $redirect_uri =
-            $this->getApplicationURI('profile/'.$project->getID().'/');
-        } else {
-          $redirect_uri =
-            $this->getApplicationURI('edit/'.$project->getID().'/');
-        }
+        $redirect_uri =
+          $this->getApplicationURI('profile/'.$project->getID().'/');
 
         return id(new AphrontRedirectResponse())->setURI($redirect_uri);
       } catch (PhabricatorApplicationTransactionValidationException $ex) {
@@ -304,7 +299,7 @@ final class PhabricatorProjectEditDetailsController
 
     if (!$is_new) {
       $nav = $this->buildIconNavView($project);
-      $nav->selectFilter("edit/{$id}/");
+      $nav->selectFilter("details/{$id}/");
       $nav->appendChild($form_box);
     } else {
       $nav = array($form_box);

src/applications/project/controller/PhabricatorProjectEditIconController.php

@@ -26,7 +26,7 @@ final class PhabricatorProjectEditIconController
       if (!$project) {
         return new Aphront404Response();
       }
-      $cancel_uri = $this->getApplicationURI('edit/'.$project->getID().'/');
+      $cancel_uri = $this->getApplicationURI('profile/'.$project->getID().'/');
       $project_icon = $project->getIcon();
     } else {
       $this->requireApplicationCapability(

src/applications/project/controller/PhabricatorProjectEditMainController.php

@@ -1,160 +0,0 @@
-<?php
-
-final class PhabricatorProjectEditMainController
-  extends PhabricatorProjectController {
-
-  private $id;
-
-  public function shouldAllowPublic() {
-    // This page shows project history and some detailed information, and
-    // it's reasonable to allow public access to it.
-    return true;
-  }
-
-  public function willProcessRequest(array $data) {
-    $this->id = idx($data, 'id');
-  }
-
-  public function processRequest() {
-    $request = $this->getRequest();
-    $viewer = $request->getUser();
-    $id = $request->getURIData('id');
-
-    $project = id(new PhabricatorProjectQuery())
-      ->setViewer($viewer)
-      ->withIDs(array($this->id))
-      ->needImages(true)
-      ->executeOne();
-    if (!$project) {
-      return new Aphront404Response();
-    }
-
-    $header = id(new PHUIHeaderView())
-      ->setHeader(pht('Edit %s', $project->getName()))
-      ->setUser($viewer)
-      ->setPolicyObject($project);
-
-    if ($project->getStatus() == PhabricatorProjectStatus::STATUS_ACTIVE) {
-      $header->setStatus('fa-check', 'bluegrey', pht('Active'));
-    } else {
-      $header->setStatus('fa-ban', 'red', pht('Archived'));
-    }
-
-    $actions = $this->buildActionListView($project);
-    $properties = $this->buildPropertyListView($project, $actions);
-
-    $object_box = id(new PHUIObjectBoxView())
-      ->setHeader($header)
-      ->addPropertyList($properties);
-
-    $timeline = $this->buildTransactionTimeline(
-      $project,
-      new PhabricatorProjectTransactionQuery());
-    $timeline->setShouldTerminate(true);
-
-    $nav = $this->buildIconNavView($project);
-    $nav->selectFilter("edit/{$id}/");
-    $nav->appendChild($object_box);
-    $nav->appendChild($timeline);
-
-    $mnav = $this->buildSideNavView();
-
-    return $this->buildApplicationPage(
-      array(
-        $nav,
-      ),
-      array(
-        'title' => $project->getName(),
-      ));
-  }
-
-  private function buildActionListView(PhabricatorProject $project) {
-    $request = $this->getRequest();
-    $viewer = $request->getUser();
-
-    $id = $project->getID();
-
-    $view = id(new PhabricatorActionListView())
-      ->setUser($viewer)
-      ->setObjectURI($request->getRequestURI());
-
-    $can_edit = PhabricatorPolicyFilter::hasCapability(
-      $viewer,
-      $project,
-      PhabricatorPolicyCapability::CAN_EDIT);
-
-    $view->addAction(
-      id(new PhabricatorActionView())
-        ->setName(pht('Edit Details'))
-        ->setIcon('fa-pencil')
-        ->setHref($this->getApplicationURI("details/{$id}/"))
-        ->setDisabled(!$can_edit)
-        ->setWorkflow(!$can_edit));
-
-    $view->addAction(
-      id(new PhabricatorActionView())
-        ->setName(pht('Edit Picture'))
-        ->setIcon('fa-picture-o')
-        ->setHref($this->getApplicationURI("picture/{$id}/"))
-        ->setDisabled(!$can_edit)
-        ->setWorkflow(!$can_edit));
-
-    if ($project->isArchived()) {
-      $view->addAction(
-        id(new PhabricatorActionView())
-          ->setName(pht('Activate Project'))
-          ->setIcon('fa-check')
-          ->setHref($this->getApplicationURI("archive/{$id}/"))
-          ->setDisabled(!$can_edit)
-          ->setWorkflow(true));
-    } else {
-      $view->addAction(
-        id(new PhabricatorActionView())
-          ->setName(pht('Archive Project'))
-          ->setIcon('fa-ban')
-          ->setHref($this->getApplicationURI("archive/{$id}/"))
-          ->setDisabled(!$can_edit)
-          ->setWorkflow(true));
-    }
-
-    return $view;
-  }
-
-  private function buildPropertyListView(
-    PhabricatorProject $project,
-    PhabricatorActionListView $actions) {
-    $request = $this->getRequest();
-    $viewer = $request->getUser();
-
-    $view = id(new PHUIPropertyListView())
-      ->setUser($viewer)
-      ->setObject($project)
-      ->setActionList($actions);
-
-    $descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions(
-      $viewer,
-      $project);
-
-    $this->loadHandles(array($project->getPHID()));
-
-    $view->addProperty(
-      pht('Looks Like'),
-      $this->getHandle($project->getPHID())->renderTag());
-
-    $view->addProperty(
-      pht('Visible To'),
-      $descriptions[PhabricatorPolicyCapability::CAN_VIEW]);
-
-    $view->addProperty(
-      pht('Editable By'),
-      $descriptions[PhabricatorPolicyCapability::CAN_EDIT]);
-
-    $view->addProperty(
-      pht('Joinable By'),
-      $descriptions[PhabricatorPolicyCapability::CAN_JOIN]);
-
-    return $view;
-  }
-
-
-}

src/applications/project/controller/PhabricatorProjectEditPictureController.php

@@ -28,7 +28,7 @@ final class PhabricatorProjectEditPictureController
       return new Aphront404Response();
     }
 
-    $edit_uri = $this->getApplicationURI('edit/'.$project->getID().'/');
+    $edit_uri = $this->getApplicationURI('profile/'.$project->getID().'/');
     $view_uri = $this->getApplicationURI('profile/'.$project->getID().'/');
 
     $supported_formats = PhabricatorFile::getTransformableImageFormats();

src/applications/project/controller/PhabricatorProjectProfileController.php

@@ -33,26 +33,12 @@ final class PhabricatorProjectProfileController
     }
 
     $picture = $project->getProfileImageURI();
-    require_celerity_resource('phabricator-profile-css');
-    $tasks = $this->renderTasksPage($project);
-    $content = phutil_tag_div('phabricator-project-layout', $tasks);
-
-    $phid = $project->getPHID();
-    $create_uri = '/maniphest/task/create/?projects='.$phid;
-    $icon_new = id(new PHUIIconView())
-      ->setIconFont('fa-plus');
-    $button_add = id(new PHUIButtonView())
-      ->setTag('a')
-      ->setText(pht('New Task'))
-      ->setHref($create_uri)
-      ->setIcon($icon_new);
 
     $header = id(new PHUIHeaderView())
       ->setHeader($project->getName())
       ->setUser($user)
       ->setPolicyObject($project)
-      ->setImage($picture)
-      ->addActionLink($button_add);
+      ->setImage($picture);
 
     if ($project->getStatus() == PhabricatorProjectStatus::STATUS_ACTIVE) {
       $header->setStatus('fa-check', 'bluegrey', pht('Active'));
@@ -67,10 +53,15 @@ final class PhabricatorProjectProfileController
       ->setHeader($header)
       ->addPropertyList($properties);
 
+    $timeline = $this->buildTransactionTimeline(
+      $project,
+      new PhabricatorProjectTransactionQuery());
+    $timeline->setShouldTerminate(true);
+
     $nav = $this->buildIconNavView($project);
     $nav->selectFilter("profile/{$id}/");
     $nav->appendChild($object_box);
-    $nav->appendChild($content);
+    $nav->appendChild($timeline);
 
     return $this->buildApplicationPage(
       array(
@@ -81,66 +72,6 @@ final class PhabricatorProjectProfileController
       ));
   }
 
-  private function renderTasksPage(PhabricatorProject $project) {
-
-    $user = $this->getRequest()->getUser();
-    $limit = 50;
-
-    $query = id(new ManiphestTaskQuery())
-      ->setViewer($user)
-      ->withAnyProjects(array($project->getPHID()))
-      ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants())
-      ->setOrderBy(ManiphestTaskQuery::ORDER_PRIORITY)
-      ->needProjectPHIDs(true)
-      ->setLimit(($limit + 1));
-    $tasks = $query->execute();
-    $count = count($tasks);
-    if ($count == ($limit + 1)) {
-      array_pop($tasks);
-    }
-
-    $phids = mpull($tasks, 'getOwnerPHID');
-    $phids = array_merge(
-      $phids,
-      array_mergev(mpull($tasks, 'getProjectPHIDs')));
-    $phids = array_filter($phids);
-    $handles = $this->loadViewerHandles($phids);
-
-    $task_list = new ManiphestTaskListView();
-    $task_list->setUser($user);
-    $task_list->setTasks($tasks);
-    $task_list->setHandles($handles);
-    $task_list->setNoDataString(pht('This project has no open tasks.'));
-
-    $phid = $project->getPHID();
-    $view_uri = urisprintf(
-      '/maniphest/?statuses=%s&allProjects=%s#R',
-      implode(',', ManiphestTaskStatus::getOpenStatusConstants()),
-      $phid);
-    $icon = id(new PHUIIconView())
-      ->setIconFont('fa-search');
-    $button_view = id(new PHUIButtonView())
-      ->setTag('a')
-      ->setText(pht('View Query'))
-      ->setHref($view_uri)
-      ->setIcon($icon);
-
-      $header = id(new PHUIHeaderView())
-        ->addActionLink($button_view);
-
-    if ($count > $limit) {
-        $header->setHeader(pht('Highest Priority (some)'));
-    } else {
-        $header->setHeader(pht('Highest Priority (all)'));
-    }
-
-    $content = id(new PHUIObjectBoxView())
-      ->setHeader($header)
-      ->appendChild($task_list);
-
-    return $content;
-  }
-
   private function buildActionListView(PhabricatorProject $project) {
     $request = $this->getRequest();
     $viewer = $request->getUser();
@@ -159,10 +90,35 @@ final class PhabricatorProjectProfileController
 
     $view->addAction(
       id(new PhabricatorActionView())
-        ->setName(pht('Edit Project'))
+        ->setName(pht('Edit Details'))
         ->setIcon('fa-pencil')
-        ->setHref($this->getApplicationURI("edit/{$id}/")));
+        ->setHref($this->getApplicationURI("details/{$id}/")));
 
+    $view->addAction(
+      id(new PhabricatorActionView())
+        ->setName(pht('Edit Picture'))
+        ->setIcon('fa-picture-o')
+        ->setHref($this->getApplicationURI("picture/{$id}/"))
+        ->setDisabled(!$can_edit)
+        ->setWorkflow(!$can_edit));
+
+    if ($project->isArchived()) {
+      $view->addAction(
+        id(new PhabricatorActionView())
+          ->setName(pht('Activate Project'))
+          ->setIcon('fa-check')
+          ->setHref($this->getApplicationURI("archive/{$id}/"))
+          ->setDisabled(!$can_edit)
+          ->setWorkflow(true));
+    } else {
+      $view->addAction(
+        id(new PhabricatorActionView())
+          ->setName(pht('Archive Project'))
+          ->setIcon('fa-ban')
+          ->setHref($this->getApplicationURI("archive/{$id}/"))
+          ->setDisabled(!$can_edit)
+          ->setWorkflow(true));
+    }
 
     $action = null;
     if (!$project->isUserMember($viewer->getPHID())) {
@@ -244,6 +200,20 @@ final class PhabricatorProjectProfileController
         ? $this->renderHandlesForPHIDs($project->getWatcherPHIDs(), ',')
         : phutil_tag('em', array(), pht('None')));
 
+    $descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions(
+      $viewer,
+      $project);
+
+    $this->loadHandles(array($project->getPHID()));
+
+    $view->addProperty(
+      pht('Looks Like'),
+      $this->getHandle($project->getPHID())->renderTag());
+
+    $view->addProperty(
+      pht('Joinable By'),
+      $descriptions[PhabricatorPolicyCapability::CAN_JOIN]);
+
     $field_list = PhabricatorCustomField::getObjectFields(
       $project,
       PhabricatorCustomField::ROLE_VIEW);

src/applications/project/storage/PhabricatorProjectTransaction.php

@@ -42,6 +42,52 @@ final class PhabricatorProjectTransaction
     return array_merge($req_phids, parent::getRequiredHandlePHIDs());
   }
 
+  public function getColor() {
+
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    switch ($this->getTransactionType()) {
+      case PhabricatorProjectTransaction::TYPE_STATUS:
+        if ($old == 0) {
+          return 'red';
+        } else {
+          return 'green';
+        }
+      }
+    return parent::getColor();
+  }
+
+  public function getIcon() {
+
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    switch ($this->getTransactionType()) {
+      case PhabricatorProjectTransaction::TYPE_STATUS:
+        if ($old == 0) {
+          return 'fa-ban';
+        } else {
+          return 'fa-check';
+        }
+      case PhabricatorProjectTransaction::TYPE_LOCKED:
+        if ($new) {
+          return 'fa-lock';
+        } else {
+          return 'fa-unlock';
+        }
+      case PhabricatorProjectTransaction::TYPE_ICON:
+        return $new;
+      case PhabricatorProjectTransaction::TYPE_IMAGE:
+        return 'fa-photo';
+      case PhabricatorProjectTransaction::TYPE_MEMBERS:
+        return 'fa-user';
+      case PhabricatorProjectTransaction::TYPE_SLUGS:
+        return 'fa-tag';
+    }
+    return parent::getIcon();
+  }
+
   public function getTitle() {
     $old = $this->getOldValue();
     $new = $this->getNewValue();
@@ -63,11 +109,11 @@ final class PhabricatorProjectTransaction
       case PhabricatorProjectTransaction::TYPE_STATUS:
         if ($old == 0) {
           return pht(
-            '%s closed this project.',
+            '%s archived this project.',
             $author_handle);
         } else {
           return pht(
-            '%s reopened this project.',
+            '%s activated this project.',
             $author_handle);
         }
       case PhabricatorProjectTransaction::TYPE_IMAGE:

src/view/layout/AphrontSideNavFilterView.php

@@ -100,10 +100,14 @@ final class AphrontSideNavFilterView extends AphrontView {
       $key, $name, $uri, PHUIListItemView::TYPE_LINK);
   }
 
-  public function addIcon($key, $name, $icon, $image = null) {
-    $href = clone $this->baseURI;
-    $href->setPath(rtrim($href->getPath().$key, '/').'/');
-    $href = (string)$href;
+  public function addIcon($key, $name, $icon, $image = null, $uri = null) {
+    if (!$uri) {
+      $href = clone $this->baseURI;
+      $href->setPath(rtrim($href->getPath().$key, '/').'/');
+      $href = (string)$href;
+    } else {
+      $href = $uri;
+    }
 
     $item = id(new PHUIListItemView())
       ->setKey($key)