mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-23 05:50:55 +01:00
Replace "loadUnsubmittedInlineComments()" with a modern "DiffQuery"
Summary: Ref T13513. All queries now go through a reasonably minimal set of pathways and should have consistent behavior. Test Plan: - Loaded a revision with inlines. - Created a new empty inline, reloaded page, saw it vanish. - Created a new empty inline, typed draft text, did not save, reloaded page, saw draft present. - Created a new empty inline, typed draft text. Submitted feedback, got prompt, answered "Y", saw draft text submit. - Created a new empty inline, typed draft text, scrolled down to bottom of page, typed non-draft text, saw preview include draft text. - Marked and submitted "Done". - Used hide/show on inlines, verified state persisted. - Did much of the same stuff in Diffusion, where it all works the same way (except: there's no prompt when submitting draft is-editing inlines). Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21234
This commit is contained in:
parent
79107574a7
commit
94a95efa05
6 changed files with 105 additions and 118 deletions
|
@ -302,24 +302,14 @@ final class DifferentialRevisionEditEngine
|
|||
|
||||
protected function newAutomaticCommentTransactions($object) {
|
||||
$viewer = $this->getViewer();
|
||||
$xactions = array();
|
||||
|
||||
$inlines = DifferentialTransactionQuery::loadUnsubmittedInlineComments(
|
||||
$viewer,
|
||||
$object);
|
||||
$inlines = msort($inlines, 'getID');
|
||||
|
||||
$editor = $object->getApplicationTransactionEditor()
|
||||
->setActor($viewer);
|
||||
|
||||
$query_template = id(new DifferentialDiffInlineCommentQuery())
|
||||
->withRevisionPHIDs(array($object->getPHID()));
|
||||
|
||||
$xactions = $editor->newAutomaticInlineTransactions(
|
||||
$object,
|
||||
$inlines,
|
||||
DifferentialTransaction::TYPE_INLINE,
|
||||
$query_template);
|
||||
new DifferentialDiffInlineCommentQuery());
|
||||
|
||||
return $xactions;
|
||||
}
|
||||
|
|
|
@ -7,9 +7,11 @@ final class DifferentialRevisionDraftEngine
|
|||
$viewer = $this->getViewer();
|
||||
$revision = $this->getObject();
|
||||
|
||||
$inlines = DifferentialTransactionQuery::loadUnsubmittedInlineComments(
|
||||
$viewer,
|
||||
$revision);
|
||||
$inlines = id(new DifferentialDiffInlineCommentQuery())
|
||||
->setViewer($viewer)
|
||||
->withRevisionPHIDs(array($revision->getPHID()))
|
||||
->withPublishableComments(true)
|
||||
->execute();
|
||||
|
||||
return (bool)$inlines;
|
||||
}
|
||||
|
|
|
@ -7,49 +7,4 @@ final class DifferentialTransactionQuery
|
|||
return new DifferentialTransaction();
|
||||
}
|
||||
|
||||
public static function loadUnsubmittedInlineComments(
|
||||
PhabricatorUser $viewer,
|
||||
DifferentialRevision $revision) {
|
||||
|
||||
$inlines = id(new DifferentialDiffInlineCommentQuery())
|
||||
->setViewer($viewer)
|
||||
->withRevisionPHIDs(array($revision->getPHID()))
|
||||
->withAuthorPHIDs(array($viewer->getPHID()))
|
||||
->withHasTransaction(false)
|
||||
->withIsDeleted(false)
|
||||
->needReplyToComments(true)
|
||||
->execute();
|
||||
|
||||
foreach ($inlines as $key => $inline) {
|
||||
$inlines[$key] = DifferentialInlineComment::newFromModernComment(
|
||||
$inline);
|
||||
}
|
||||
|
||||
PhabricatorInlineComment::loadAndAttachVersionedDrafts(
|
||||
$viewer,
|
||||
$inlines);
|
||||
|
||||
// Don't count void inlines when considering draft state.
|
||||
foreach ($inlines as $key => $inline) {
|
||||
if ($inline->isVoidComment($viewer)) {
|
||||
unset($inlines[$key]);
|
||||
continue;
|
||||
}
|
||||
|
||||
// For other inlines: if they have a nonempty draft state, set their
|
||||
// content to the draft state content. We want to submit the comment
|
||||
// as it is currently shown to the user, not as it was stored the last
|
||||
// time they clicked "Save".
|
||||
|
||||
$draft_content = $inline->getContentForEdit($viewer);
|
||||
if (strlen($draft_content)) {
|
||||
$inline->setContent($draft_content);
|
||||
}
|
||||
}
|
||||
|
||||
$inlines = mpull($inlines, 'getStorageObject');
|
||||
|
||||
return $inlines;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -126,27 +126,14 @@ final class DiffusionCommitEditEngine
|
|||
|
||||
protected function newAutomaticCommentTransactions($object) {
|
||||
$viewer = $this->getViewer();
|
||||
$xactions = array();
|
||||
|
||||
$inlines = id(new DiffusionDiffInlineCommentQuery())
|
||||
->setViewer($viewer)
|
||||
->withObjectPHIDs(array($object->getPHID()))
|
||||
->withPublishableComments(true)
|
||||
->needReplyToComments(true)
|
||||
->execute();
|
||||
$inlines = msort($inlines, 'getID');
|
||||
|
||||
$editor = $object->getApplicationTransactionEditor()
|
||||
->setActor($viewer);
|
||||
|
||||
$query_template = id(new DiffusionDiffInlineCommentQuery())
|
||||
->withCommitPHIDs(array($object->getPHID()));
|
||||
|
||||
$xactions = $editor->newAutomaticInlineTransactions(
|
||||
$object,
|
||||
$inlines,
|
||||
PhabricatorAuditActionConstants::INLINE,
|
||||
$query_template);
|
||||
new DiffusionDiffInlineCommentQuery());
|
||||
|
||||
return $xactions;
|
||||
}
|
||||
|
|
|
@ -5027,18 +5027,23 @@ abstract class PhabricatorApplicationTransactionEditor
|
|||
|
||||
public function newAutomaticInlineTransactions(
|
||||
PhabricatorLiskDAO $object,
|
||||
array $inlines,
|
||||
$transaction_type,
|
||||
PhabricatorCursorPagedPolicyAwareQuery $query_template) {
|
||||
|
||||
$actor = $this->getActor();
|
||||
|
||||
$inlines = id(clone $query_template)
|
||||
->setViewer($actor)
|
||||
->withObjectPHIDs(array($object->getPHID()))
|
||||
->withPublishableComments(true)
|
||||
->needAppliedDrafts(true)
|
||||
->needReplyToComments(true)
|
||||
->execute();
|
||||
$inlines = msort($inlines, 'getID');
|
||||
|
||||
$xactions = array();
|
||||
|
||||
foreach ($inlines as $key => $inline) {
|
||||
if ($inline->isEmptyInlineComment()) {
|
||||
unset($inlines[$key]);
|
||||
continue;
|
||||
}
|
||||
|
||||
$xactions[] = $object->getApplicationTransactionTemplate()
|
||||
->setTransactionType($transaction_type)
|
||||
->attachComment($inline);
|
||||
|
@ -5065,31 +5070,17 @@ abstract class PhabricatorApplicationTransactionEditor
|
|||
|
||||
$state_map = PhabricatorTransactions::getInlineStateMap();
|
||||
|
||||
$query = id(clone $query_template)
|
||||
$inline_query = id(clone $query_template)
|
||||
->setViewer($this->getActor())
|
||||
->withFixedStates(array_keys($state_map));
|
||||
|
||||
$inlines = array();
|
||||
|
||||
$inlines[] = id(clone $query)
|
||||
->withAuthorPHIDs(array($actor_phid))
|
||||
->withHasTransaction(false)
|
||||
->execute();
|
||||
->withObjectPHIDs(array($object->getPHID()))
|
||||
->withFixedStates(array_keys($state_map))
|
||||
->withPublishableComments(true);
|
||||
|
||||
if ($actor_is_author) {
|
||||
$inlines[] = id(clone $query)
|
||||
->withHasTransaction(true)
|
||||
->execute();
|
||||
$inline_query->withPublishedComments(true);
|
||||
}
|
||||
|
||||
$inlines = array_mergev($inlines);
|
||||
|
||||
foreach ($inlines as $key => $inline) {
|
||||
if ($inline->isEmptyInlineComment()) {
|
||||
unset($inlines[$key]);
|
||||
continue;
|
||||
}
|
||||
}
|
||||
$inlines = $inline_query->execute();
|
||||
|
||||
if (!$inlines) {
|
||||
return null;
|
||||
|
|
|
@ -8,6 +8,7 @@ abstract class PhabricatorDiffInlineCommentQuery
|
|||
private $publishedComments;
|
||||
private $publishableComments;
|
||||
private $needHidden;
|
||||
private $needAppliedDrafts;
|
||||
|
||||
abstract protected function buildInlineCommentWhereClauseParts(
|
||||
AphrontDatabaseConnection $conn);
|
||||
|
@ -41,6 +42,11 @@ abstract class PhabricatorDiffInlineCommentQuery
|
|||
return $this;
|
||||
}
|
||||
|
||||
final public function needAppliedDrafts($need_applied) {
|
||||
$this->needAppliedDrafts = $need_applied;
|
||||
return $this;
|
||||
}
|
||||
|
||||
protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
|
||||
$where = parent::buildWhereClauseParts($conn);
|
||||
$alias = $this->getPrimaryTableAlias();
|
||||
|
@ -124,65 +130,121 @@ abstract class PhabricatorDiffInlineCommentQuery
|
|||
return $where;
|
||||
}
|
||||
|
||||
protected function willFilterPage(array $comments) {
|
||||
protected function willFilterPage(array $inlines) {
|
||||
$viewer = $this->getViewer();
|
||||
|
||||
if ($this->needReplyToComments) {
|
||||
$reply_phids = array();
|
||||
foreach ($comments as $comment) {
|
||||
$reply_phid = $comment->getReplyToCommentPHID();
|
||||
foreach ($inlines as $inline) {
|
||||
$reply_phid = $inline->getReplyToCommentPHID();
|
||||
if ($reply_phid) {
|
||||
$reply_phids[] = $reply_phid;
|
||||
}
|
||||
}
|
||||
|
||||
if ($reply_phids) {
|
||||
$reply_comments = newv(get_class($this), array())
|
||||
$reply_inlines = newv(get_class($this), array())
|
||||
->setViewer($this->getViewer())
|
||||
->setParentQuery($this)
|
||||
->withPHIDs($reply_phids)
|
||||
->execute();
|
||||
$reply_comments = mpull($reply_comments, null, 'getPHID');
|
||||
$reply_inlines = mpull($reply_inlines, null, 'getPHID');
|
||||
} else {
|
||||
$reply_comments = array();
|
||||
$reply_inlines = array();
|
||||
}
|
||||
|
||||
foreach ($comments as $key => $comment) {
|
||||
$reply_phid = $comment->getReplyToCommentPHID();
|
||||
foreach ($inlines as $key => $inline) {
|
||||
$reply_phid = $inline->getReplyToCommentPHID();
|
||||
if (!$reply_phid) {
|
||||
$comment->attachReplyToComment(null);
|
||||
$inline->attachReplyToComment(null);
|
||||
continue;
|
||||
}
|
||||
$reply = idx($reply_comments, $reply_phid);
|
||||
$reply = idx($reply_inlines, $reply_phid);
|
||||
if (!$reply) {
|
||||
$this->didRejectResult($comment);
|
||||
unset($comments[$key]);
|
||||
$this->didRejectResult($inline);
|
||||
unset($inlines[$key]);
|
||||
continue;
|
||||
}
|
||||
$comment->attachReplyToComment($reply);
|
||||
$inline->attachReplyToComment($reply);
|
||||
}
|
||||
}
|
||||
|
||||
if (!$comments) {
|
||||
return $comments;
|
||||
if (!$inlines) {
|
||||
return $inlines;
|
||||
}
|
||||
|
||||
if ($this->needHidden) {
|
||||
$viewer = $this->getViewer();
|
||||
$viewer_phid = $viewer->getPHID();
|
||||
|
||||
if ($viewer_phid) {
|
||||
$hidden = $this->loadHiddenCommentIDs(
|
||||
$viewer_phid,
|
||||
$comments);
|
||||
$inlines);
|
||||
} else {
|
||||
$hidden = array();
|
||||
}
|
||||
|
||||
foreach ($comments as $inline) {
|
||||
foreach ($inlines as $inline) {
|
||||
$inline->attachIsHidden(isset($hidden[$inline->getID()]));
|
||||
}
|
||||
}
|
||||
|
||||
return $comments;
|
||||
if (!$inlines) {
|
||||
return $inlines;
|
||||
}
|
||||
|
||||
$need_drafts = $this->needAppliedDrafts;
|
||||
$drop_void = $this->publishableComments;
|
||||
$convert_objects = ($need_drafts || $drop_void);
|
||||
|
||||
if ($convert_objects) {
|
||||
$inlines = mpull($inlines, 'newInlineCommentObject');
|
||||
|
||||
PhabricatorInlineComment::loadAndAttachVersionedDrafts(
|
||||
$viewer,
|
||||
$inlines);
|
||||
|
||||
if ($need_drafts) {
|
||||
// Don't count void inlines when considering draft state.
|
||||
foreach ($inlines as $key => $inline) {
|
||||
if ($inline->isVoidComment($viewer)) {
|
||||
$this->didRejectResult($inline->getStorageObject());
|
||||
unset($inlines[$key]);
|
||||
continue;
|
||||
}
|
||||
|
||||
// For other inlines: if they have a nonempty draft state, set their
|
||||
// content to the draft state content. We want to submit the comment
|
||||
// as it is currently shown to the user, not as it was stored the last
|
||||
// time they clicked "Save".
|
||||
|
||||
$draft_content = $inline->getContentForEdit($viewer);
|
||||
if (strlen($draft_content)) {
|
||||
$inline->setContent($draft_content);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// If we're loading publishable comments, discard any comments that are
|
||||
// empty.
|
||||
if ($drop_void) {
|
||||
foreach ($inlines as $key => $inline) {
|
||||
if ($inline->getTransactionPHID()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($inline->isVoidComment($viewer)) {
|
||||
$this->didRejectResult($inline->getStorageObject());
|
||||
unset($inlines[$key]);
|
||||
continue;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
$inlines = mpull($inlines, 'getStorageObject');
|
||||
}
|
||||
|
||||
return $inlines;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue