Modernize worker task detail view

Summary: Make mobile-friendly and provide UI to cancel/retry tasks. Remove display of task data to arbitrary users, as it may be sensitive.

Test Plan:
{F22502}
{F22503}
{F22504}
{F22505}
{F22506}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2015

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

src/__celerity_resource_map__.php

@@ -51,8 +51,8 @@ celerity_register_resource_map(array(
   ),
   '/rsrc/image/autosprite.png' =>
   array(
-    'hash' => 'e1735b5cadbaf1f70b70a857eab53601',
+    'hash' => 'bc9479b2a610a3ecee18dc88744c4ce6',
-    'uri' => '/res/e1735b5c/rsrc/image/autosprite.png',
+    'uri' => '/res/bc9479b2/rsrc/image/autosprite.png',
     'disk' => '/rsrc/image/autosprite.png',
     'type' => 'png',
   ),
@@ -713,7 +713,7 @@ celerity_register_resource_map(array(
   ),
   'autosprite-css' =>
   array(
-    'uri' => '/res/10fb7fdc/rsrc/css/autosprite.css',
+    'uri' => '/res/6be3e4b3/rsrc/css/autosprite.css',
     'type' => 'css',
     'requires' =>
     array(

src/applications/daemon/controller/PhabricatorWorkerTaskDetailController.php

@@ -31,129 +31,163 @@ final class PhabricatorWorkerTaskDetailController
 
     $task = id(new PhabricatorWorkerActiveTask())->load($this->id);
     if (!$task) {
+      $task = id(new PhabricatorWorkerArchiveTask())->load($this->id);
+    }
+
+    if (!$task) {
+      $title = pht('Task Does Not Exist');
+
       $error_view = new AphrontErrorView();
       $error_view->setTitle('No Such Task');
       $error_view->appendChild(
-        '<p>This task may have recently completed.</p>');
+        '<p>This task may have recently been garbage collected.</p>');
-      $error_view->setSeverity(AphrontErrorView::SEVERITY_WARNING);
+      $error_view->setSeverity(AphrontErrorView::SEVERITY_NODATA);
-      return $this->buildStandardPageResponse(
+
-        $error_view,
+      $content = $error_view;
-        array(
+    } else {
-          'title' => 'Task Does Not Exist',
+      $title = 'Task '.$task->getID();
-        ));
+
+      $header = id(new PhabricatorHeaderView())
+        ->setHeader('Task '.$task->getID().' ('.$task->getTaskClass().')');
+
+      $actions    = $this->buildActionListView($task);
+      $properties = $this->buildPropertyListView($task);
+
+      $content = array(
+        $header,
+        $actions,
+        $properties,
+      );
     }
 
-    $data = id(new PhabricatorWorkerTaskData())->loadOneWhere(
-      'id = %d',
-      $task->getDataID());
-
-    $extra = null;
-    switch ($task->getTaskClass()) {
-      case 'PhabricatorRepositorySvnCommitChangeParserWorker':
-      case 'PhabricatorRepositoryGitCommitChangeParserWorker':
-        $commit_id = idx($data->getData(), 'commitID');
-        if ($commit_id) {
-          $commit = id(new PhabricatorRepositoryCommit())->load($commit_id);
-          if ($commit) {
-            $repository = id(new PhabricatorRepository())->load(
-              $commit->getRepositoryID());
-            if ($repository) {
-              $extra =
-                "<strong>NOTE:</strong> ".
-                "You can manually retry this task by running this script:".
-                "<pre>".
-                  "phabricator/\$ ./scripts/repository/reparse.php ".
-                  "r".
-                  phutil_escape_html($repository->getCallsign()).
-                  phutil_escape_html($commit->getCommitIdentifier()).
-                  " ".
-                  "--change".
-                "</pre>";
-            }
-          }
-        }
-        break;
-      default:
-        break;
-    }
-
-    if ($data) {
-      $data = json_encode($data->getData());
-    }
-
-    $form = id(new AphrontFormView())
-      ->setUser($user)
-      ->appendChild(
-        id(new AphrontFormStaticControl())
-          ->setLabel('ID')
-          ->setValue($task->getID()))
-      ->appendChild(
-        id(new AphrontFormStaticControl())
-          ->setLabel('Type')
-          ->setValue($task->getTaskClass()))
-      ->appendChild(
-        id(new AphrontFormStaticControl())
-          ->setLabel('Lease Owner')
-          ->setValue($task->getLeaseOwner()))
-      ->appendChild(
-        id(new AphrontFormStaticControl())
-          ->setLabel('Lease Expires')
-          ->setValue($task->getLeaseExpires() - time()))
-      ->appendChild(
-        id(new AphrontFormStaticControl())
-          ->setLabel('Failure Count')
-          ->setValue($task->getFailureCount()))
-      ->appendChild(
-        id(new AphrontFormTextAreaControl())
-          ->setLabel('Data')
-          ->setValue($data));
-
-    if ($extra) {
-      $form->appendChild(
-        id(new AphrontFormMarkupControl())
-          ->setLabel('More')
-          ->setValue($extra));
-    }
-
-    $form
-      ->appendChild(
-        id(new AphrontFormSubmitControl())
-          ->addCancelButton('/daemon/', 'Back'));
-
-    $panel = new AphrontPanelView();
-    $panel->setHeader('Task Detail');
-    $panel->setWidth(AphrontPanelView::WIDTH_WIDE);
-    $panel->appendChild($form);
-
-    $panel->addButton(
-      javelin_render_tag(
-        'a',
-        array(
-          'href' => '/daemon/task/'.$task->getID().'/delete/',
-          'class' => 'button grey',
-          'sigil' => 'workflow',
-        ),
-        'Delete Task'));
-
-    $panel->addButton(
-      javelin_render_tag(
-        'a',
-        array(
-          'href' => '/daemon/task/'.$task->getID().'/release/',
-          'class' => 'button grey',
-          'sigil' => 'workflow',
-        ),
-        'Free Lease'));
-
     $nav = $this->buildSideNavView();
     $nav->selectFilter('');
-    $nav->appendChild($panel);
+    $nav->appendChild($content);
 
     return $this->buildApplicationPage(
       $nav,
       array(
-        'title' => 'Task',
+        'title' => $title,
       ));
   }
 
+  private function buildActionListView(PhabricatorWorkerTask $task) {
+    $user = $this->getRequest()->getUser();
+
+    $view = new PhabricatorActionListView();
+    $view->setUser($user);
+
+    $id = $task->getID();
+
+    if ($task->isArchived()) {
+      $result_success = PhabricatorWorkerArchiveTask::RESULT_SUCCESS;
+      $can_retry = ($task->getResult() != $result_success);
+
+      $view->addAction(
+        id(new PhabricatorActionView())
+          ->setName(pht('Retry Task'))
+          ->setHref($this->getApplicationURI('/task/'.$id.'/retry/'))
+          ->setIcon('undo')
+          ->setWorkflow(true)
+          ->setDisabled(!$can_retry));
+    } else {
+      $view->addAction(
+        id(new PhabricatorActionView())
+          ->setName(pht('Cancel Task'))
+          ->setHref($this->getApplicationURI('/task/'.$id.'/cancel/'))
+          ->setIcon('delete')
+          ->setWorkflow(true));
+    }
+
+    $can_release = (!$task->isArchived()) &&
+                   ($task->getLeaseOwner());
+
+    $view->addAction(
+      id(new PhabricatorActionView())
+        ->setName(pht('Free Lease'))
+        ->setHref($this->getApplicationURI('/task/'.$id.'/release/'))
+        ->setIcon('unlock')
+        ->setWorkflow(true)
+        ->setDisabled(!$can_release));
+
+    return $view;
+  }
+
+  private function buildPropertyListView(PhabricatorWorkerTask $task) {
+    $view = new PhabricatorPropertyListView();
+
+    if ($task->isArchived()) {
+      switch ($task->getResult()) {
+        case PhabricatorWorkerArchiveTask::RESULT_SUCCESS:
+          $status = pht('Complete');
+          break;
+        case PhabricatorWorkerArchiveTask::RESULT_FAILURE:
+          $status = pht('Failed');
+          break;
+        case PhabricatorWorkerArchiveTask::RESULT_CANCELLED:
+          $status = pht('Cancelled');
+          break;
+        default:
+          throw new Exception("Unknown task status!");
+      }
+    } else {
+      $status = pht('Queued');
+    }
+
+    $view->addProperty(
+      pht('Task Status'),
+      $status);
+
+    $view->addProperty(
+      pht('Task Class'),
+      phutil_escape_html($task->getTaskClass()));
+
+    if ($task->getLeaseExpires()) {
+      if ($task->getLeaseExpires() > time()) {
+        $lease_status = pht('Leased');
+      } else {
+        $lease_status = pht('Lease Expired');
+      }
+    } else {
+      $lease_status = '<em>'.pht('Not Leased').'</em>';
+    }
+
+    $view->addProperty(
+      pht('Lease Status'),
+      $lease_status);
+
+    $view->addProperty(
+      pht('Lease Owner'),
+      $task->getLeaseOwner()
+        ? phutil_escape_html($task->getLeaseOwner())
+        : '<em>'.pht('None').'</em>');
+
+    if ($task->getLeaseExpires() && $task->getLeaseOwner()) {
+      $expires = ($task->getLeaseExpires() - time());
+      $expires = phabricator_format_relative_time_detailed($expires);
+    } else {
+      $expires = '<em>'.pht('None').'</em>';
+    }
+
+    $view->addProperty(
+      pht('Lease Expires'),
+      $expires);
+
+    $view->addProperty(
+      pht('Failure Count'),
+      phutil_escape_html($task->getFailureCount()));
+
+    if ($task->isArchived()) {
+      $duration = phutil_escape_html(number_format($task->getDuration()).' us');
+    } else {
+      $duration = '<em>'.pht('Not Completed').'</em>';
+    }
+
+    $view->addProperty(
+      pht('Duration'),
+      $duration);
+
+    return $view;
+  }
+
 }

src/applications/daemon/controller/PhabricatorWorkerTaskUpdateController.php

@@ -32,48 +32,104 @@ final class PhabricatorWorkerTaskUpdateController
     $user = $request->getUser();
 
     $task = id(new PhabricatorWorkerActiveTask())->load($this->id);
+    if (!$task) {
+      $task = id(new PhabricatorWorkerArchiveTask())->load($this->id);
+    }
+
     if (!$task) {
       return new Aphront404Response();
     }
 
+    $result_success = PhabricatorWorkerArchiveTask::RESULT_SUCCESS;
+    $can_retry = ($task->isArchived()) &&
+                 ($task->getResult() != $result_success);
+
+    $can_cancel = !$task->isArchived();
+    $can_release = (!$task->isArchived()) &&
+                   ($task->getLeaseOwner());
+
+    $next_uri = $this->getApplicationURI('/task/'.$task->getID().'/');
+
     if ($request->isFormPost()) {
       switch ($this->action) {
-        case 'delete':
+        case 'retry':
-          $task->delete();
+          if ($can_retry) {
+            $task->unarchiveTask();
+          }
+          break;
+        case 'cancel':
+          if ($can_cancel) {
+            // Forcibly break the lease if one exists, so we can archive the
+            // task.
+            $task->setLeaseOwner(null);
+            $task->setLeaseExpires(time());
+
+            $task->archiveTask(
+              PhabricatorWorkerArchiveTask::RESULT_CANCELLED,
+              0);
+          }
           break;
         case 'release':
-          $task->setLeaseOwner(null);
+          if ($can_release) {
-          $task->setLeaseExpires(time());
+            $task->setLeaseOwner(null);
-          $task->save();
+            $task->setLeaseExpires(time());
+            $task->save();
+          }
           break;
       }
-      return id(new AphrontRedirectResponse())->setURI('/daemon/');
+      return id(new AphrontRedirectResponse())
+        ->setURI($next_uri);
     }
 
     $dialog = new AphrontDialogView();
     $dialog->setUser($user);
 
     switch ($this->action) {
-      case 'delete':
+      case 'retry':
-        $dialog->setTitle('Really delete task?');
+        if ($can_retry) {
-        $dialog->appendChild(
+          $dialog->setTitle('Really retry task?');
-          '<p>The work this task represents will never be performed if you '.
+          $dialog->appendChild(
-          'delete it. Are you sure you want to delete it?</p>');
+            '<p>The task will be put back in the queue and executed '.
-        $dialog->addSubmitButton('Delete Task');
+            'again.</p>');
+          $dialog->addSubmitButton('Retry Task');
+        } else {
+          $dialog->setTitle('Can Not Retry');
+          $dialog->appendChild(
+            '<p>Only archived, unsuccessful tasks can be retried.</p>');
+        }
+        break;
+      case 'cancel':
+        if ($can_cancel) {
+          $dialog->setTitle('Really cancel task?');
+          $dialog->appendChild(
+            '<p>The work this task represents will never be performed if you '.
+            'cancel it. Are you sure you want to cancel it?</p>');
+          $dialog->addSubmitButton('Cancel Task');
+        } else {
+          $dialog->setTitle('Can Not Cancel');
+          $dialog->appendChild(
+            '<p>Only active tasks can be cancelled.</p>');
+        }
         break;
       case 'release':
-        $dialog->setTitle('Really free task lease?');
+        if ($can_release) {
-        $dialog->appendChild(
+          $dialog->setTitle('Really free task lease?');
-          '<p>If the process which owns the task lease is still doing work '.
+          $dialog->appendChild(
-          'on it, the work may be performed twice. Are you sure you '.
+            '<p>If the process which owns the task lease is still doing work '.
-          'want to free the lease?</p>');
+            'on it, the work may be performed twice. Are you sure you '.
-        $dialog->addSubmitButton('Free Lease');
+            'want to free the lease?</p>');
+          $dialog->addSubmitButton('Free Lease');
+        } else {
+          $dialog->setTitle('Can Not Free Lease');
+          $dialog->appendChild(
+            '<p>Only active, leased tasks may have their leases freed.</p>');
+        }
         break;
       default:
         return new Aphront404Response();
     }
 
-    $dialog->addCancelButton('/daemon/');
+    $dialog->addCancelButton($next_uri);
 
     return id(new AphrontDialogResponse())->setDialog($dialog);
   }

src/infrastructure/daemon/workers/storage/PhabricatorWorkerArchiveTask.php

@@ -18,8 +18,9 @@
 
 final class PhabricatorWorkerArchiveTask extends PhabricatorWorkerTask {
 
-  const RESULT_SUCCESS = 0;
+  const RESULT_SUCCESS    = 0;
-  const RESULT_FAILURE = 1;
+  const RESULT_FAILURE    = 1;
+  const RESULT_CANCELLED  = 2;
 
   protected $duration;
   protected $result;
@@ -63,4 +64,22 @@ final class PhabricatorWorkerArchiveTask extends PhabricatorWorkerTask {
     return $result;
   }
 
+  public function unarchiveTask() {
+    $this->openTransaction();
+      $active = id(new PhabricatorWorkerActiveTask())
+        ->setID($this->getID())
+        ->setTaskClass($this->getTaskClass())
+        ->setLeaseOwner(null)
+        ->setLeaseExpires(0)
+        ->setFailureCount(0)
+        ->setDataID($this->getDataID())
+        ->insert();
+
+      $this->setDataID(null);
+      $this->delete();
+    $this->saveTransaction();
+
+    return $active;
+  }
+
 }

src/infrastructure/daemon/workers/storage/PhabricatorWorkerTask.php

@@ -37,4 +37,8 @@ abstract class PhabricatorWorkerTask extends PhabricatorWorkerDAO {
     return $this->data;
   }
 
+  public function isArchived() {
+    return ($this instanceof PhabricatorWorkerArchiveTask);
+  }
+
 }

src/view/layout/PhabricatorActionView.php

@@ -151,6 +151,8 @@ final class PhabricatorActionView extends AphrontView {
       'subscribe-add',
       'subscribe-auto',
       'subscribe-delete',
+      'undo',
+      'unlock',
       'unpublish',
       'world',
     );

webroot/rsrc/css/autosprite.css

@@ -808,50 +808,58 @@
   background-position: 0px -7823px;
 }
 
-.action-unpublish {
+.action-undo {
   background-position: 0px -7840px;
 }
 
-.action-world {
+.action-unlock {
   background-position: 0px -7857px;
 }
 
-.remarkup-assist-b {
+.action-unpublish {
   background-position: 0px -7874px;
 }
 
+.action-world {
+  background-position: 0px -7891px;
+}
+
+.remarkup-assist-b {
+  background-position: 0px -7908px;
+}
+
 .remarkup-assist-code {
-  background-position: 0px -7889px;
+  background-position: 0px -7923px;
 }
 
 .remarkup-assist-i {
-  background-position: 0px -7904px;
+  background-position: 0px -7938px;
 }
 
 .remarkup-assist-image {
-  background-position: 0px -7919px;
+  background-position: 0px -7953px;
 }
 
 .remarkup-assist-ol {
-  background-position: 0px -7934px;
+  background-position: 0px -7968px;
 }
 
 .remarkup-assist-tag {
-  background-position: 0px -7949px;
+  background-position: 0px -7983px;
 }
 
 .remarkup-assist-tt {
-  background-position: 0px -7964px;
+  background-position: 0px -7998px;
 }
 
 .remarkup-assist-ul {
-  background-position: 0px -7979px;
+  background-position: 0px -8013px;
 }
 
 .remarkup-assist-help {
-  background-position: 0px -7994px;
+  background-position: 0px -8028px;
 }
 
 .remarkup-assist-table {
-  background-position: 0px -8009px;
+  background-position: 0px -8043px;
 }