Write pathway for Differential Comments

src/__phutil_library_map__.php

@@ -8,6 +8,7 @@
 phutil_register_library_map(array(
   'class' =>
   array(
+    'Aphront400Response' => 'aphront/response/400',
     'Aphront404Response' => 'aphront/response/404',
     'AphrontAjaxResponse' => 'aphront/response/ajax',
     'AphrontApplicationConfiguration' => 'aphront/applicationconfiguration',
@@ -74,6 +75,9 @@ phutil_register_library_map(array(
     'DifferentialChangesetParser' => 'applications/differential/parser/changeset',
     'DifferentialChangesetViewController' => 'applications/differential/controller/changesetview',
     'DifferentialComment' => 'applications/differential/storage/comment',
+    'DifferentialCommentEditor' => 'applications/differential/editor/comment',
+    'DifferentialCommentMail' => 'applications/differential/mail/comment',
+    'DifferentialCommentSaveController' => 'applications/differential/controller/commentsave',
     'DifferentialController' => 'applications/differential/controller/base',
     'DifferentialDAO' => 'applications/differential/storage/base',
     'DifferentialDiff' => 'applications/differential/storage/diff',
@@ -81,7 +85,6 @@ phutil_register_library_map(array(
     'DifferentialDiffProperty' => 'applications/differential/storage/diffproperty',
     'DifferentialDiffTableOfContentsView' => 'applications/differential/view/difftableofcontents',
     'DifferentialDiffViewController' => 'applications/differential/controller/diffview',
-    'DifferentialFeedbackMail' => 'applications/differential/mail/feedback',
     'DifferentialHunk' => 'applications/differential/storage/hunk',
     'DifferentialLintStatus' => 'applications/differential/constants/lintstatus',
     'DifferentialMail' => 'applications/differential/mail/base',
@@ -184,6 +187,7 @@ phutil_register_library_map(array(
   ),
   'requires_class' =>
   array(
+    'Aphront400Response' => 'AphrontResponse',
     'Aphront404Response' => 'AphrontResponse',
     'AphrontAjaxResponse' => 'AphrontResponse',
     'AphrontDefaultApplicationConfiguration' => 'AphrontApplicationConfiguration',
@@ -232,6 +236,8 @@ phutil_register_library_map(array(
     'DifferentialChangesetListView' => 'AphrontView',
     'DifferentialChangesetViewController' => 'DifferentialController',
     'DifferentialComment' => 'DifferentialDAO',
+    'DifferentialCommentMail' => 'DifferentialMail',
+    'DifferentialCommentSaveController' => 'DifferentialController',
     'DifferentialController' => 'PhabricatorController',
     'DifferentialDAO' => 'PhabricatorLiskDAO',
     'DifferentialDiff' => 'DifferentialDAO',
@@ -239,7 +245,6 @@ phutil_register_library_map(array(
     'DifferentialDiffProperty' => 'DifferentialDAO',
     'DifferentialDiffTableOfContentsView' => 'AphrontView',
     'DifferentialDiffViewController' => 'DifferentialController',
-    'DifferentialFeedbackMail' => 'DifferentialMail',
     'DifferentialHunk' => 'DifferentialDAO',
     'DifferentialNewDiffMail' => 'DifferentialReviewRequestMail',
     'DifferentialReviewRequestMail' => 'DifferentialMail',

src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php

@@ -86,6 +86,14 @@ class AphrontDefaultApplicationConfiguration
         'changeset/(?<id>\d+)/$'  => 'DifferentialChangesetViewController',
         'revision/edit/(?:(?<id>\d+)/)?$'
           => 'DifferentialRevisionEditController',
+        'comment/' => array(
+          'preview/$' => 'DifferentialCommentPreviewController',
+          'save/$' => 'DifferentialCommentSaveController',
+          'inline/' => array(
+            'preview/$' => 'DifferentialInlineCommentPreviewController',
+            'edit/$' => 'DifferentialInlineCommentEditController',
+          ),
+        ),
       ),
 
       '/res/' => array(

src/aphront/response/400/Aphront400Response.php

@@ -0,0 +1,28 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * @group aphront
+ */
+class Aphront400Response extends AphrontResponse {
+
+  public function buildResponseString() {
+    return '400 Bad Request';
+  }
+
+}

src/aphront/response/400/__init__.php

@@ -0,0 +1,12 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'aphront/response/base');
+
+
+phutil_require_source('Aphront400Response.php');

src/applications/differential/controller/commentsave/DifferentialCommentSaveController.php

@@ -0,0 +1,56 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+class DifferentialCommentSaveController extends DifferentialController {
+
+  public function processRequest() {
+    $request = $this->getRequest();
+    if (!$request->isFormPost()) {
+      return new Aphront400Response();
+    }
+
+    $revision_id = $request->getInt('revision_id');
+    $revision = id(new DifferentialRevision())->load($revision_id);
+    if (!$revision) {
+      return new Aphront400Response();
+    }
+
+    $comment    = $request->getStr('comment');
+    $action     = $request->getStr('action');
+    $reviewers  = $request->getStr('reviewers');
+
+    $editor = new DifferentialCommentEditor(
+      $revision,
+      $request->getUser()->getPHID(),
+      $action);
+
+    $editor
+      ->setMessage($comment)
+      ->setAttachInlineComments(true)
+      ->setAddCC($action != DifferentialAction::ACTION_RESIGN)
+      ->setAddedReviewers($reviewers)
+      ->save();
+
+    // TODO: Diff change detection?
+    // TODO: Clear draft
+
+    return id(new AphrontRedirectResponse())
+      ->setURI('/D'.$revision->getID());
+  }
+
+}

src/applications/differential/controller/commentsave/__init__.php

@@ -0,0 +1,18 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'aphront/response/400');
+phutil_require_module('phabricator', 'aphront/response/redirect');
+phutil_require_module('phabricator', 'applications/differential/controller/base');
+phutil_require_module('phabricator', 'applications/differential/editor/comment');
+phutil_require_module('phabricator', 'applications/differential/storage/revision');
+
+phutil_require_module('phutil', 'utils');
+
+
+phutil_require_source('DifferentialCommentSaveController.php');

src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php

@@ -83,6 +83,7 @@ class DifferentialRevisionViewController extends DifferentialController {
     $comment_form = new DifferentialAddCommentView();
     $comment_form->setRevision($revision);
     $comment_form->setActions($this->getRevisionCommentActions($revision));
+    $comment_form->setActionURI('/differential/comment/save/');
 
     return $this->buildStandardPageResponse(
       '<div class="differential-primary-pane">'.

src/applications/differential/editor/comment/DifferentialCommentEditor.php

@@ -0,0 +1,329 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+class DifferentialCommentEditor {
+
+  protected $revision;
+  protected $actorPHID;
+  protected $action;
+
+  protected $attachInlineComments;
+  protected $message;
+  protected $addCC;
+  protected $changedByCommit;
+  protected $addedReviewers = array();
+
+  public function __construct(
+    DifferentialRevision $revision,
+    $actor_phid,
+    $action) {
+
+    $this->revision = $revision;
+    $this->actorPHID  = $actor_phid;
+    $this->action   = $action;
+  }
+
+  public function setMessage($message) {
+    $this->message = $message;
+    return $this;
+  }
+
+  public function setAttachInlineComments($attach) {
+    $this->attachInlineComments = $attach;
+    return $this;
+  }
+
+  public function setAddCC($add) {
+    $this->addCC = $add;
+    return $this;
+  }
+
+  public function setChangedByCommit($changed_by_commit) {
+    $this->changedByCommit = $changed_by_commit;
+    return $this;
+  }
+
+  public function getChangedByCommit() {
+    return $this->changedByCommit;
+  }
+
+  public function setAddedReviewers($added_reviewers) {
+    $this->addedReviewers = $added_reviewers;
+    return $this;
+  }
+
+  public function getAddedReviewers() {
+    return $this->addedReviewers;
+  }
+
+  public function save() {
+    $revision = $this->revision;
+    $action = $this->action;
+    $actor_phid = $this->actorPHID;
+    $actor_is_author = ($actor_phid == $revision->getAuthorPHID());
+    $revision_status = $revision->getStatus();
+
+    $revision->loadRelationships();
+    $reviewer_phids = $revision->getReviewers();
+    if ($reviewer_phids) {
+      $reviewer_phids = array_combine($reviewer_phids, $reviewer_phids);
+    }
+
+    switch ($action) {
+      case DifferentialAction::ACTION_COMMENT:
+        break;
+
+      case DifferentialAction::ACTION_RESIGN:
+        if ($actor_is_author) {
+          throw new Exception('You can not resign from your own revision!');
+        }
+        if (isset($reviewer_phids[$actor_phid])) {
+          DifferentialRevisionEditor::alterReviewers(
+            $revision,
+            $reviewer_phids,
+            $rem = array($actor_phid),
+            $add = array(),
+            $actor_phid);
+        }
+        break;
+
+      case DifferentialAction::ACTION_ABANDON:
+        if (!$actor_is_author) {
+          throw new Exception('You can only abandon your revisions.');
+        }
+        if ($revision_status == DifferentialRevisionStatus::COMMITTED) {
+          throw new Exception('You can not abandon a committed revision.');
+        }
+        if ($revision_status == DifferentialRevisionStatus::ABANDONED) {
+          $action = DifferentialAction::ACTION_COMMENT;
+          break;
+        }
+
+        $revision
+          ->setStatus(DifferentialRevisionStatus::ABANDONED)
+          ->save();
+        break;
+
+      case DifferentialAction::ACTION_ACCEPT:
+        if ($actor_is_author) {
+          throw new Exception('You can not accept your own revision.');
+        }
+        if (($revision_status != DifferentialRevisionStatus::NEEDS_REVIEW) &&
+            ($revision_status != DifferentialRevisionStatus::NEEDS_REVISION)) {
+          $action = DifferentialAction::ACTION_COMMENT;
+          break;
+        }
+
+        $revision
+          ->setStatus(DifferentialRevisionStatus::ACCEPTED)
+          ->save();
+
+        if (!isset($reviewer_phids[$actor_phid])) {
+          DifferentialRevisionEditor::addReviewers(
+            $revision,
+            $reviewer_phids,
+            $rem = array(),
+            $add = array($actor_phid),
+            $actor_phid);
+        }
+        break;
+
+      case DifferentialAction::ACTION_REQUEST:
+        if (!$actor_is_author) {
+          throw new Exception('You must own a revision to request review.');
+        }
+        if (($revision_status != DifferentialRevisionStatus::NEEDS_REVISION) &&
+            ($revision_status != DifferentialRevisionStatus::ACCEPTED)) {
+          $action = DifferentialAction::ACTION_COMMENT;
+          break;
+        }
+
+        $revision
+          ->setStatus(DifferentialRevisionStatus::NEEDS_REVIEW)
+          ->save();
+        break;
+
+      case DifferentialAction::ACTION_REJECT:
+        if ($actor_is_author) {
+          throw new Exception(
+            'You can not request changes to your own revision.');
+        }
+        if (($revision_status != DifferentialRevisionStatus::NEEDS_REVIEW) &&
+            ($revision_status != DifferentialRevisionStatus::ACCEPTED)) {
+          $action = DifferentialAction::ACTION_COMMENT;
+          break;
+        }
+
+        if (!isset($reviewer_phids[$actor_phid])) {
+          DifferentialRevisionEditor::addReviewers(
+            $revision,
+            $reviewer_phids,
+            $rem = array(),
+            $add = array($actor_phid),
+            $actor_phid);
+        }
+
+        $revision
+          ->setStatus(DifferentialRevisionStatus::NEEDS_REVISION)
+          ->save();
+        break;
+
+      case DifferentialAction::ACTION_RECLAIM:
+        if (!$actor_is_author) {
+          throw new Exception('You can not reclaim a revision you do not own.');
+        }
+        if ($revision_status != DifferentialRevisionStatus::ABANDONED) {
+          $action = DifferentialAction::ACTION_COMMENT;
+          break;
+        }
+        $revision
+          ->setStatus(DifferentialRevisionStatus::NEEDS_REVIEW)
+          ->save();
+        break;
+
+      case DifferentialAction::ACTION_COMMIT:
+        // This is handled externally. (TODO)
+        break;
+
+      case DifferentialAction::ACTION_ADDREVIEWERS:
+        $added_reviewers = $this->getAddedReviewers();
+        foreach ($added_reviewers as $k => $user_phid) {
+          if ($user_phid == $revision->getAuthorPHID()) {
+            unset($added_reviewers[$k]);
+          }
+          if (!empty($reviewer_phids[$user_phid])) {
+            unset($added_reviewers[$k]);
+          }
+        }
+
+        $added_reviewers = array_unique($added_reviewers);
+
+        if ($added_reviewers) {
+          DifferentialRevisionEditor::addReviewers(
+            $revision,
+            $reviewer_phids,
+            $rem = array(),
+            $add = $added_reviewers,
+            $actor_phid);
+
+// TODO
+//          $unixnames = unixname_multi($added_reviewers);
+          $usernames = $added_reviewers;
+
+          $this->message =
+            'Added reviewers: '.implode(', ', $usernames)."\n\n".
+            $this->message;
+
+        } else {
+          $action = DifferentialAction::ACTION_COMMENT;
+        }
+        break;
+
+      default:
+        throw new Exception('Unsupported action.');
+    }
+
+    // Reload relationships to pick up any reviewer changes.
+    $revision->loadRelationships();
+
+/*
+  TODO
+
+    $inline_comments = array();
+    if ($this->attachInlineComments) {
+      $inline_comments = id(new DifferentialInlineComment())
+        ->loadAllUnsaved($revision, $this->actorPHID);
+    }
+*/
+
+    $comment = id(new DifferentialComment())
+      ->setAuthorPHID($this->actorPHID)
+      ->setRevisionID($revision->getID())
+      ->setAction($action)
+      ->setContent((string)$this->message)
+      ->save();
+
+/*
+    $diff = id(new Diff())->loadActiveWithRevision($revision);
+    $changesets = id(new DifferentialChangeset())->loadAllWithDiff($diff);
+
+    if ($inline_comments) {
+      // We may have feedback on non-current changesets. Rather than orphaning
+      // it, just submit it. This is non-ideal but not horrible.
+      $inline_changeset_ids = array_pull($inline_comments, 'getChangesetID');
+      $load = array();
+      foreach ($inline_changeset_ids as $id) {
+        if (empty($changesets[$id])) {
+          $load[] = $id;
+        }
+      }
+      if ($load) {
+        $changesets += id(new DifferentialChangeset())->loadAllWithIDs($load);
+      }
+      foreach ($inline_comments as $inline) {
+        $inline->setFeedbackID($feedback->getID());
+        $inline->save();
+      }
+    }
+*/
+
+    id(new DifferentialCommentMail(
+      $revision,
+      $this->actorPHID,
+      $comment,
+      /* $changesets TODO */ array(),
+      /* $inline_comments TODO */ array()))
+      ->setToPHIDs(
+        array_merge(
+          $revision->getReviewers(),
+          array($revision->getAuthorPHID())))
+      ->setCCPHIDs($revision->getCCPHIDs())
+      ->setChangedByCommit($this->getChangedByCommit())
+      ->send();
+
+/*
+
+  tODO
+
+    if ($this->addCC) {
+      require_module_lazy('site/tools/differential/lib/editor/revision');
+      DifferentialRevisionEditor::addCCFBID(
+        $revision,
+        $this->actorPHID,
+        $this->actorPHID);
+    }
+*/
+
+/*
+
+  TODO
+
+    $event = array(
+      'revision_id' => $revision->getID(),
+      'fbid'        => $revision->getFBID(),
+      'feedback_id' => $feedback->getID(),
+      'action'      => $feedback->getAction(),
+      'actor'       => $this->actorPHID,
+    );
+    id(new ToolsTimelineEvent('difx', fb_json_encode($event)))->record();
+*/
+
+    return $comment;
+  }
+
+}

src/applications/differential/editor/comment/__init__.php

@@ -0,0 +1,18 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/differential/constants/action');
+phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus');
+phutil_require_module('phabricator', 'applications/differential/editor/revision');
+phutil_require_module('phabricator', 'applications/differential/mail/comment');
+phutil_require_module('phabricator', 'applications/differential/storage/comment');
+
+phutil_require_module('phutil', 'utils');
+
+
+phutil_require_source('DifferentialCommentEditor.php');

src/applications/differential/mail/base/DifferentialMail.php

@@ -27,7 +27,7 @@ abstract class DifferentialMail {
   protected $actorID;
 
   protected $revision;
-  protected $feedback;
+  protected $comment;
   protected $changesets;
   protected $inlineComments;
   protected $isFirstMailAboutRevision;
@@ -214,13 +214,13 @@ EOTEXT;
     return "<differential-rev-{$phid}-req@TODO.com>";
   }
 
-  public function setFeedback($feedback) {
-    $this->feedback = $feedback;
+  public function setComment($comment) {
+    $this->comment = $comment;
     return $this;
   }
 
-  public function getFeedback() {
-    return $this->feedback;
+  public function getComment() {
+    return $this->comment;
   }
 
   public function setChangesets($changesets) {

src/applications/differential/mail/comment/DifferentialCommentMail.php

@@ -16,7 +16,7 @@
  * limitations under the License.
  */
 
-class DifferentialFeedbackMail extends DifferentialMail {
+class DifferentialCommentMail extends DifferentialMail {
 
   protected $changedByCommit;
 
@@ -32,13 +32,13 @@ class DifferentialFeedbackMail extends DifferentialMail {
   public function __construct(
     DifferentialRevision $revision,
     $actor_id,
-    DifferentialFeedback $feedback,
+    DifferentialComment $comment,
     array $changesets,
     array $inline_comments) {
 
     $this->setRevision($revision);
     $this->setActorID($actor_id);
-    $this->setFeedback($feedback);
+    $this->setComment($comment);
     $this->setChangesets($changesets);
     $this->setInlineComments($inline_comments);
 
@@ -47,22 +47,22 @@ class DifferentialFeedbackMail extends DifferentialMail {
   protected function renderSubject() {
     $revision = $this->getRevision();
     $verb = $this->getVerb();
-    return ucwords($verb).': '.$revision->getName();
+    return ucwords($verb).': '.$revision->getTitle();
   }
 
   protected function getVerb() {
-    $feedback = $this->getFeedback();
-    $action = $feedback->getAction();
-    $verb = DifferentialAction::getActionVerb($action);
+    $comment = $this->getComment();
+    $action = $comment->getAction();
+    $verb = DifferentialAction::getActionPastTenseVerb($action);
     return $verb;
   }
 
   protected function renderBody() {
 
-    $feedback = $this->getFeedback();
+    $comment = $this->getComment();
 
     $actor = $this->getActorName();
-    $name  = $this->getRevision()->getName();
+    $name  = $this->getRevision()->getTitle();
     $verb  = $this->getVerb();
 
     $body  = array();
@@ -70,7 +70,7 @@ class DifferentialFeedbackMail extends DifferentialMail {
     $body[] = "{$actor} has {$verb} the revision \"{$name}\".";
     $body[] = null;
 
-    $content = $feedback->getContent();
+    $content = $comment->getContent();
     if (strlen($content)) {
       $body[] = $this->formatText($content);
       $body[] = null;

src/applications/differential/mail/comment/__init__.php

@@ -11,4 +11,4 @@ phutil_require_module('phabricator', 'applications/differential/constants/revisi
 phutil_require_module('phabricator', 'applications/differential/mail/base');
 
 
-phutil_require_source('DifferentialFeedbackMail.php');
+phutil_require_source('DifferentialCommentMail.php');

src/applications/differential/view/addcomment/DifferentialAddCommentView.php

@@ -38,6 +38,8 @@ final class DifferentialAddCommentView extends AphrontView {
 
   public function render() {
 
+    $revision = $this->revision;
+
     $actions = array();
     foreach ($this->actions as $action) {
       $actions[$action] = DifferentialAction::getActionVerb($action);
@@ -46,12 +48,15 @@ final class DifferentialAddCommentView extends AphrontView {
     $form = new AphrontFormView();
     $form
       ->setAction($this->actionURI)
+      ->addHiddenInput('revision_id', $revision->getID())
       ->appendChild(
         id(new AphrontFormSelectControl())
           ->setLabel('Action')
+          ->setName('action')
           ->setOptions($actions))
       ->appendChild(
         id(new AphrontFormTextAreaControl())
+          ->setName('comment')
           ->setLabel('Comment'))
       ->appendChild(
         id(new AphrontFormSubmitControl())