diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4831225f97..d67bb525ce 100644 --- a/src/__phutil_library_map__.php +++ b/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', diff --git a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php index ee7c71caa6..4344d37c7e 100644 --- a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php @@ -86,6 +86,14 @@ class AphrontDefaultApplicationConfiguration 'changeset/(?\d+)/$' => 'DifferentialChangesetViewController', 'revision/edit/(?:(?\d+)/)?$' => 'DifferentialRevisionEditController', + 'comment/' => array( + 'preview/$' => 'DifferentialCommentPreviewController', + 'save/$' => 'DifferentialCommentSaveController', + 'inline/' => array( + 'preview/$' => 'DifferentialInlineCommentPreviewController', + 'edit/$' => 'DifferentialInlineCommentEditController', + ), + ), ), '/res/' => array( diff --git a/src/aphront/response/400/Aphront400Response.php b/src/aphront/response/400/Aphront400Response.php new file mode 100644 index 0000000000..46ecbe095a --- /dev/null +++ b/src/aphront/response/400/Aphront400Response.php @@ -0,0 +1,28 @@ +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()); + } + +} diff --git a/src/applications/differential/controller/commentsave/__init__.php b/src/applications/differential/controller/commentsave/__init__.php new file mode 100644 index 0000000000..d7c03ad621 --- /dev/null +++ b/src/applications/differential/controller/commentsave/__init__.php @@ -0,0 +1,18 @@ +setRevision($revision); $comment_form->setActions($this->getRevisionCommentActions($revision)); + $comment_form->setActionURI('/differential/comment/save/'); return $this->buildStandardPageResponse( '
'. diff --git a/src/applications/differential/editor/comment/DifferentialCommentEditor.php b/src/applications/differential/editor/comment/DifferentialCommentEditor.php new file mode 100755 index 0000000000..b9e23528ad --- /dev/null +++ b/src/applications/differential/editor/comment/DifferentialCommentEditor.php @@ -0,0 +1,329 @@ +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; + } + +} diff --git a/src/applications/differential/editor/comment/__init__.php b/src/applications/differential/editor/comment/__init__.php new file mode 100644 index 0000000000..5eb988d3ae --- /dev/null +++ b/src/applications/differential/editor/comment/__init__.php @@ -0,0 +1,18 @@ +"; } - 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) { diff --git a/src/applications/differential/mail/feedback/DifferentialFeedbackMail.php b/src/applications/differential/mail/comment/DifferentialCommentMail.php similarity index 86% rename from src/applications/differential/mail/feedback/DifferentialFeedbackMail.php rename to src/applications/differential/mail/comment/DifferentialCommentMail.php index c3715126fd..11ae980ab2 100755 --- a/src/applications/differential/mail/feedback/DifferentialFeedbackMail.php +++ b/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; diff --git a/src/applications/differential/mail/feedback/__init__.php b/src/applications/differential/mail/comment/__init__.php similarity index 86% rename from src/applications/differential/mail/feedback/__init__.php rename to src/applications/differential/mail/comment/__init__.php index dd323e6309..38f77fa0cb 100644 --- a/src/applications/differential/mail/feedback/__init__.php +++ b/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'); diff --git a/src/applications/differential/view/addcomment/DifferentialAddCommentView.php b/src/applications/differential/view/addcomment/DifferentialAddCommentView.php index ec836065bc..7d8c135baf 100644 --- a/src/applications/differential/view/addcomment/DifferentialAddCommentView.php +++ b/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())