From 454771446306f213a3ee68f3d89c8ad1e5e6bbf6 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 9 Sep 2019 13:00:38 -0700
Subject: [PATCH] Add a "Remove flag" action to Herald

Summary: Fixes T13409. This is a companion to the existing "Mark with flag" rule.

Test Plan: Used a "remove flag" rule on an object with no flag (not removed), the right type of flag (removed), and a different type of flag (not removed).

Maniphest Tasks: T13409

Differential Revision: https://secure.phabricator.com/D20796
---
 src/__phutil_library_map__.php                |  6 +-
 .../PhabricatorFlagAddFlagHeraldAction.php    | 15 +---
 .../herald/PhabricatorFlagHeraldAction.php    | 18 +++++
 .../PhabricatorFlagRemoveFlagHeraldAction.php | 79 +++++++++++++++++++
 4 files changed, 104 insertions(+), 14 deletions(-)
 create mode 100644 src/applications/flag/herald/PhabricatorFlagHeraldAction.php
 create mode 100644 src/applications/flag/herald/PhabricatorFlagRemoveFlagHeraldAction.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index e6e9063453..fad919f4dd 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -3438,8 +3438,10 @@ phutil_register_library_map(array(
     'PhabricatorFlagDeleteController' => 'applications/flag/controller/PhabricatorFlagDeleteController.php',
     'PhabricatorFlagDestructionEngineExtension' => 'applications/flag/engineextension/PhabricatorFlagDestructionEngineExtension.php',
     'PhabricatorFlagEditController' => 'applications/flag/controller/PhabricatorFlagEditController.php',
+    'PhabricatorFlagHeraldAction' => 'applications/flag/herald/PhabricatorFlagHeraldAction.php',
     'PhabricatorFlagListController' => 'applications/flag/controller/PhabricatorFlagListController.php',
     'PhabricatorFlagQuery' => 'applications/flag/query/PhabricatorFlagQuery.php',
+    'PhabricatorFlagRemoveFlagHeraldAction' => 'applications/flag/herald/PhabricatorFlagRemoveFlagHeraldAction.php',
     'PhabricatorFlagSearchEngine' => 'applications/flag/query/PhabricatorFlagSearchEngine.php',
     'PhabricatorFlagSelectControl' => 'applications/flag/view/PhabricatorFlagSelectControl.php',
     'PhabricatorFlaggableInterface' => 'applications/flag/interface/PhabricatorFlaggableInterface.php',
@@ -9799,7 +9801,7 @@ phutil_register_library_map(array(
       'PhabricatorFlagDAO',
       'PhabricatorPolicyInterface',
     ),
-    'PhabricatorFlagAddFlagHeraldAction' => 'HeraldAction',
+    'PhabricatorFlagAddFlagHeraldAction' => 'PhabricatorFlagHeraldAction',
     'PhabricatorFlagColor' => 'PhabricatorFlagConstants',
     'PhabricatorFlagConstants' => 'Phobject',
     'PhabricatorFlagController' => 'PhabricatorController',
@@ -9807,8 +9809,10 @@ phutil_register_library_map(array(
     'PhabricatorFlagDeleteController' => 'PhabricatorFlagController',
     'PhabricatorFlagDestructionEngineExtension' => 'PhabricatorDestructionEngineExtension',
     'PhabricatorFlagEditController' => 'PhabricatorFlagController',
+    'PhabricatorFlagHeraldAction' => 'HeraldAction',
     'PhabricatorFlagListController' => 'PhabricatorFlagController',
     'PhabricatorFlagQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+    'PhabricatorFlagRemoveFlagHeraldAction' => 'PhabricatorFlagHeraldAction',
     'PhabricatorFlagSearchEngine' => 'PhabricatorApplicationSearchEngine',
     'PhabricatorFlagSelectControl' => 'AphrontFormControl',
     'PhabricatorFlaggableInterface' => 'PhabricatorPHIDInterface',
diff --git a/src/applications/flag/herald/PhabricatorFlagAddFlagHeraldAction.php b/src/applications/flag/herald/PhabricatorFlagAddFlagHeraldAction.php
index e4ae03915f..57772ecb5f 100644
--- a/src/applications/flag/herald/PhabricatorFlagAddFlagHeraldAction.php
+++ b/src/applications/flag/herald/PhabricatorFlagAddFlagHeraldAction.php
@@ -1,6 +1,7 @@
 <?php
 
-final class PhabricatorFlagAddFlagHeraldAction extends HeraldAction {
+final class PhabricatorFlagAddFlagHeraldAction
+  extends PhabricatorFlagHeraldAction {
 
   const ACTIONCONST = 'flag';
 
@@ -11,18 +12,6 @@ final class PhabricatorFlagAddFlagHeraldAction extends HeraldAction {
     return pht('Mark with flag');
   }
 
-  public function getActionGroupKey() {
-    return HeraldSupportActionGroup::ACTIONGROUPKEY;
-  }
-
-  public function supportsObject($object) {
-    return ($object instanceof PhabricatorFlaggableInterface);
-  }
-
-  public function supportsRuleType($rule_type) {
-    return ($rule_type == HeraldRuleTypeConfig::RULE_TYPE_PERSONAL);
-  }
-
   public function applyEffect($object, HeraldEffect $effect) {
     $phid = $this->getAdapter()->getPHID();
     $rule = $effect->getRule();
diff --git a/src/applications/flag/herald/PhabricatorFlagHeraldAction.php b/src/applications/flag/herald/PhabricatorFlagHeraldAction.php
new file mode 100644
index 0000000000..6f87a4ee6c
--- /dev/null
+++ b/src/applications/flag/herald/PhabricatorFlagHeraldAction.php
@@ -0,0 +1,18 @@
+<?php
+
+abstract class PhabricatorFlagHeraldAction
+  extends HeraldAction {
+
+  public function getActionGroupKey() {
+    return HeraldSupportActionGroup::ACTIONGROUPKEY;
+  }
+
+  public function supportsObject($object) {
+    return ($object instanceof PhabricatorFlaggableInterface);
+  }
+
+  public function supportsRuleType($rule_type) {
+    return ($rule_type === HeraldRuleTypeConfig::RULE_TYPE_PERSONAL);
+  }
+
+}
diff --git a/src/applications/flag/herald/PhabricatorFlagRemoveFlagHeraldAction.php b/src/applications/flag/herald/PhabricatorFlagRemoveFlagHeraldAction.php
new file mode 100644
index 0000000000..93d5b02e2f
--- /dev/null
+++ b/src/applications/flag/herald/PhabricatorFlagRemoveFlagHeraldAction.php
@@ -0,0 +1,79 @@
+<?php
+
+final class PhabricatorFlagRemoveFlagHeraldAction
+  extends PhabricatorFlagHeraldAction {
+
+  const ACTIONCONST = 'unflag';
+
+  const DO_UNFLAG = 'do.unflag';
+  const DO_IGNORE_UNFLAG = 'do.ignore-unflag';
+
+  public function getHeraldActionName() {
+    return pht('Remove flag');
+  }
+
+  public function applyEffect($object, HeraldEffect $effect) {
+    $phid = $this->getAdapter()->getPHID();
+    $rule = $effect->getRule();
+    $author = $rule->getAuthor();
+
+    $flag = PhabricatorFlagQuery::loadUserFlag($author, $phid);
+    if (!$flag) {
+      $this->logEffect(self::DO_IGNORE_UNFLAG, null);
+      return;
+    }
+
+    if ($flag->getColor() !== $effect->getTarget()) {
+      $this->logEffect(self::DO_IGNORE_UNFLAG, $flag->getColor());
+      return;
+    }
+
+    $flag->delete();
+
+    $this->logEffect(self::DO_UNFLAG, $flag->getColor());
+  }
+
+  public function getHeraldActionValueType() {
+    return id(new HeraldSelectFieldValue())
+      ->setKey('flag.color')
+      ->setOptions(PhabricatorFlagColor::getColorNameMap())
+      ->setDefault(PhabricatorFlagColor::COLOR_BLUE);
+  }
+
+  protected function getActionEffectMap() {
+    return array(
+      self::DO_IGNORE_UNFLAG => array(
+        'icon' => 'fa-times',
+        'color' => 'grey',
+        'name' => pht('Did Not Remove Flag'),
+      ),
+      self::DO_UNFLAG => array(
+        'icon' => 'fa-flag',
+        'name' => pht('Removed Flag'),
+      ),
+    );
+  }
+
+  public function renderActionDescription($value) {
+    $color = PhabricatorFlagColor::getColorName($value);
+    return pht('Remove %s flag.', $color);
+  }
+
+  protected function renderActionEffectDescription($type, $data) {
+    switch ($type) {
+      case self::DO_IGNORE_UNFLAG:
+        if (!$data) {
+          return pht('Not marked with any flag.');
+        } else {
+          return pht(
+            'Marked with flag of the wrong color ("%s").',
+            PhabricatorFlagColor::getColorName($data));
+        }
+      case self::DO_UNFLAG:
+        return pht(
+          'Removed "%s" flag.',
+          PhabricatorFlagColor::getColorName($data));
+    }
+  }
+
+}