From 99af097ff613512d0065abb9e93c1ce7744e46ac Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 11 Feb 2016 20:30:36 -0800
Subject: [PATCH] Allow task statuses to have claiming disabled

Summary:
Fixes T10343. All solutions here seem basically fine. I think adding this small bit of complexity is OK, and sorrrrt of like this behavior sometimes.

  - Allow disabling this behavior per-status.
  - Disable it by default for "Invalid" and "Duplicate" (I left "wontfix", since that's a resolution?).

Beyond being more flexible, I think this is slightly better?

Test Plan:
  - Closed a task as invalid: no claim.
  - Closed a task as resolved: claim.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10343

Differential Revision: https://secure.phabricator.com/D15257
---
 .../maniphest/config/PhabricatorManiphestConfigOptions.php | 7 +++++--
 .../maniphest/constants/ManiphestTaskStatus.php            | 5 +++++
 .../maniphest/editor/ManiphestTransactionEditor.php        | 5 ++++-
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php
index cb011984bf..724309175d 100644
--- a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php
+++ b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php
@@ -111,6 +111,7 @@ final class PhabricatorManiphestConfigOptions
         'name' => pht('Invalid'),
         'name.full' => pht('Closed, Invalid'),
         'closed' => true,
+        'claim' => false,
         'prefixes' => array(
           'invalidate',
           'invalidates',
@@ -126,6 +127,7 @@ final class PhabricatorManiphestConfigOptions
         'transaction.icon' => 'fa-files-o',
         'special' => ManiphestTaskStatus::SPECIAL_DUPLICATE,
         'closed' => true,
+        'claim' => false,
       ),
       'spite' => array(
         'name' => pht('Spite'),
@@ -202,6 +204,9 @@ The keys you can provide in a specification are:
     tasks can not be created or edited to have this status. Existing tasks with
     this status will not be affected, but you can batch edit them or let them
     die out on their own.
+  - `claim` //Optional bool.// By default, closing an unassigned task claims
+    it. You can set this to `false` to disable this behavior for a particular
+    status.
 
 Statuses will appear in the UI in the order specified. Note the status marked
 `special` as `duplicate` is not settable directly and will not appear in UI
@@ -289,8 +294,6 @@ See the example below for a starting point.
 EOTEXT
 ));
 
-
-
     return array(
       $this->newOption('maniphest.custom-field-definitions', 'wild', array())
         ->setSummary(pht('Custom Maniphest fields.'))
diff --git a/src/applications/maniphest/constants/ManiphestTaskStatus.php b/src/applications/maniphest/constants/ManiphestTaskStatus.php
index ab99f212e5..3a839d8fac 100644
--- a/src/applications/maniphest/constants/ManiphestTaskStatus.php
+++ b/src/applications/maniphest/constants/ManiphestTaskStatus.php
@@ -155,6 +155,10 @@ final class ManiphestTaskStatus extends ManiphestConstants {
     return false;
   }
 
+  public static function isClaimStatus($status) {
+    return self::getStatusAttribute($status, 'claim', true);
+  }
+
   public static function isClosedStatus($status) {
     return !self::isOpenStatus($status);
   }
@@ -279,6 +283,7 @@ final class ManiphestTaskStatus extends ManiphestConstants {
           'suffixes' => 'optional list<string>',
           'keywords' => 'optional list<string>',
           'disabled' => 'optional bool',
+          'claim' => 'optional bool',
         ));
     }
 
diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php
index 922317b206..08cfc66632 100644
--- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php
+++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php
@@ -971,8 +971,11 @@ final class ManiphestTransactionEditor
     // If the task is not assigned, not being assigned, currently open, and
     // being closed, try to assign the actor as the owner.
     if ($is_unassigned && !$any_assign && $is_open && $is_closing) {
+      $is_claim = ManiphestTaskStatus::isClaimStatus($new_status);
+
       // Don't assign the actor if they aren't a real user.
-      if ($actor_phid) {
+      // Don't claim the task if the status is configured to not claim.
+      if ($actor_phid && $is_claim) {
         $results[] = id(new ManiphestTransaction())
           ->setTransactionType(ManiphestTransaction::TYPE_OWNER)
           ->setNewValue($actor_phid);