diff --git a/src/applications/differential/editor/comment/DifferentialCommentEditor.php b/src/applications/differential/editor/comment/DifferentialCommentEditor.php index 0ea5f93255..f125f78d27 100644 --- a/src/applications/differential/editor/comment/DifferentialCommentEditor.php +++ b/src/applications/differential/editor/comment/DifferentialCommentEditor.php @@ -31,6 +31,8 @@ class DifferentialCommentEditor { private $parentMessageID; private $contentSource; + private $isDaemonWorkflow; + public function __construct( DifferentialRevision $revision, $actor_phid, @@ -88,6 +90,11 @@ class DifferentialCommentEditor { return $this; } + public function setIsDaemonWorkflow($is_daemon) { + $this->isDaemonWorkflow = $is_daemon; + return $this; + } + public function save() { $revision = $this->revision; $action = $this->action; @@ -321,12 +328,30 @@ class DifferentialCommentEditor { case DifferentialAction::ACTION_COMMIT: - // TODO: We allow this from any state because the daemons are - // considered authoritative. However, this technically means that anyone - // can mark anything committed at any time with the right POST. - // Ideally we should set a flag on the editor to tell it whether it - // should enforce the web workflow rules (actor must be author and - // state must be 'accepted') or the daemon workflow rules (no rules). + // NOTE: The daemons can mark things committed from any state. We treat + // them as completely authoritative. + + if (!$this->isDaemonWorkflow) { + if (!$actor_is_author) { + throw new Exception( + "You can not mark a revision you don't own as committed."); + } + + $status_committed = ArcanistDifferentialRevisionStatus::COMMITTED; + $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; + + if ($revision_status == $status_committed) { + throw new DifferentialActionHasNoEffectException( + "You can not mark this revision as committed because it has ". + "already been marked as committed."); + } + + if ($revision_status != $status_accepted) { + throw new DifferentialActionHasNoEffectException( + "You can not mark this revision as committed because it is ". + "has not been accepted."); + } + } $revision ->setStatus(ArcanistDifferentialRevisionStatus::COMMITTED); diff --git a/src/applications/differential/mail/reviewrequest/DifferentialReviewRequestMail.php b/src/applications/differential/mail/reviewrequest/DifferentialReviewRequestMail.php index 4679acd59f..caff1f1dd3 100644 --- a/src/applications/differential/mail/reviewrequest/DifferentialReviewRequestMail.php +++ b/src/applications/differential/mail/reviewrequest/DifferentialReviewRequestMail.php @@ -95,7 +95,8 @@ abstract class DifferentialReviewRequestMail extends DifferentialMail { if (PhabricatorEnv::getEnvConfig('metamta.differential.attach-patches')) { - $revision_id = $this->getRevision()->getID(); + $revision = $this->getRevision(); + $revision_id = $revision->getID(); $diffs = $revision->loadDiffs(); $diff_number = count($diffs); diff --git a/src/applications/repository/worker/commitmessageparser/base/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/base/PhabricatorRepositoryCommitMessageParserWorker.php index a5f522c2c1..ec1b7fa2ce 100644 --- a/src/applications/repository/worker/commitmessageparser/base/PhabricatorRepositoryCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/base/PhabricatorRepositoryCommitMessageParserWorker.php @@ -105,6 +105,7 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker $revision, $committer, DifferentialAction::ACTION_COMMIT); + $editor->setIsDaemonWorkflow(true); $editor->setMessage($message)->save(); } }