Move Releeph branch controllers toward a modern/stable state

Summary:
Ref T3644. Ref T3657. Ref T3549. Basically:

  - Move these controllers to modern query/policy infrastructure.
  - Move them to consistent, ID-based URIs.
  - Rename "Project" to "Product"; "Pick Request" to "Pull Request".
  - Clean up a few UI things here and there.

Test Plan:
  - Created and edited branches.
  - Opened and closed branches.
  - Viewed branch history.
  - Searched within a branch.
  - Browsed to branches from products.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3644, T3549, T3657

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

src/__phutil_library_map__.php

@@ -2497,6 +2497,7 @@ phutil_register_library_map(array(
     'ReleephBranch' => 'applications/releeph/storage/ReleephBranch.php',
     'ReleephBranchAccessController' => 'applications/releeph/controller/branch/ReleephBranchAccessController.php',
     'ReleephBranchCommitFieldSpecification' => 'applications/releeph/field/specification/ReleephBranchCommitFieldSpecification.php',
+    'ReleephBranchController' => 'applications/releeph/controller/branch/ReleephBranchController.php',
     'ReleephBranchCreateController' => 'applications/releeph/controller/branch/ReleephBranchCreateController.php',
     'ReleephBranchEditController' => 'applications/releeph/controller/branch/ReleephBranchEditController.php',
     'ReleephBranchEditor' => 'applications/releeph/editor/ReleephBranchEditor.php',
@@ -5479,12 +5480,13 @@ phutil_register_library_map(array(
       0 => 'ReleephDAO',
       1 => 'PhabricatorPolicyInterface',
     ),
-    'ReleephBranchAccessController' => 'ReleephProjectController',
+    'ReleephBranchAccessController' => 'ReleephBranchController',
     'ReleephBranchCommitFieldSpecification' => 'ReleephFieldSpecification',
-    'ReleephBranchCreateController' => 'ReleephProjectController',
+    'ReleephBranchController' => 'ReleephController',
-    'ReleephBranchEditController' => 'ReleephProjectController',
+    'ReleephBranchCreateController' => 'ReleephProductController',
+    'ReleephBranchEditController' => 'ReleephBranchController',
     'ReleephBranchEditor' => 'PhabricatorEditor',
-    'ReleephBranchHistoryController' => 'ReleephProjectController',
+    'ReleephBranchHistoryController' => 'ReleephBranchController',
     'ReleephBranchNamePreviewController' => 'ReleephController',
     'ReleephBranchPreviewView' => 'AphrontFormControl',
     'ReleephBranchQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
@@ -5493,7 +5495,7 @@ phutil_register_library_map(array(
     'ReleephBranchTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
     'ReleephBranchViewController' =>
     array(
-      0 => 'ReleephProjectController',
+      0 => 'ReleephBranchController',
       1 => 'PhabricatorApplicationSearchResultsControllerInterface',
     ),
     'ReleephCommitFinderException' => 'Exception',

src/applications/releeph/controller/ReleephController.php

@@ -38,4 +38,13 @@ abstract class ReleephController extends PhabricatorController {
     return $this->buildSideNavView(true)->getMenu();
   }
 
+
+  protected function getProductViewURI(ReleephProject $product) {
+    return $this->getApplicationURI('project/'.$product->getID().'/');
+  }
+
+  protected function getBranchViewURI(ReleephBranch $branch) {
+    return $this->getApplicationURI('branch/'.$branch->getID().'/');
+  }
+
 }

src/applications/releeph/controller/branch/ReleephBranchAccessController.php

@@ -1,59 +1,81 @@
 <?php
 
-final class ReleephBranchAccessController extends ReleephProjectController {
+final class ReleephBranchAccessController extends ReleephBranchController {
 
   private $action;
+  private $branchID;
 
   public function willProcessRequest(array $data) {
     $this->action = $data['action'];
-    parent::willProcessRequest($data);
+    $this->branchID = $data['branchID'];
   }
 
   public function processRequest() {
-    $branch = $this->getReleephBranch();
     $request = $this->getRequest();
+    $viewer = $request->getUser();
 
-    $done_uri = $branch->getURI();
+    $branch = id(new ReleephBranchQuery())
+      ->setViewer($viewer)
+      ->withIDs(array($this->branchID))
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->executeOne();
+    if (!$branch) {
+      return new Aphront404Response();
+    }
+    $this->setBranch($branch);
 
-    switch ($this->action) {
+    $action = $this->action;
+    switch ($action) {
       case 'close':
-        $is_active = false;
-        $title_text = pht('Close Branch');
-        $body_text = pht(
-          'Really close the branch "%s"?',
-          $branch->getBasename());
-        $button_text = pht('Close Branch');
-        break;
       case 're-open':
-        $is_active = true;
-        $title_text = pht('Reopen Branch');
-        $body_text = pht(
-          'Really reopen the branch "%s"?',
-          $branch->getBasename());
-        $button_text = pht('Reopen Branch');
         break;
       default:
-        throw new Exception("Unknown action '{$this->action}'!");
+        return new Aphront404Response();
-        break;
     }
 
-    if ($request->isDialogFormPost()) {
+    $branch_uri = $this->getBranchViewURI($branch);
+    if ($request->isFormPost()) {
+
+      if ($action == 're-open') {
+        $is_active = 1;
+      } else {
+        $is_active = 0;
+      }
+
       id(new ReleephBranchEditor())
         ->setActor($request->getUser())
         ->setReleephBranch($branch)
-        ->changeBranchAccess($is_active ? 1 : 0);
+        ->changeBranchAccess($is_active);
 
-      return id(new AphrontReloadResponse())->setURI($done_uri);
+      return id(new AphrontReloadResponse())->setURI($branch_uri);
     }
 
-    $dialog = new AphrontDialogView();
+    if ($action == 'close') {
-    $dialog
+      $title_text = pht('Really Close Branch?');
-      ->setUser($request->getUser())
+      $short = pht('Close Branch');
+      $body_text = pht(
+        'Really close the branch "%s"?',
+        phutil_tag('strong', array(), $branch->getBasename()));
+      $button_text = pht('Close Branch');
+    } else {
+      $title_text = pht('Really Reopen Branch?');
+      $short = pht('Reopen Branch');
+      $body_text = pht(
+        'Really reopen the branch "%s"?',
+        phutil_tag('strong', array(), $branch->getBasename()));
+      $button_text = pht('Reopen Branch');
+    }
+
+    return $this->newDialog()
       ->setTitle($title_text)
+      ->setShortTitle($short)
       ->appendChild($body_text)
       ->addSubmitButton($button_text)
-      ->addCancelButton($done_uri);
+      ->addCancelButton($branch_uri);
-
-    return id(new AphrontDialogResponse())->setDialog($dialog);
   }
+
 }

src/applications/releeph/controller/branch/ReleephBranchController.php

@@ -0,0 +1,35 @@
+<?php
+
+abstract class ReleephBranchController extends ReleephController {
+
+  private $branch;
+
+  public function setBranch($branch) {
+    $this->branch = $branch;
+    return $this;
+  }
+
+  public function getBranch() {
+    return $this->branch;
+  }
+
+  protected function buildApplicationCrumbs() {
+    $crumbs = parent::buildApplicationCrumbs();
+
+    $branch = $this->getBranch();
+    if ($branch) {
+      $product = $branch->getProduct();
+
+      $crumbs->addTextCrumb(
+        $product->getName(),
+        $this->getProductViewURI($product));
+
+      $crumbs->addTextCrumb(
+        $branch->getName(),
+        $this->getBranchViewURI($branch));
+    }
+
+    return $crumbs;
+  }
+
+}

src/applications/releeph/controller/branch/ReleephBranchCreateController.php

@@ -1,23 +1,43 @@
 <?php
 
-final class ReleephBranchCreateController extends ReleephProjectController {
+final class ReleephBranchCreateController extends ReleephProductController {
+
+  private $productID;
+
+  public function willProcessRequest(array $data) {
+    $this->productID = $data['projectID'];
+  }
 
   public function processRequest() {
-    $releeph_project = $this->getReleephProject();
-
     $request = $this->getRequest();
+    $viewer = $request->getUser();
+
+    $product = id(new ReleephProjectQuery())
+      ->setViewer($viewer)
+      ->withIDs(array($this->productID))
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->executeOne();
+    if (!$product) {
+      return new Aphront404Response();
+    }
+    $this->setProduct($product);
+
 
     $cut_point = $request->getStr('cutPoint');
     $symbolic_name = $request->getStr('symbolicName');
 
     if (!$cut_point) {
-      $repository = $releeph_project->loadPhabricatorRepository();
+      $repository = $product->loadPhabricatorRepository();
       switch ($repository->getVersionControlSystem()) {
         case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
           break;
 
         case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
-          $cut_point = $releeph_project->getTrunkBranch();
+          $cut_point = $product->getTrunkBranch();
           break;
       }
     }
@@ -42,7 +62,7 @@ final class ReleephBranchCreateController extends ReleephProjectController {
         try {
           $finder = id(new ReleephCommitFinder())
             ->setUser($request->getUser())
-            ->setReleephProject($releeph_project);
+            ->setReleephProject($product);
           $cut_commit = $finder->fromPartial($cut_point);
         } catch (Exception $e) {
           $e_cut = pht('Invalid');
@@ -52,7 +72,7 @@ final class ReleephBranchCreateController extends ReleephProjectController {
 
       if (!$errors) {
         $branch = id(new ReleephBranchEditor())
-          ->setReleephProject($releeph_project)
+          ->setReleephProject($product)
           ->setActor($request->getUser())
           ->newBranchFromCommit(
             $cut_commit,
@@ -64,14 +84,7 @@ final class ReleephBranchCreateController extends ReleephProjectController {
       }
     }
 
-    $error_view = array();
+    $product_uri = $this->getProductViewURI($product);
-    if ($errors) {
-      $error_view = new AphrontErrorView();
-      $error_view->setErrors($errors);
-    }
-
-    $project_id = $releeph_project->getID();
-    $project_uri = $this->getApplicationURI("project/{$project_id}/");
 
     $form = id(new AphrontFormView())
       ->setUser($request->getUser())
@@ -95,7 +108,12 @@ final class ReleephBranchCreateController extends ReleephProjectController {
       ->appendChild(
         id(new AphrontFormSubmitControl())
           ->setValue(pht('Cut Branch'))
-          ->addCancelButton($project_uri));
+          ->addCancelButton($product_uri));
+
+    $box = id(new PHUIObjectBoxView())
+      ->setHeaderText(pht('New Branch'))
+      ->setFormErrors($errors)
+      ->appendChild($form);
 
     $crumbs = $this->buildApplicationCrumbs();
     $crumbs->addTextCrumb(pht('New Branch'));
@@ -103,8 +121,7 @@ final class ReleephBranchCreateController extends ReleephProjectController {
     return $this->buildApplicationPage(
       array(
         $crumbs,
-        $error_view,
+        $box,
-        $form,
       ),
       array(
         'title' => pht('New Branch'),

src/applications/releeph/controller/branch/ReleephBranchEditController.php

@@ -1,27 +1,46 @@
 <?php
 
-final class ReleephBranchEditController extends ReleephProjectController {
+final class ReleephBranchEditController extends ReleephBranchController {
+
+  private $branchID;
+
+  public function willProcessRequest(array $data) {
+    $this->branchID = $data['branchID'];
+  }
 
   public function processRequest() {
     $request = $this->getRequest();
-    $releeph_branch = $this->getReleephBranch();
+    $viewer = $request->getUser();
+
+    $branch = id(new ReleephBranchQuery())
+      ->setViewer($viewer)
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->withIDs(array($this->branchID))
+      ->executeOne();
+    if (!$branch) {
+      return new Aphront404Response();
+    }
+    $this->setBranch($branch);
+
     $symbolic_name = $request->getStr(
       'symbolicName',
-      $releeph_branch->getSymbolicName());
+      $branch->getSymbolicName());
-
-    $errors = array();
 
     if ($request->isFormPost()) {
       $existing_with_same_symbolic_name =
         id(new ReleephBranch())
           ->loadOneWhere(
               'id != %d AND releephProjectID = %d AND symbolicName = %s',
-              $releeph_branch->getID(),
+              $branch->getID(),
-              $releeph_branch->getReleephProjectID(),
+              $branch->getReleephProjectID(),
               $symbolic_name);
 
-      $releeph_branch->openTransaction();
+      $branch->openTransaction();
-      $releeph_branch
+      $branch
         ->setSymbolicName($symbolic_name);
 
       if ($existing_with_same_symbolic_name) {
@@ -30,17 +49,17 @@ final class ReleephBranchEditController extends ReleephProjectController {
           ->save();
       }
 
-      $releeph_branch->save();
+      $branch->save();
-      $releeph_branch->saveTransaction();
+      $branch->saveTransaction();
 
       return id(new AphrontRedirectResponse())
-        ->setURI($releeph_branch->getURI());
+        ->setURI($this->getBranchViewURI($branch));
     }
 
     $phids = array();
 
-    $phids[] = $creator_phid = $releeph_branch->getCreatedByUserPHID();
+    $phids[] = $creator_phid = $branch->getCreatedByUserPHID();
-    $phids[] = $cut_commit_phid = $releeph_branch->getCutPointCommitPHID();
+    $phids[] = $cut_commit_phid = $branch->getCutPointCommitPHID();
 
     $handles = id(new PhabricatorHandleQuery())
       ->setViewer($request->getUser())
@@ -52,7 +71,7 @@ final class ReleephBranchEditController extends ReleephProjectController {
       ->appendChild(
         id(new AphrontFormStaticControl())
         ->setLabel(pht('Branch Name'))
-        ->setValue($releeph_branch->getName()))
+        ->setValue($branch->getName()))
       ->appendChild(
         id(new AphrontFormMarkupControl())
           ->setLabel(pht('Cut Point'))
@@ -70,29 +89,24 @@ final class ReleephBranchEditController extends ReleephProjectController {
               '(e.g. "LATEST")')))
       ->appendChild(
         id(new AphrontFormSubmitControl())
-          ->addCancelButton($releeph_branch->getURI())
+          ->addCancelButton($this->getBranchViewURI($branch))
-          ->setValue(pht('Save')));
+          ->setValue(pht('Save Branch')));
-
-    $error_view = null;
-    if ($errors) {
-      $error_view = id(new AphrontErrorView())
-        ->setSeverity(AphrontErrorView::SEVERITY_ERROR)
-        ->setErrors($errors)
-        ->setTitle(pht('Errors'));
-    }
 
     $title = pht(
       'Edit Branch %s',
-      $releeph_branch->getDisplayNameWithDetail());
+      $branch->getDisplayNameWithDetail());
 
     $crumbs = $this->buildApplicationCrumbs();
     $crumbs->addTextCrumb(pht('Edit'));
 
+    $box = id(new PHUIObjectBoxView())
+      ->setHeaderText($title)
+      ->appendChild($form);
+
     return $this->buildApplicationPage(
       array(
         $crumbs,
-        $error_view,
+        $box,
-        $form,
       ),
       array(
         'title' => $title,

src/applications/releeph/controller/branch/ReleephBranchHistoryController.php

@@ -1,12 +1,15 @@
 <?php
 
-final class ReleephBranchHistoryController extends ReleephProjectController {
+final class ReleephBranchHistoryController extends ReleephBranchController {
 
-  private $id;
+  private $branchID;
+
+  public function shouldAllowPublic() {
+    return true;
+  }
 
   public function willProcessRequest(array $data) {
-    $this->id = $data['branchID'];
+    $this->branchID = $data['branchID'];
-    parent::willProcessRequest($data);
   }
 
   public function processRequest() {
@@ -15,11 +18,12 @@ final class ReleephBranchHistoryController extends ReleephProjectController {
 
     $branch = id(new ReleephBranchQuery())
       ->setViewer($viewer)
-      ->withIDs(array($this->id))
+      ->withIDs(array($this->branchID))
       ->executeOne();
     if (!$branch) {
       return new Aphront404Response();
     }
+    $this->setBranch($branch);
 
     $xactions = id(new ReleephBranchTransactionQuery())
       ->setViewer($viewer)
@@ -29,7 +33,8 @@ final class ReleephBranchHistoryController extends ReleephProjectController {
     $timeline = id(new PhabricatorApplicationTransactionView())
       ->setUser($viewer)
       ->setObjectPHID($branch->getPHID())
-      ->setTransactions($xactions);
+      ->setTransactions($xactions)
+      ->setShouldTerminate(true);
 
     $crumbs = $this->buildApplicationCrumbs();
     $crumbs->addTextCrumb(pht('History'));

src/applications/releeph/controller/branch/ReleephBranchViewController.php

@@ -1,22 +1,32 @@
 <?php
 
-final class ReleephBranchViewController extends ReleephProjectController
+final class ReleephBranchViewController extends ReleephBranchController
   implements PhabricatorApplicationSearchResultsControllerInterface {
 
   private $queryKey;
+  private $branchID;
 
   public function shouldAllowPublic() {
     return true;
   }
 
   public function willProcessRequest(array $data) {
-    parent::willProcessRequest($data);
+    $this->branchID = $data['branchID'];
     $this->queryKey = idx($data, 'queryKey');
   }
 
-
   public function processRequest() {
     $request = $this->getRequest();
+    $viewer = $request->getUser();
+
+    $branch = id(new ReleephBranchQuery())
+      ->setViewer($viewer)
+      ->withIDs(array($this->branchID))
+      ->executeOne();
+    if (!$branch) {
+      return new Aphront404Response();
+    }
+    $this->setBranch($branch);
 
     $controller = id(new PhabricatorApplicationSearchController($request))
       ->setPreface($this->renderPreface())
@@ -34,11 +44,10 @@ final class ReleephBranchViewController extends ReleephProjectController
     assert_instances_of($requests, 'ReleephRequest');
     $viewer = $this->getRequest()->getUser();
 
-    $releeph_branch = $this->getReleephBranch();
+    $branch = $this->getBranch();
-    $releeph_project = $this->getReleephProject();
 
-    // TODO: Really gross.
+    // TODO: Really really gross.
-    $releeph_branch->populateReleephRequestHandles(
+    $branch->populateReleephRequestHandles(
       $viewer,
       $requests);
 
@@ -46,8 +55,8 @@ final class ReleephBranchViewController extends ReleephProjectController
       ->setOriginType('branch')
       ->setUser($viewer)
       ->setAphrontRequest($this->getRequest())
-      ->setReleephProject($releeph_project)
+      ->setReleephProject($branch->getProduct())
-      ->setReleephBranch($releeph_branch)
+      ->setReleephBranch($branch)
       ->setReleephRequests($requests);
 
     return $list;
@@ -59,7 +68,6 @@ final class ReleephBranchViewController extends ReleephProjectController
     $nav = new AphrontSideNavFilterView();
     $nav->setBaseURI(new PhutilURI($this->getApplicationURI()));
 
-
     $this->getSearchEngine()->addNavigationItems($nav->getMenu());
 
     $nav->selectFilter(null);
@@ -68,45 +76,43 @@ final class ReleephBranchViewController extends ReleephProjectController
   }
 
   private function getSearchEngine() {
-    $branch = $this->getReleephBranch();
+    $branch = $this->getBranch();
     return id(new ReleephRequestSearchEngine())
       ->setBranch($branch)
-      ->setBaseURI($branch->getURI())
+      ->setBaseURI($this->getApplicationURI('branch/'.$branch->getID().'/'))
       ->setViewer($this->getRequest()->getUser());
   }
 
   public function buildApplicationCrumbs() {
-    $releeph_branch = $this->getReleephBranch();
-
     $crumbs = parent::buildApplicationCrumbs();
 
-    if ($releeph_branch->isActive()) {
+    $branch = $this->getBranch();
-      $create_uri = $releeph_branch->getURI('request/');
+    $create_uri = $branch->getURI('request/');
-      $crumbs->addAction(
+    $crumbs->addAction(
-        id(new PHUIListItemView())
+      id(new PHUIListItemView())
-          ->setHref($create_uri)
+        ->setHref($create_uri)
-          ->setName(pht('Request Pick'))
+        ->setName(pht('New Pull Request'))
-          ->setIcon('create'));
+        ->setIcon('create')
-    }
+        ->setDisabled(!$branch->isActive()));
 
     return $crumbs;
   }
 
   private function renderPreface() {
-    $branch = $this->getReleephBranch();
     $viewer = $this->getRequest()->getUser();
 
+    $branch = $this->getBranch();
     $id = $branch->getID();
 
     $header = id(new PHUIHeaderView())
-      ->setHeader($branch->getDisplayName());
+      ->setHeader($branch->getDisplayName())
+      ->setUser($viewer)
+      ->setPolicyObject($branch);
 
-    if (!$branch->getIsActive()) {
+    if ($branch->getIsActive()) {
-      $header->addTag(
+      $header->setStatus('oh-ok', '', pht('Active'));
-        id(new PHUITagView())
+    } else {
-          ->setType(PHUITagView::TYPE_STATE)
+      $header->setStatus('policy-noone', '', pht('Closed'));
-          ->setBackgroundColor(PHUITagView::COLOR_BLACK)
-          ->setName(pht('Closed')));
     }
 
     $actions = id(new PhabricatorActionListView())
@@ -119,11 +125,9 @@ final class ReleephBranchViewController extends ReleephProjectController
       $branch,
       PhabricatorPolicyCapability::CAN_EDIT);
 
-    $edit_uri = $branch->getURI('edit/');
+    $edit_uri = $this->getApplicationURI("branch/edit/{$id}/");
-    $close_uri = $branch->getURI('close/');
+    $close_uri = $this->getApplicationURI("branch/close/{$id}/");
-    $reopen_uri = $branch->getURI('re-open/');
+    $reopen_uri = $this->getApplicationURI("branch/re-open/{$id}/");
-
-    $id = $branch->getID();
     $history_uri = $this->getApplicationURI("branch/{$id}/history/");
 
     $actions->addAction(
@@ -149,7 +153,6 @@ final class ReleephBranchViewController extends ReleephProjectController
           ->setHref($reopen_uri)
           ->setIcon('new')
           ->setUser($viewer)
-          ->setRenderAsForm(true)
           ->setDisabled(!$can_edit)
           ->setWorkflow(true));
     }

src/applications/releeph/controller/project/ReleephProductController.php

@@ -13,10 +13,6 @@ abstract class ReleephProductController extends ReleephController {
     return $this->product;
   }
 
-  protected function getProductViewURI(ReleephProject $product) {
-    return $this->getApplicationURI('project/'.$product->getID().'/');
-  }
-
   protected function buildApplicationCrumbs() {
     $crumbs = parent::buildApplicationCrumbs();
 

src/applications/releeph/controller/project/ReleephProductHistoryController.php

@@ -4,6 +4,10 @@ final class ReleephProductHistoryController extends ReleephProductController {
 
   private $id;
 
+  public function shouldAllowPublic() {
+    return true;
+  }
+
   public function willProcessRequest(array $data) {
     $this->id = $data['projectID'];
   }

src/applications/releeph/controller/project/ReleephProductViewController.php

@@ -99,7 +99,7 @@ final class ReleephProductViewController extends ReleephProductController
 
       $item = id(new PHUIObjectItemView())
         ->setHeader($branch->getDisplayName())
-        ->setHref($branch->getURI())
+        ->setHref($this->getApplicationURI('branch/'.$branch->getID().'/'))
         ->addAttribute($branch_link);
 
       if (!$branch->getIsActive()) {

src/applications/releeph/storage/ReleephBranch.php

@@ -162,6 +162,10 @@ final class ReleephBranch extends ReleephDAO
     return $this->assertAttached($this->project);
   }
 
+  public function getProduct() {
+    return $this->getProject();
+  }
+
   public function attachCutPointCommit(
     PhabricatorRepositoryCommit $commit = null) {
     $this->cutPointCommit = $commit;