Give Harbormaster build steps a "View" page

Summary: Fixes T9519. Right now, build steps go straight from the build to the edit screen. This means that there's no way to see their edit history or review details without edit permission. In particular, this makes it a bit harder to catch the Drydock Blueprint authorization warnings from T9519. - Add a standard view screen. - Add a little warning callout to blueprint authorizations. This also does a bit of a touchup on the weird dropshadow element from T9586. Maybe not totally design-approved now but it's less ugly, at least. Test Plan: {F906695} {F906696} Reviewers: chad Reviewed By: chad Maniphest Tasks: T9519 Differential Revision: https://secure.phabricator.com/D14330
2024-11-10 00:42:41 +01:00 · 2015-10-26 12:38:32 -07:00 · 2015-10-26 12:38:32 -07:00 · 3f193cb9e0
commit 3f193cb9e0
parent 5ee4a1a306
10 changed files with 175 additions and 45 deletions
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@ -71,7 +71,7 @@ return array(
    'rsrc/css/application/feed/feed.css' => 'ecd4ec57',
    'rsrc/css/application/files/global-drag-and-drop.css' => '697324ad',
    'rsrc/css/application/flag/flag.css' => '5337623f',
-    'rsrc/css/application/harbormaster/harbormaster.css' => '49d64eb4',
+    'rsrc/css/application/harbormaster/harbormaster.css' => 'b0758ca5',
    'rsrc/css/application/herald/herald-test.css' => 'a52e323e',
    'rsrc/css/application/herald/herald.css' => '826075fa',
    'rsrc/css/application/maniphest/batch-editor.css' => 'b0f0b6d5',
@ -528,7 +528,7 @@ return array(
    'font-lato' => '5ab1a46a',
    'font-roboto-slab' => 'f24a53cb',
    'global-drag-and-drop-css' => '697324ad',
-    'harbormaster-css' => '49d64eb4',
+    'harbormaster-css' => 'b0758ca5',
    'herald-css' => '826075fa',
    'herald-rule-editor' => '91a6031b',
    'herald-test-css' => 'a52e323e',
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@ -1078,6 +1078,7 @@ phutil_register_library_map(array(
    'HarbormasterStepAddController' => 'applications/harbormaster/controller/HarbormasterStepAddController.php',
    'HarbormasterStepDeleteController' => 'applications/harbormaster/controller/HarbormasterStepDeleteController.php',
    'HarbormasterStepEditController' => 'applications/harbormaster/controller/HarbormasterStepEditController.php',
+    'HarbormasterStepViewController' => 'applications/harbormaster/controller/HarbormasterStepViewController.php',
    'HarbormasterTargetEngine' => 'applications/harbormaster/engine/HarbormasterTargetEngine.php',
    'HarbormasterTargetWorker' => 'applications/harbormaster/worker/HarbormasterTargetWorker.php',
    'HarbormasterTestBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterTestBuildStepGroup.php',
@ -4919,6 +4920,7 @@ phutil_register_library_map(array(
    'HarbormasterStepAddController' => 'HarbormasterController',
    'HarbormasterStepDeleteController' => 'HarbormasterController',
    'HarbormasterStepEditController' => 'HarbormasterController',
+    'HarbormasterStepViewController' => 'HarbormasterController',
    'HarbormasterTargetEngine' => 'Phobject',
    'HarbormasterTargetWorker' => 'HarbormasterWorker',
    'HarbormasterTestBuildStepGroup' => 'HarbormasterBuildStepGroup',
--- a/src/applications/drydock/storage/DrydockAuthorization.php
+++ b/src/applications/drydock/storage/DrydockAuthorization.php
@ -66,7 +66,7 @@ final class DrydockAuthorization extends DrydockDAO

  public static function getBlueprintStateIcon($state) {
    $map = array(
-      self::BLUEPRINTAUTH_REQUESTED => 'fa-exclamation-circle indigo',
+      self::BLUEPRINTAUTH_REQUESTED => 'fa-exclamation-circle pink',
      self::BLUEPRINTAUTH_AUTHORIZED => 'fa-check-circle green',
      self::BLUEPRINTAUTH_DECLINED => 'fa-times red',
    );
--- a/src/applications/drydock/view/DrydockObjectAuthorizationView.php
+++ b/src/applications/drydock/view/DrydockObjectAuthorizationView.php
@ -47,6 +47,7 @@ final class DrydockObjectAuthorizationView extends AphrontView {
      $authorizations = array();
    }

+    $warnings = array();
    $items = array();
    foreach ($blueprint_phids as $phid) {
      $authorization = idx($authorizations, $phid);
@ -65,10 +66,28 @@ final class DrydockObjectAuthorizationView extends AphrontView {
        null,
        DrydockAuthorization::getBlueprintStateName($state));

+      switch ($state) {
+        case DrydockAuthorization::BLUEPRINTAUTH_REQUESTED:
+        case DrydockAuthorization::BLUEPRINTAUTH_DECLINED:
+          $warnings[] = $authorization;
+          break;
+      }
+
      $items[] = $item;
    }

    $status = new PHUIStatusListView();
+
+    if ($warnings) {
+      $status->addItem(
+        id(new PHUIStatusItemView())
+          ->setIcon('fa-exclamation-triangle', 'pink')
+          ->setTarget(
+            pht(
+              'WARNING: There are %s unapproved authorization(s)!',
+              new PhutilNumber(count($warnings)))));
+    }
+
    foreach ($items as $item) {
      $status->addItem($item);
    }
--- a/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php
+++ b/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php
@ -61,6 +61,7 @@ final class PhabricatorHarbormasterApplication extends PhabricatorApplication {
          'add/(?:(?P<id>\d+)/)?' => 'HarbormasterStepAddController',
          'new/(?P<plan>\d+)/(?P<class>[^/]+)/'
            => 'HarbormasterStepEditController',
+          'view/(?P<id>\d+)/' => 'HarbormasterStepViewController',
          'edit/(?:(?P<id>\d+)/)?' => 'HarbormasterStepEditController',
          'delete/(?:(?P<id>\d+)/)?' => 'HarbormasterStepDeleteController',
        ),
--- a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php
+++ b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php
@ -117,16 +117,7 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController {
          ->setStatusIcon('fa-warning red')
          ->addAttribute(pht(
            'This step has an invalid implementation (%s).',
-            $step->getClassName()))
-          ->addAction(
-            id(new PHUIListItemView())
-              ->setIcon('fa-times')
-              ->addSigil('harbormaster-build-step-delete')
-              ->setWorkflow(true)
-              ->setRenderNameAsTooltip(true)
-              ->setName(pht('Delete'))
-              ->setHref(
-                $this->getApplicationURI('step/delete/'.$step->getID().'/')));
+            $step->getClassName()));
        $step_list->addItem($item);
        continue;
      }
@ -137,23 +128,9 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController {
      $item->addAttribute($implementation->getDescription());

      $step_id = $step->getID();
-      $edit_uri = $this->getApplicationURI("step/edit/{$step_id}/");
-      $delete_uri = $this->getApplicationURI("step/delete/{$step_id}/");

-      if ($can_edit) {
-        $item->setHref($edit_uri);
-      }
-
-      $item
-        ->setHref($edit_uri)
-        ->addAction(
-          id(new PHUIListItemView())
-            ->setIcon('fa-times')
-            ->addSigil('harbormaster-build-step-delete')
-            ->setWorkflow(true)
-            ->setDisabled(!$can_edit)
-            ->setHref(
-              $this->getApplicationURI('step/delete/'.$step->getID().'/')));
+      $view_uri = $this->getApplicationURI("step/view/{$step_id}/");
+      $item->setHref($view_uri);

      $depends = $step->getStepImplementation()->getDependencies($step);
      $inputs = $step->getStepImplementation()->getArtifactInputs();
--- a/src/applications/harbormaster/controller/HarbormasterStepEditController.php
+++ b/src/applications/harbormaster/controller/HarbormasterStepEditController.php
@ -58,6 +58,12 @@ final class HarbormasterStepEditController extends HarbormasterController {

    $plan_uri = $this->getApplicationURI('plan/'.$plan->getID().'/');

+    if ($is_new) {
+      $cancel_uri = $plan_uri;
+    } else {
+      $cancel_uri = $this->getApplicationURI('step/view/'.$step->getID().'/');
+    }
+
    $implementation = $step->getStepImplementation();

    $field_list = PhabricatorCustomField::getObjectFields(
@ -119,7 +125,10 @@ final class HarbormasterStepEditController extends HarbormasterController {

      try {
        $editor->applyTransactions($step, $xactions);
-        return id(new AphrontRedirectResponse())->setURI($plan_uri);
+
+        $step_uri = $this->getApplicationURI('step/view/'.$step->getID().'/');
+
+        return id(new AphrontRedirectResponse())->setURI($step_uri);
      } catch (PhabricatorApplicationTransactionValidationException $ex) {
        $validation_exception = $ex;
      }
@ -162,31 +171,31 @@ final class HarbormasterStepEditController extends HarbormasterController {
          ->setError($e_description)
          ->setValue($v_description));

+    $crumbs = $this->buildApplicationCrumbs();
+    $id = $plan->getID();
+    $crumbs->addTextCrumb(pht('Plan %d', $id), $plan_uri);
+
    if ($is_new) {
      $submit = pht('Create Build Step');
      $header = pht('New Step: %s', $implementation->getName());
-      $crumb = pht('Add Step');
+      $crumbs->addTextCrumb(pht('Add Step'));
    } else {
      $submit = pht('Save Build Step');
      $header = pht('Edit Step: %s', $implementation->getName());
-      $crumb = pht('Edit Step');
+      $crumbs->addTextCrumb(pht('Step %d', $step->getID()), $cancel_uri);
+      $crumbs->addTextCrumb(pht('Edit Step'));
    }

    $form->appendChild(
      id(new AphrontFormSubmitControl())
        ->setValue($submit)
-        ->addCancelButton($plan_uri));
+        ->addCancelButton($cancel_uri));

    $box = id(new PHUIObjectBoxView())
      ->setHeaderText($header)
      ->setValidationException($validation_exception)
      ->setForm($form);

-    $crumbs = $this->buildApplicationCrumbs();
-    $id = $plan->getID();
-    $crumbs->addTextCrumb(pht('Plan %d', $id), $plan_uri);
-    $crumbs->addTextCrumb($crumb);
-
    $variables = $this->renderBuildVariablesTable();

    if ($is_new) {
--- a/src/applications/harbormaster/controller/HarbormasterStepViewController.php
+++ b/src/applications/harbormaster/controller/HarbormasterStepViewController.php
@ -0,0 +1,122 @@
+<?php
+
+final class HarbormasterStepViewController extends HarbormasterController {
+
+  public function handleRequest(AphrontRequest $request) {
+    $viewer = $this->getViewer();
+    $id = $request->getURIData('id');
+
+    $step = id(new HarbormasterBuildStepQuery())
+      ->setViewer($viewer)
+      ->withIDs(array($id))
+      ->executeOne();
+    if (!$step) {
+      return new Aphront404Response();
+    }
+    $plan = $step->getBuildPlan();
+
+    $plan_id = $plan->getID();
+    $plan_uri = $this->getApplicationURI("plan/{$plan_id}/");
+
+    $implementation = $step->getStepImplementation();
+
+    $field_list = PhabricatorCustomField::getObjectFields(
+      $step,
+      PhabricatorCustomField::ROLE_VIEW);
+    $field_list
+      ->setViewer($viewer)
+      ->readFieldsFromStorage($step);
+
+    $crumbs = $this->buildApplicationCrumbs();
+    $crumbs->addTextCrumb(pht('Plan %d', $plan_id), $plan_uri);
+    $crumbs->addTextCrumb(pht('Step %d', $id));
+
+    $box = id(new PHUIObjectBoxView())
+      ->setHeaderText(pht('Build Step %d: %s', $id, $step->getName()));
+
+    $properties = $this->buildPropertyList($step, $field_list);
+    $actions = $this->buildActionList($step);
+    $properties->setActionList($actions);
+
+    $box->addPropertyList($properties);
+
+    $timeline = $this->buildTransactionTimeline(
+      $step,
+      new HarbormasterBuildStepTransactionQuery());
+    $timeline->setShouldTerminate(true);
+
+    return $this->buildApplicationPage(
+      array(
+        $crumbs,
+        $box,
+        $timeline,
+      ),
+      array(
+        'title' => pht('Step %d', $id),
+      ));
+  }
+
+  private function buildPropertyList(
+    HarbormasterBuildStep $step,
+    PhabricatorCustomFieldList $field_list) {
+    $viewer = $this->getViewer();
+
+    $view = id(new PHUIPropertyListView())
+      ->setUser($viewer)
+      ->setObject($step);
+
+    $view->addProperty(
+      pht('Created'),
+      phabricator_datetime($step->getDateCreated(), $viewer));
+
+    $field_list->appendFieldsToPropertyList(
+      $step,
+      $viewer,
+      $view);
+
+    $view->invokeWillRenderEvent();
+
+    $description = $step->getDescription();
+    if (strlen($description)) {
+      $view->addSectionHeader(
+        pht('Description'),
+        PHUIPropertyListView::ICON_SUMMARY);
+      $view->addTextContent(
+        new PHUIRemarkupView($viewer, $description));
+    }
+
+    return $view;
+  }
+
+
+  private function buildActionList(HarbormasterBuildStep $step) {
+    $viewer = $this->getViewer();
+    $id = $step->getID();
+
+    $list = id(new PhabricatorActionListView())
+      ->setUser($viewer)
+      ->setObject($step);
+
+    $can_edit = true;
+
+    $list->addAction(
+      id(new PhabricatorActionView())
+        ->setName(pht('Edit Step'))
+        ->setHref($this->getApplicationURI("step/edit/{$id}/"))
+        ->setWorkflow(!$can_edit)
+        ->setDisabled(!$can_edit)
+        ->setIcon('fa-pencil'));
+
+    $list->addAction(
+      id(new PhabricatorActionView())
+        ->setName(pht('Delete Step'))
+        ->setHref($this->getApplicationURI("step/delete/{$id}/"))
+        ->setWorkflow(true)
+        ->setDisabled(!$can_edit)
+        ->setIcon('fa-times'));
+
+    return $list;
+  }
+
+
+}
--- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
+++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
@ -1420,6 +1420,11 @@ final class PhabricatorUSEnglishTranslation
        ),
      ),

+      'WARNING: There are %s unapproved authorization(s)!' => array(
+        'WARNING: There is an unapproved authorization!',
+        'WARNING: There are unapproved authorizations!',
+      ),
+
    );
  }

--- a/webroot/rsrc/css/application/harbormaster/harbormaster.css
+++ b/webroot/rsrc/css/application/harbormaster/harbormaster.css
@ -3,13 +3,8 @@
 */

 .harbormaster-artifact-io {
-  margin: 0 0 0 8px;
-  padding: 4px 8px;
-  border-width: 1px 0 0 1px;
-  border-style: solid;
-  box-shadow: inset 2px 2px 1px rgba(0, 0, 0, 0.075);
-  background: {$lightbluebackground};
-  border-color: {$lightblueborder};
+  margin: 4px 0 4px 8px;
+  padding: 8px;
 }

 .harbormaster-artifact-summary-header {