Fix two issues with embedding other fields inside "Summary" or "Test Plan" in Differential with the web UI

Summary:
Ref T11114. Converting to EditEngine caused us to stop running this validation, since these fields no longer subclass this parent. Restore the validation.

Also, make sure we check the //first// line of the value, too. After the change to make "Tests: xyz" a valid title, you could write silly summaries / test plans and escape the check if the first line was bogus.

Test Plan: {F2493228}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11114

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

src/applications/differential/customfield/DifferentialCoreCustomField.php

@@ -64,52 +64,11 @@ abstract class DifferentialCoreCustomField
           continue;
         }
       }
-
-      if (is_string($value)) {
-        $parser = $this->getFieldParser();
-        $result = $parser->parseCorpus($value);
-
-        unset($result['__title__']);
-        unset($result['__summary__']);
-
-        if ($result) {
-          $error = new PhabricatorApplicationTransactionValidationError(
-            $type,
-            pht('Invalid'),
-            pht(
-              'The value you have entered in "%s" can not be parsed '.
-              'unambiguously when rendered in a commit message. Edit the '.
-              'message so that keywords like "Summary:" and "Test Plan:" do '.
-              'not appear at the beginning of lines. Parsed keys: %s.',
-              $this->getFieldName(),
-              implode(', ', array_keys($result))),
-            $xaction);
-          $errors[] = $error;
-          $this->setFieldError(pht('Invalid'));
-          continue;
-        }
-      }
     }
 
     return $errors;
   }
 
-  private function getFieldParser() {
-    if (!$this->fieldParser) {
-      $viewer = $this->getViewer();
-      $parser = DifferentialCommitMessageParser::newStandardParser($viewer);
-
-      // Set custom title and summary keys so we can detect the presence of
-      // "Summary:" in, e.g., a test plan.
-      $parser->setTitleKey('__title__');
-      $parser->setSummaryKey('__summary__');
-
-      $this->fieldParser = $parser;
-    }
-
-    return $this->fieldParser;
-  }
-
   public function canDisableField() {
     return false;
   }

src/applications/differential/field/DifferentialTestPlanCommitMessageField.php

@@ -50,4 +50,11 @@ final class DifferentialTestPlanCommitMessageField
     );
   }
 
+  public function validateTransactions($object, array $xactions) {
+    return $this->validateCommitMessageCorpusTransactions(
+      $object,
+      $xactions,
+      pht('Test Plan'));
+  }
+
 }

src/applications/differential/xaction/DifferentialRevisionSummaryTransaction.php

@@ -54,4 +54,11 @@ final class DifferentialRevisionSummaryTransaction
     return $changes;
   }
 
+  public function validateTransactions($object, array $xactions) {
+    return $this->validateCommitMessageCorpusTransactions(
+      $object,
+      $xactions,
+      pht('Summary'));
+  }
+
 }

src/applications/differential/xaction/DifferentialRevisionTransactionType.php

@@ -1,4 +1,60 @@
 <?php
 
 abstract class DifferentialRevisionTransactionType
-  extends PhabricatorModularTransactionType {}
+  extends PhabricatorModularTransactionType {
+
+  protected function validateCommitMessageCorpusTransactions(
+    $object,
+    array $xactions,
+    $field_name) {
+
+    $errors = array();
+    foreach ($xactions as $xaction) {
+      $error = $this->validateMessageCorpus($xaction, $field_name);
+      if ($error) {
+        $errors[] = $error;
+      }
+    }
+
+    return $errors;
+  }
+
+  private function validateMessageCorpus($xaction, $field_name) {
+    $value = $xaction->getNewValue();
+    if (!strlen($value)) {
+      return null;
+    }
+
+    // Put a placeholder title on the message, because the first line of a
+    // message is now always parsed as a title.
+    $value = "<placeholder>\n".$value;
+
+    $viewer = $this->getActor();
+    $parser = DifferentialCommitMessageParser::newStandardParser($viewer);
+
+    // Set custom title and summary keys so we can detect the presence of
+    // "Summary:" in, e.g., a test plan.
+    $parser->setTitleKey('__title__');
+    $parser->setSummaryKey('__summary__');
+
+    $result = $parser->parseCorpus($value);
+
+    unset($result['__title__']);
+    unset($result['__summary__']);
+
+    if (!$result) {
+      return null;
+    }
+
+    return $this->newInvalidError(
+      pht(
+        'The value you have entered in "%s" can not be parsed '.
+        'unambiguously when rendered in a commit message. Edit the '.
+        'message so that keywords like "Summary:" and "Test Plan:" do '.
+        'not appear at the beginning of lines. Parsed keys: %s.',
+        $field_name,
+        implode(', ', array_keys($result))),
+      $xaction);
+  }
+
+}