diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php index 984e2c1472..c04bb8185b 100644 --- a/src/applications/audit/editor/PhabricatorAuditEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditEditor.php @@ -258,19 +258,31 @@ final class PhabricatorAuditEditor $this->didExpandInlineState = true; $actor_phid = $this->getActingAsPHID(); - $actor_is_author = ($object->getAuthorPHID() == $actor_phid); - if (!$actor_is_author) { - break; - } + $author_phid = $object->getAuthorPHID(); + $actor_is_author = ($actor_phid == $author_phid); $state_map = PhabricatorTransactions::getInlineStateMap(); - $inlines = id(new DiffusionDiffInlineCommentQuery()) + $query = id(new DiffusionDiffInlineCommentQuery()) ->setViewer($this->getActor()) ->withCommitPHIDs(array($object->getPHID())) - ->withFixedStates(array_keys($state_map)) + ->withFixedStates(array_keys($state_map)); + + $inlines = array(); + + $inlines[] = id(clone $query) + ->withAuthorPHIDs(array($actor_phid)) + ->withHasTransaction(false) ->execute(); + if ($actor_is_author) { + $inlines[] = id(clone $query) + ->withHasTransaciton(true) + ->execute(); + } + + $inlines = array_mergev($inlines); + if (!$inlines) { break; } diff --git a/src/applications/differential/controller/DifferentialInlineCommentEditController.php b/src/applications/differential/controller/DifferentialInlineCommentEditController.php index 2e44e86822..9741cc93ee 100644 --- a/src/applications/differential/controller/DifferentialInlineCommentEditController.php +++ b/src/applications/differential/controller/DifferentialInlineCommentEditController.php @@ -118,8 +118,21 @@ final class DifferentialInlineCommentEditController throw new Exception(pht('Unable to load revision.')); } - if ($revision->getAuthorPHID() !== $viewer->getPHID()) { - throw new Exception(pht('You are not the revision owner.')); + $viewer_phid = $viewer->getPHID(); + $is_owner = ($viewer_phid == $revision->getAuthorPHID()); + $is_author = ($viewer_phid == $inline->getAuthorPHID()); + $is_draft = ($inline->isDraft()); + + if ($is_owner) { + // You own the revision, so you can mark the comment as "Done". + } else if ($is_author && $is_draft) { + // You made this comment and it's still a draft, so you can mark + // it as "Done". + } else { + throw new Exception( + pht( + 'You are not the revision owner, and this is not a draft comment '. + 'you authored.')); } return $inline; diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index 29dac38b5a..c452cb9346 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -248,19 +248,34 @@ final class DifferentialTransactionEditor $this->didExpandInlineState = true; $actor_phid = $this->getActingAsPHID(); - $actor_is_author = ($object->getAuthorPHID() == $actor_phid); - if (!$actor_is_author) { - break; - } + $author_phid = $object->getAuthorPHID(); + $actor_is_author = ($actor_phid == $author_phid); $state_map = PhabricatorTransactions::getInlineStateMap(); - $inlines = id(new DifferentialDiffInlineCommentQuery()) + $query = id(new DifferentialDiffInlineCommentQuery()) ->setViewer($this->getActor()) ->withRevisionPHIDs(array($object->getPHID())) - ->withFixedStates(array_keys($state_map)) + ->withFixedStates(array_keys($state_map)); + + $inlines = array(); + + // We're going to undraft any "done" marks on your own inlines. + $inlines[] = id(clone $query) + ->withAuthorPHIDs(array($actor_phid)) + ->withHasTransaction(false) ->execute(); + // If you're the author, we also undraft any "done" marks on other + // inlines. + if ($actor_is_author) { + $inlines[] = id(clone $query) + ->withHasTransaction(true) + ->execute(); + } + + $inlines = array_mergev($inlines); + if (!$inlines) { break; } diff --git a/src/applications/diffusion/controller/DiffusionInlineCommentController.php b/src/applications/diffusion/controller/DiffusionInlineCommentController.php index bc7448a9e2..ce69511dde 100644 --- a/src/applications/diffusion/controller/DiffusionInlineCommentController.php +++ b/src/applications/diffusion/controller/DiffusionInlineCommentController.php @@ -75,9 +75,21 @@ final class DiffusionInlineCommentController throw new Exception(pht('Failed to load commit.')); } - if ((!$commit->getAuthorPHID()) || - ($commit->getAuthorPHID() != $viewer->getPHID())) { - throw new Exception(pht('You can not mark this comment as complete.')); + $owner_phid = $commit->getAuthorPHID(); + $viewer_phid = $viewer->getPHID(); + $viewer_is_owner = ($owner_phid && ($owner_phid == $viewer_phid)); + $viewer_is_author = ($viewer_phid == $inline->getAuthorPHID()); + $is_draft = $inline->isDraft(); + + if ($viewer_is_owner) { + // You can mark inlines on your own commits as "Done". + } else if ($viewer_is_author && $is_draft) { + // You can mark your own unsubmitted inlines as "Done". + } else { + throw new Exception( + pht( + 'You can not mark this comment as complete: you did not author '. + 'the commit and the comment is not a draft you wrote.')); } return $inline; diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php index a32197059a..4da3bbd8fa 100644 --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php @@ -287,15 +287,24 @@ final class PHUIDiffInlineCommentDetailView $done_button = null; + $mark_done = $this->getCanMarkDone(); + + // Allow users to mark their own draft inlines as "Done". + if ($viewer_phid == $inline->getAuthorPHID()) { + if ($inline->isDraft()) { + $mark_done = true; + } + } + if (!$is_synthetic) { $draft_state = false; switch ($inline->getFixedState()) { case PhabricatorInlineCommentInterface::STATE_DRAFT: - $is_done = ($this->getCanMarkDone()); + $is_done = $mark_done; $draft_state = true; break; case PhabricatorInlineCommentInterface::STATE_UNDRAFT: - $is_done = !($this->getCanMarkDone()); + $is_done = !$mark_done; $draft_state = true; break; case PhabricatorInlineCommentInterface::STATE_DONE: @@ -309,7 +318,7 @@ final class PHUIDiffInlineCommentDetailView // If you don't have permission to mark the comment as "Done", you also // can not see the draft state. - if (!$this->getCanMarkDone()) { + if (!$mark_done) { $draft_state = false; } @@ -321,21 +330,19 @@ final class PHUIDiffInlineCommentDetailView $classes[] = 'inline-state-is-draft'; } - if ($this->getCanMarkDone()) { + if ($mark_done && !$this->preview) { $done_input = javelin_tag( 'input', array( 'type' => 'checkbox', 'checked' => ($is_done ? 'checked' : null), - 'disabled' => ($this->getCanMarkDone() ? null : 'disabled'), 'class' => 'differential-inline-done', 'sigil' => 'differential-inline-done', )); $done_button = phutil_tag( 'label', array( - 'class' => 'differential-inline-done-label '. - ($this->getCanMarkDone() ? null : 'done-is-disabled'), + 'class' => 'differential-inline-done-label ', ), array( $done_input,