Allow triggers to be attached to and removed from workboard columns

Summary: Depends on D20286. Ref T5474. Attaches triggers to columns and makes "Remove Trigger" work. (There's no "pick an existing named trigger from a list" UI yet, but I plan to add that at some point.) Test Plan: Attached and removed triggers, saw column UI update appropriately. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T5474 Differential Revision: https://secure.phabricator.com/D20287
2024-11-22 23:02:42 +01:00 · 2019-03-14 08:36:43 -07:00 · 2019-03-14 08:36:43 -07:00 · 916bf1a8f9
commit 916bf1a8f9
parent 0204489a52
8 changed files with 257 additions and 40 deletions
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@ -4070,6 +4070,7 @@ phutil_register_library_map(array(
    'PhabricatorProjectColumnPositionQuery' => 'applications/project/query/PhabricatorProjectColumnPositionQuery.php',
    'PhabricatorProjectColumnPriorityOrder' => 'applications/project/order/PhabricatorProjectColumnPriorityOrder.php',
    'PhabricatorProjectColumnQuery' => 'applications/project/query/PhabricatorProjectColumnQuery.php',
+    'PhabricatorProjectColumnRemoveTriggerController' => 'applications/project/controller/PhabricatorProjectColumnRemoveTriggerController.php',
    'PhabricatorProjectColumnSearchEngine' => 'applications/project/query/PhabricatorProjectColumnSearchEngine.php',
    'PhabricatorProjectColumnStatusOrder' => 'applications/project/order/PhabricatorProjectColumnStatusOrder.php',
    'PhabricatorProjectColumnStatusTransaction' => 'applications/project/xaction/column/PhabricatorProjectColumnStatusTransaction.php',
@ -4078,6 +4079,7 @@ phutil_register_library_map(array(
    'PhabricatorProjectColumnTransactionEditor' => 'applications/project/editor/PhabricatorProjectColumnTransactionEditor.php',
    'PhabricatorProjectColumnTransactionQuery' => 'applications/project/query/PhabricatorProjectColumnTransactionQuery.php',
    'PhabricatorProjectColumnTransactionType' => 'applications/project/xaction/column/PhabricatorProjectColumnTransactionType.php',
+    'PhabricatorProjectColumnTriggerTransaction' => 'applications/project/xaction/column/PhabricatorProjectColumnTriggerTransaction.php',
    'PhabricatorProjectConfigOptions' => 'applications/project/config/PhabricatorProjectConfigOptions.php',
    'PhabricatorProjectConfiguredCustomField' => 'applications/project/customfield/PhabricatorProjectConfiguredCustomField.php',
    'PhabricatorProjectController' => 'applications/project/controller/PhabricatorProjectController.php',
@ -10184,6 +10186,7 @@ phutil_register_library_map(array(
    'PhabricatorProjectColumnPositionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
    'PhabricatorProjectColumnPriorityOrder' => 'PhabricatorProjectColumnOrder',
    'PhabricatorProjectColumnQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+    'PhabricatorProjectColumnRemoveTriggerController' => 'PhabricatorProjectBoardController',
    'PhabricatorProjectColumnSearchEngine' => 'PhabricatorApplicationSearchEngine',
    'PhabricatorProjectColumnStatusOrder' => 'PhabricatorProjectColumnOrder',
    'PhabricatorProjectColumnStatusTransaction' => 'PhabricatorProjectColumnTransactionType',
@ -10192,6 +10195,7 @@ phutil_register_library_map(array(
    'PhabricatorProjectColumnTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
    'PhabricatorProjectColumnTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
    'PhabricatorProjectColumnTransactionType' => 'PhabricatorModularTransactionType',
+    'PhabricatorProjectColumnTriggerTransaction' => 'PhabricatorProjectColumnTransactionType',
    'PhabricatorProjectConfigOptions' => 'PhabricatorApplicationConfigOptions',
    'PhabricatorProjectConfiguredCustomField' => array(
      'PhabricatorProjectStandardCustomField',
--- a/src/applications/project/application/PhabricatorProjectApplication.php
+++ b/src/applications/project/application/PhabricatorProjectApplication.php
@ -89,6 +89,10 @@ final class PhabricatorProjectApplication extends PhabricatorApplication {
          'background/'
            => 'PhabricatorProjectBoardBackgroundController',
        ),
+        'column/' => array(
+          'remove/(?P<id>\d+)/' =>
+            'PhabricatorProjectColumnRemoveTriggerController',
+        ),
        'trigger/' => array(
          $this->getQueryRoutePattern() =>
            'PhabricatorProjectTriggerListController',
--- a/src/applications/project/controller/PhabricatorProjectBoardViewController.php
+++ b/src/applications/project/controller/PhabricatorProjectBoardViewController.php
@ -574,6 +574,11 @@ final class PhabricatorProjectBoardViewController
      $column_menu = $this->buildColumnMenu($project, $column);
      $panel->addHeaderAction($column_menu);

+      if ($column->canHaveTrigger()) {
+        $trigger_menu = $this->buildTriggerMenu($column);
+        $panel->addHeaderAction($trigger_menu);
+      }
+
      $count_tag = id(new PHUITagView())
        ->setType(PHUITagView::TYPE_SHADE)
        ->setColor(PHUITagView::COLOR_BLUE)
@ -1172,40 +1177,6 @@ final class PhabricatorProjectBoardViewController
        ->setWorkflow(true);
    }

-    if ($column->canHaveTrigger()) {
-      $column_items[] = id(new PhabricatorActionView())
-        ->setType(PhabricatorActionView::TYPE_DIVIDER);
-
-      $trigger = $column->getTrigger();
-      if (!$trigger) {
-        $set_uri = $this->getApplicationURI(
-          new PhutilURI(
-            'trigger/edit/',
-            array(
-              'columnPHID' => $column->getPHID(),
-            )));
-
-        $column_items[] = id(new PhabricatorActionView())
-          ->setIcon('fa-cogs')
-          ->setName(pht('New Trigger...'))
-          ->setHref($set_uri)
-          ->setDisabled(!$can_edit);
-      } else {
-        $column_items[] = id(new PhabricatorActionView())
-          ->setIcon('fa-cogs')
-          ->setName(pht('View Trigger'))
-          ->setHref($trigger->getURI())
-          ->setDisabled(!$can_edit);
-      }
-
-      $column_items[] = id(new PhabricatorActionView())
-        ->setIcon('fa-times')
-        ->setName(pht('Remove Trigger'))
-        ->setHref('#')
-        ->setWorkflow(true)
-        ->setDisabled(!$can_edit || !$trigger);
-    }
-
    $column_menu = id(new PhabricatorActionListView())
      ->setUser($viewer);
    foreach ($column_items as $item) {
@ -1213,7 +1184,7 @@ final class PhabricatorProjectBoardViewController
    }

    $column_button = id(new PHUIIconView())
-      ->setIcon('fa-caret-down')
+      ->setIcon('fa-pencil')
      ->setHref('#')
      ->addSigil('boards-dropdown-menu')
      ->setMetadata(
@ -1224,6 +1195,85 @@ final class PhabricatorProjectBoardViewController
    return $column_button;
  }

+  private function buildTriggerMenu(PhabricatorProjectColumn $column) {
+    $viewer = $this->getViewer();
+    $trigger = $column->getTrigger();
+
+    $can_edit = PhabricatorPolicyFilter::hasCapability(
+      $viewer,
+      $column,
+      PhabricatorPolicyCapability::CAN_EDIT);
+
+    $trigger_items = array();
+    if (!$trigger) {
+      $set_uri = $this->getApplicationURI(
+        new PhutilURI(
+          'trigger/edit/',
+          array(
+            'columnPHID' => $column->getPHID(),
+          )));
+
+      $trigger_items[] = id(new PhabricatorActionView())
+        ->setIcon('fa-cogs')
+        ->setName(pht('New Trigger...'))
+        ->setHref($set_uri)
+        ->setDisabled(!$can_edit);
+    } else {
+      $trigger_items[] = id(new PhabricatorActionView())
+        ->setIcon('fa-cogs')
+        ->setName(pht('View Trigger'))
+        ->setHref($trigger->getURI())
+        ->setDisabled(!$can_edit);
+    }
+
+    $remove_uri = $this->getApplicationURI(
+      new PhutilURI(
+        urisprintf(
+          'column/remove/%d/',
+          $column->getID())));
+
+    $trigger_items[] = id(new PhabricatorActionView())
+      ->setIcon('fa-times')
+      ->setName(pht('Remove Trigger'))
+      ->setHref($remove_uri)
+      ->setWorkflow(true)
+      ->setDisabled(!$can_edit || !$trigger);
+
+    $trigger_menu = id(new PhabricatorActionListView())
+      ->setUser($viewer);
+    foreach ($trigger_items as $item) {
+      $trigger_menu->addAction($item);
+    }
+
+    if ($trigger) {
+      $trigger_icon = 'fa-cogs';
+    } else {
+      $trigger_icon = 'fa-cogs grey';
+    }
+
+    if ($trigger) {
+      $trigger_tip = array(
+        pht('%s: %s', $trigger->getObjectName(), $trigger->getDisplayName()),
+        $trigger->getRulesDescription(),
+      );
+      $trigger_tip = implode("\n", $trigger_tip);
+    } else {
+      $trigger_tip = pht('No column trigger.');
+    }
+
+    $trigger_button = id(new PHUIIconView())
+      ->setIcon($trigger_icon)
+      ->setHref('#')
+      ->addSigil('boards-dropdown-menu')
+      ->addSigil('has-tooltip')
+      ->setMetadata(
+        array(
+          'items' => hsprintf('%s', $trigger_menu),
+          'tip' => $trigger_tip,
+        ));
+
+    return $trigger_button;
+  }

  /**
   * Add current state parameters (like order and the visibility of hidden
--- a/src/applications/project/controller/PhabricatorProjectColumnRemoveTriggerController.php
+++ b/src/applications/project/controller/PhabricatorProjectColumnRemoveTriggerController.php
@ -0,0 +1,60 @@
+<?php
+
+final class PhabricatorProjectColumnRemoveTriggerController
+  extends PhabricatorProjectBoardController {
+
+  public function handleRequest(AphrontRequest $request) {
+    $viewer = $request->getViewer();
+    $id = $request->getURIData('id');
+
+    $column = id(new PhabricatorProjectColumnQuery())
+      ->setViewer($viewer)
+      ->withIDs(array($id))
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->executeOne();
+    if (!$column) {
+      return new Aphront404Response();
+    }
+
+    $done_uri = $column->getBoardURI();
+
+    if (!$column->getTriggerPHID()) {
+      return $this->newDialog()
+        ->setTitle(pht('No Trigger'))
+        ->appendParagraph(
+          pht('This column does not have a trigger.'))
+        ->addCancelButton($done_uri);
+    }
+
+    if ($request->isFormPost()) {
+      $column_xactions = array();
+
+      $column_xactions[] = $column->getApplicationTransactionTemplate()
+        ->setTransactionType(
+          PhabricatorProjectColumnTriggerTransaction::TRANSACTIONTYPE)
+        ->setNewValue(null);
+
+      $column_editor = $column->getApplicationTransactionEditor()
+        ->setActor($viewer)
+        ->setContentSourceFromRequest($request)
+        ->setContinueOnNoEffect(true)
+        ->setContinueOnMissingFields(true);
+
+      $column_editor->applyTransactions($column, $column_xactions);
+
+      return id(new AphrontRedirectResponse())->setURI($done_uri);
+    }
+
+    $body = pht('Really remove the trigger from this column?');
+
+    return $this->newDialog()
+      ->setTitle(pht('Remove Trigger'))
+      ->appendParagraph($body)
+      ->addSubmitButton(pht('Remove Trigger'))
+      ->addCancelButton($done_uri);
+  }
+}
--- a/src/applications/project/controller/trigger/PhabricatorProjectTriggerEditController.php
+++ b/src/applications/project/controller/trigger/PhabricatorProjectTriggerEditController.php
@ -93,13 +93,16 @@ final class PhabricatorProjectTriggerEditController
        if ($column) {
          $column_xactions = array();

-          // TODO: Modularize column transactions so we can change the column
-          // trigger here. For now, this does nothing.
+          $column_xactions[] = $column->getApplicationTransactionTemplate()
+            ->setTransactionType(
+              PhabricatorProjectColumnTriggerTransaction::TRANSACTIONTYPE)
+            ->setNewValue($trigger->getPHID());

          $column_editor = $column->getApplicationTransactionEditor()
            ->setActor($viewer)
            ->setContentSourceFromRequest($request)
-            ->setContinueOnNoEffect(true);
+            ->setContinueOnNoEffect(true)
+            ->setContinueOnMissingFields(true);

          $column_editor->applyTransactions($column, $column_xactions);

--- a/src/applications/project/query/PhabricatorProjectColumnQuery.php
+++ b/src/applications/project/query/PhabricatorProjectColumnQuery.php
@ -148,7 +148,7 @@ final class PhabricatorProjectColumnQuery
        $triggers = id(new PhabricatorProjectTriggerQuery())
          ->setViewer($this->getViewer())
          ->setParentQuery($this)
-          ->withPHIDs(array($this->getPHID()))
+          ->withPHIDs($trigger_phids)
          ->execute();
        $triggers = mpull($triggers, null, 'getPHID');
      } else {
--- a/src/applications/project/storage/PhabricatorProjectTrigger.php
+++ b/src/applications/project/storage/PhabricatorProjectTrigger.php
@ -60,6 +60,11 @@ final class PhabricatorProjectTrigger
    return pht('Trigger %d', $this->getID());
  }

+  public function getRulesDescription() {
+    // TODO: Summarize the trigger rules in human-readable text.
+    return pht('Does things.');
+  }
+

 /* -(  PhabricatorApplicationTransactionInterface  )------------------------- */

@ -102,7 +107,20 @@ final class PhabricatorProjectTrigger

  public function destroyObjectPermanently(
    PhabricatorDestructionEngine $engine) {
-    $this->delete();
+
+    $this->openTransaction();
+      $conn = $this->establishConnection('w');
+
+      // Remove the reference to this trigger from any columns which use it.
+      queryfx(
+        $conn,
+        'UPDATE %R SET triggerPHID = null WHERE triggerPHID = %s',
+        new PhabricatorProjectColumn(),
+        $this->getPHID());
+
+      $this->delete();
+
+    $this->saveTransaction();
  }

 }
--- a/src/applications/project/xaction/column/PhabricatorProjectColumnTriggerTransaction.php
+++ b/src/applications/project/xaction/column/PhabricatorProjectColumnTriggerTransaction.php
@ -0,0 +1,78 @@
+<?php
+
+final class PhabricatorProjectColumnTriggerTransaction
+  extends PhabricatorProjectColumnTransactionType {
+
+  const TRANSACTIONTYPE = 'trigger';
+
+  public function generateOldValue($object) {
+    return $object->getTriggerPHID();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setTriggerPHID($value);
+  }
+
+  public function getTitle() {
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    if (!$old) {
+      return pht(
+        '%s set the column trigger to %s.',
+        $this->renderAuthor(),
+        $this->renderNewHandle());
+    } else if (!$new) {
+      return pht(
+        '%s removed the trigger for this column (was %s).',
+        $this->renderAuthor(),
+        $this->renderOldHandle());
+    } else {
+      return pht(
+        '%s changed the trigger for this column from %s to %s.',
+        $this->renderAuthor(),
+        $this->renderOldHandle(),
+        $this->renderNewHandle());
+    }
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $actor = $this->getActor();
+    $errors = array();
+
+    foreach ($xactions as $xaction) {
+      $trigger_phid = $xaction->getNewValue();
+
+      // You can always remove a trigger.
+      if (!$trigger_phid) {
+        continue;
+      }
+
+      // You can't put a trigger on a column that can't have triggers, like
+      // a backlog column or a proxy column.
+      if (!$object->canHaveTrigger()) {
+        $errors[] = $this->newInvalidError(
+          pht('This column can not have a trigger.'),
+          $xaction);
+        continue;
+      }
+
+      $trigger = id(new PhabricatorProjectTriggerQuery())
+        ->setViewer($actor)
+        ->withPHIDs(array($trigger_phid))
+        ->execute();
+      if (!$trigger) {
+        $errors[] = $this->newInvalidError(
+          pht(
+            'Trigger "%s" is not a valid trigger, or you do not have '.
+            'permission to view it.',
+            $trigger_phid),
+          $xaction);
+        continue;
+      }
+    }
+
+    return $errors;
+  }
+
+}