mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-23 22:10:55 +01:00
Allow reviewers to mark their own inlines as "Done" before they submit them
Summary: Ref T13195. Ref T8573. This allows reviewers to mark their own inline comments as "Done" before they submit them. If you're leaving a non-actionable comment like "this is good", you can pre-check "Done" to give the author a hint that you don't expect any response. Test Plan: On revisions and commits, added inlines as the author and a reviewer/auditor. Marked them done/not-done before submitting. As author, marked the not-done ones done after submitting. Checked preivews, toggled done/not done states. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13195, T8573 Differential Revision: https://secure.phabricator.com/D19634
This commit is contained in:
parent
046c1b5b82
commit
16a6fc8341
5 changed files with 83 additions and 24 deletions
|
@ -258,19 +258,31 @@ final class PhabricatorAuditEditor
|
||||||
$this->didExpandInlineState = true;
|
$this->didExpandInlineState = true;
|
||||||
|
|
||||||
$actor_phid = $this->getActingAsPHID();
|
$actor_phid = $this->getActingAsPHID();
|
||||||
$actor_is_author = ($object->getAuthorPHID() == $actor_phid);
|
$author_phid = $object->getAuthorPHID();
|
||||||
if (!$actor_is_author) {
|
$actor_is_author = ($actor_phid == $author_phid);
|
||||||
break;
|
|
||||||
}
|
|
||||||
|
|
||||||
$state_map = PhabricatorTransactions::getInlineStateMap();
|
$state_map = PhabricatorTransactions::getInlineStateMap();
|
||||||
|
|
||||||
$inlines = id(new DiffusionDiffInlineCommentQuery())
|
$query = id(new DiffusionDiffInlineCommentQuery())
|
||||||
->setViewer($this->getActor())
|
->setViewer($this->getActor())
|
||||||
->withCommitPHIDs(array($object->getPHID()))
|
->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();
|
->execute();
|
||||||
|
|
||||||
|
if ($actor_is_author) {
|
||||||
|
$inlines[] = id(clone $query)
|
||||||
|
->withHasTransaciton(true)
|
||||||
|
->execute();
|
||||||
|
}
|
||||||
|
|
||||||
|
$inlines = array_mergev($inlines);
|
||||||
|
|
||||||
if (!$inlines) {
|
if (!$inlines) {
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
|
@ -118,8 +118,21 @@ final class DifferentialInlineCommentEditController
|
||||||
throw new Exception(pht('Unable to load revision.'));
|
throw new Exception(pht('Unable to load revision.'));
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($revision->getAuthorPHID() !== $viewer->getPHID()) {
|
$viewer_phid = $viewer->getPHID();
|
||||||
throw new Exception(pht('You are not the revision owner.'));
|
$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;
|
return $inline;
|
||||||
|
|
|
@ -248,19 +248,34 @@ final class DifferentialTransactionEditor
|
||||||
$this->didExpandInlineState = true;
|
$this->didExpandInlineState = true;
|
||||||
|
|
||||||
$actor_phid = $this->getActingAsPHID();
|
$actor_phid = $this->getActingAsPHID();
|
||||||
$actor_is_author = ($object->getAuthorPHID() == $actor_phid);
|
$author_phid = $object->getAuthorPHID();
|
||||||
if (!$actor_is_author) {
|
$actor_is_author = ($actor_phid == $author_phid);
|
||||||
break;
|
|
||||||
}
|
|
||||||
|
|
||||||
$state_map = PhabricatorTransactions::getInlineStateMap();
|
$state_map = PhabricatorTransactions::getInlineStateMap();
|
||||||
|
|
||||||
$inlines = id(new DifferentialDiffInlineCommentQuery())
|
$query = id(new DifferentialDiffInlineCommentQuery())
|
||||||
->setViewer($this->getActor())
|
->setViewer($this->getActor())
|
||||||
->withRevisionPHIDs(array($object->getPHID()))
|
->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();
|
->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) {
|
if (!$inlines) {
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
|
@ -75,9 +75,21 @@ final class DiffusionInlineCommentController
|
||||||
throw new Exception(pht('Failed to load commit.'));
|
throw new Exception(pht('Failed to load commit.'));
|
||||||
}
|
}
|
||||||
|
|
||||||
if ((!$commit->getAuthorPHID()) ||
|
$owner_phid = $commit->getAuthorPHID();
|
||||||
($commit->getAuthorPHID() != $viewer->getPHID())) {
|
$viewer_phid = $viewer->getPHID();
|
||||||
throw new Exception(pht('You can not mark this comment as complete.'));
|
$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;
|
return $inline;
|
||||||
|
|
|
@ -287,15 +287,24 @@ final class PHUIDiffInlineCommentDetailView
|
||||||
|
|
||||||
$done_button = null;
|
$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) {
|
if (!$is_synthetic) {
|
||||||
$draft_state = false;
|
$draft_state = false;
|
||||||
switch ($inline->getFixedState()) {
|
switch ($inline->getFixedState()) {
|
||||||
case PhabricatorInlineCommentInterface::STATE_DRAFT:
|
case PhabricatorInlineCommentInterface::STATE_DRAFT:
|
||||||
$is_done = ($this->getCanMarkDone());
|
$is_done = $mark_done;
|
||||||
$draft_state = true;
|
$draft_state = true;
|
||||||
break;
|
break;
|
||||||
case PhabricatorInlineCommentInterface::STATE_UNDRAFT:
|
case PhabricatorInlineCommentInterface::STATE_UNDRAFT:
|
||||||
$is_done = !($this->getCanMarkDone());
|
$is_done = !$mark_done;
|
||||||
$draft_state = true;
|
$draft_state = true;
|
||||||
break;
|
break;
|
||||||
case PhabricatorInlineCommentInterface::STATE_DONE:
|
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
|
// If you don't have permission to mark the comment as "Done", you also
|
||||||
// can not see the draft state.
|
// can not see the draft state.
|
||||||
if (!$this->getCanMarkDone()) {
|
if (!$mark_done) {
|
||||||
$draft_state = false;
|
$draft_state = false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -321,21 +330,19 @@ final class PHUIDiffInlineCommentDetailView
|
||||||
$classes[] = 'inline-state-is-draft';
|
$classes[] = 'inline-state-is-draft';
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($this->getCanMarkDone()) {
|
if ($mark_done && !$this->preview) {
|
||||||
$done_input = javelin_tag(
|
$done_input = javelin_tag(
|
||||||
'input',
|
'input',
|
||||||
array(
|
array(
|
||||||
'type' => 'checkbox',
|
'type' => 'checkbox',
|
||||||
'checked' => ($is_done ? 'checked' : null),
|
'checked' => ($is_done ? 'checked' : null),
|
||||||
'disabled' => ($this->getCanMarkDone() ? null : 'disabled'),
|
|
||||||
'class' => 'differential-inline-done',
|
'class' => 'differential-inline-done',
|
||||||
'sigil' => 'differential-inline-done',
|
'sigil' => 'differential-inline-done',
|
||||||
));
|
));
|
||||||
$done_button = phutil_tag(
|
$done_button = phutil_tag(
|
||||||
'label',
|
'label',
|
||||||
array(
|
array(
|
||||||
'class' => 'differential-inline-done-label '.
|
'class' => 'differential-inline-done-label ',
|
||||||
($this->getCanMarkDone() ? null : 'done-is-disabled'),
|
|
||||||
),
|
),
|
||||||
array(
|
array(
|
||||||
$done_input,
|
$done_input,
|
||||||
|
|
Loading…
Reference in a new issue