From e4e56bb431e9004400e2a7d9357c2f9b22ae4bb9 Mon Sep 17 00:00:00 2001
From: Nick Harper <nharper@fb.com>
Date: Tue, 22 May 2012 17:47:12 -0700
Subject: [PATCH] Return partial differential fields when there's an error
 parsing

Summary:
If someone typos one name in a cc or reviewer list, it would be nice if we
display all of the valid names in the field when running arc diff (in addition
to the error message).

Test Plan:
used the conduit console to check that calling differential.parsecommitmessage
with a list of some valid and some invalid ccs returns a result with both an
error and a list of some ccs. Also ran arc diff with that list of ccs to check
for the correct user experience.

Reviewers: epriestley, jungejason, vrana

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2542
---
 ...uitAPI_differential_parsecommitmessage_Method.php |  3 ++-
 .../parse/DifferentialFieldParseException.php        | 12 +++++++++++-
 .../base/DifferentialFieldSpecification.php          |  3 ++-
 .../DifferentialManiphestTasksFieldSpecification.php |  5 +++--
 4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/src/applications/conduit/method/differential/parsecommitmessage/ConduitAPI_differential_parsecommitmessage_Method.php b/src/applications/conduit/method/differential/parsecommitmessage/ConduitAPI_differential_parsecommitmessage_Method.php
index a25d977652..b7b85deed7 100644
--- a/src/applications/conduit/method/differential/parsecommitmessage/ConduitAPI_differential_parsecommitmessage_Method.php
+++ b/src/applications/conduit/method/differential/parsecommitmessage/ConduitAPI_differential_parsecommitmessage_Method.php
@@ -69,11 +69,12 @@ final class ConduitAPI_differential_parsecommitmessage_Method
       $field = $aux_fields[$field_key];
       try {
         $fields[$field_key] = $field->parseValueFromCommitMessage($field_value);
-        $field->setValueFromParsedCommitMessage($fields[$field_key]);
       } catch (DifferentialFieldParseException $ex) {
         $field_label = $field->renderLabelForCommitMessage();
         $errors[] = "Error parsing field '{$field_label}': ".$ex->getMessage();
+        $fields[$field_key] = $ex->getPartialParse();
       }
+      $field->setValueFromParsedCommitMessage($fields[$field_key]);
     }
 
     if (!$is_partial) {
diff --git a/src/applications/differential/field/exception/parse/DifferentialFieldParseException.php b/src/applications/differential/field/exception/parse/DifferentialFieldParseException.php
index 49610947e4..992fb72168 100644
--- a/src/applications/differential/field/exception/parse/DifferentialFieldParseException.php
+++ b/src/applications/differential/field/exception/parse/DifferentialFieldParseException.php
@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,4 +18,14 @@
 
 final class DifferentialFieldParseException extends Exception {
 
+  private $partialParse;
+
+  public function __construct($message, $partial_parse = null) {
+    parent::__construct($message);
+    $this->partialParse = $partial_parse;
+  }
+
+  public function getPartialParse() {
+    return $this->partialParse;
+  }
 }
diff --git a/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php b/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php
index 970071d95f..fb6e93f615 100644
--- a/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php
+++ b/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php
@@ -750,7 +750,8 @@ abstract class DifferentialFieldSpecification {
         ? "users and mailing lists"
         : "users";
       throw new DifferentialFieldParseException(
-        "Commit message references nonexistent {$what}: {$invalid}.");
+        "Commit message references nonexistent {$what}: {$invalid}.",
+        array_unique($results));
     }
 
     return array_unique($results);
diff --git a/src/applications/differential/field/specification/maniphesttasks/DifferentialManiphestTasksFieldSpecification.php b/src/applications/differential/field/specification/maniphesttasks/DifferentialManiphestTasksFieldSpecification.php
index 3c69861607..b24d1a86e0 100644
--- a/src/applications/differential/field/specification/maniphesttasks/DifferentialManiphestTasksFieldSpecification.php
+++ b/src/applications/differential/field/specification/maniphesttasks/DifferentialManiphestTasksFieldSpecification.php
@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -149,7 +149,8 @@ final class DifferentialManiphestTasksFieldSpecification
       }
       $invalid = implode(', ', $invalid);
       throw new DifferentialFieldParseException(
-        "Commit message references nonexistent {$what}: {$invalid}.");
+        "Commit message references nonexistent {$what}: {$invalid}.",
+        $task_phids);
     }
 
     return $task_phids;