Page MenuHomePhabricator
Files F15446260

D10052.id24203.diff
No OneTemporary
Actions

D10052.id24203.diff
View Options

diff --git a/src/applications/audit/controller/PhabricatorAuditAddCommentController.php b/src/applications/audit/controller/PhabricatorAuditAddCommentController.php
--- a/src/applications/audit/controller/PhabricatorAuditAddCommentController.php
+++ b/src/applications/audit/controller/PhabricatorAuditAddCommentController.php
@@ -15,41 +15,55 @@
$commit = id(new PhabricatorRepositoryCommit())->loadOneWhere(
'phid = %s',
$commit_phid);
-
if (!$commit) {
return new Aphront404Response();
}
$phids = array($commit_phid);
- $action = $request->getStr('action');
-
- $comment = id(new PhabricatorAuditComment())
- ->setAction($action)
- ->setContent($request->getStr('content'));
+ $comments = array();
// make sure we only add auditors or ccs if the action matches
+ $action = $request->getStr('action');
switch ($action) {
- case 'add_auditors':
+ case PhabricatorAuditActionConstants::ADD_AUDITORS:
$auditors = $request->getArr('auditors');
- $ccs = array();
+ $comments[] = id(new PhabricatorAuditComment())
+ ->setAction(PhabricatorAuditActionConstants::ADD_AUDITORS)
+ ->setMetadata(
+ array(
+ PhabricatorAuditComment::METADATA_ADDED_AUDITORS => $auditors,
+ ));
break;
- case 'add_ccs':
- $auditors = array();
+ case PhabricatorAuditActionConstants::ADD_CCS:
$ccs = $request->getArr('ccs');
+ $comments[] = id(new PhabricatorAuditComment())
+ ->setAction(PhabricatorAuditActionConstants::ADD_CCS)
+ ->setMetadata(
+ array(
+ PhabricatorAuditComment::METADATA_ADDED_CCS => $ccs,
+ ));
+ break;
+ case PhabricatorAuditActionConstants::COMMENT:
+ // We'll deal with this below.
break;
default:
- $auditors = array();
- $ccs = array();
+ $comments[] = id(new PhabricatorAuditComment())
+ ->setAction($action);
break;
}
+ $content = $request->getStr('content');
+ if (strlen($content)) {
+ $comments[] = id(new PhabricatorAuditComment())
+ ->setAction(PhabricatorAuditActionConstants::COMMENT)
+ ->setContent($content);
+ }
+
id(new PhabricatorAuditCommentEditor($commit))
->setActor($user)
->setAttachInlineComments(true)
- ->addAuditors($auditors)
- ->addCCs($ccs)
- ->addComment($comment);
+ ->addComments($comments);
$handles = $this->loadViewerHandles($phids);
$uri = $handles[$commit_phid]->getURI();
diff --git a/src/applications/audit/controller/PhabricatorAuditPreviewController.php b/src/applications/audit/controller/PhabricatorAuditPreviewController.php
--- a/src/applications/audit/controller/PhabricatorAuditPreviewController.php
+++ b/src/applications/audit/controller/PhabricatorAuditPreviewController.php
@@ -20,57 +20,81 @@
$action = $request->getStr('action');
- $comment = id(new PhabricatorAuditComment())
- ->setActorPHID($user->getPHID())
- ->setTargetPHID($commit->getPHID())
- ->setAction($action)
- ->setContent($request->getStr('content'));
-
$phids = array(
$user->getPHID(),
$commit->getPHID(),
);
- $auditors = $request->getStrList('auditors');
- if ($action == PhabricatorAuditActionConstants::ADD_AUDITORS && $auditors) {
- $comment->setMetadata(array(
- PhabricatorAuditComment::METADATA_ADDED_AUDITORS => $auditors));
- $phids = array_merge($phids, $auditors);
+ $comments = array();
+
+ if ($action != PhabricatorAuditActionConstants::COMMENT) {
+ $action_comment = id(new PhabricatorAuditComment())
+ ->setActorPHID($user->getPHID())
+ ->setTargetPHID($commit->getPHID())
+ ->setAction($action);
+
+ $auditors = $request->getStrList('auditors');
+ if ($action == PhabricatorAuditActionConstants::ADD_AUDITORS &&
+ $auditors) {
+
+ $action_comment->setMetadata(array(
+ PhabricatorAuditComment::METADATA_ADDED_AUDITORS => $auditors));
+ $phids = array_merge($phids, $auditors);
+ }
+
+ $ccs = $request->getStrList('ccs');
+ if ($action == PhabricatorAuditActionConstants::ADD_CCS && $ccs) {
+ $action_comment->setMetadata(array(
+ PhabricatorAuditComment::METADATA_ADDED_CCS => $ccs));
+ $phids = array_merge($phids, $ccs);
+ }
+
+ $comments[] = $action_comment;
}
- $ccs = $request->getStrList('ccs');
- if ($action == PhabricatorAuditActionConstants::ADD_CCS && $ccs) {
- $comment->setMetadata(array(
- PhabricatorAuditComment::METADATA_ADDED_CCS => $ccs));
- $phids = array_merge($phids, $ccs);
+ $content = $request->getStr('content');
+ if (strlen($content)) {
+ $comments[] = id(new PhabricatorAuditComment())
+ ->setActorPHID($user->getPHID())
+ ->setTargetPHID($commit->getPHID())
+ ->setAction(PhabricatorAuditActionConstants::COMMENT)
+ ->setContent($content);
}
$engine = new PhabricatorMarkupEngine();
$engine->setViewer($user);
- $engine->addObject(
- $comment,
- PhabricatorAuditComment::MARKUP_FIELD_BODY);
+ foreach ($comments as $comment) {
+ $engine->addObject(
+ $comment,
+ PhabricatorAuditComment::MARKUP_FIELD_BODY);
+ }
$engine->process();
- $view = id(new DiffusionCommentView())
- ->setMarkupEngine($engine)
- ->setUser($user)
- ->setComment($comment)
- ->setIsPreview(true);
+ $views = array();
+ foreach ($comments as $comment) {
+ $view = id(new DiffusionCommentView())
+ ->setMarkupEngine($engine)
+ ->setUser($user)
+ ->setComment($comment)
+ ->setIsPreview(true);
- $phids = array_merge($phids, $view->getRequiredHandlePHIDs());
+ $phids = array_merge($phids, $view->getRequiredHandlePHIDs());
+ $views[] = $view;
+ }
$handles = $this->loadViewerHandles($phids);
- $view->setHandles($handles);
+
+ foreach ($views as $view) {
+ $view->setHandles($handles);
+ }
id(new PhabricatorDraft())
- ->setAuthorPHID($comment->getActorPHID())
+ ->setAuthorPHID($user->getPHID())
->setDraftKey('diffusion-audit-'.$this->id)
- ->setDraft($comment->getContent())
+ ->setDraft($content)
->replaceOrDelete();
- return id(new AphrontAjaxResponse())
- ->setContent($view->render());
+ return id(new AphrontAjaxResponse())->setContent(hsprintf('%s', $views));
}
}
diff --git a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
--- a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
+++ b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
@@ -3,11 +3,7 @@
final class PhabricatorAuditCommentEditor extends PhabricatorEditor {
private $commit;
-
private $attachInlineComments;
- private $auditors = array();
- private $ccs = array();
-
private $noEmail;
public function __construct(PhabricatorRepositoryCommit $commit) {
@@ -15,16 +11,6 @@
return $this;
}
- public function addAuditors(array $auditor_phids) {
- $this->auditors = array_merge($this->auditors, $auditor_phids);
- return $this;
- }
-
- public function addCCs(array $cc_phids) {
- $this->ccs = array_merge($this->ccs, $cc_phids);
- return $this;
- }
-
public function setAttachInlineComments($attach_inline_comments) {
$this->attachInlineComments = $attach_inline_comments;
return $this;
@@ -35,7 +21,8 @@
return $this;
}
- public function addComment(PhabricatorAuditComment $comment) {
+ public function addComments(array $comments) {
+ assert_instances_of($comments, 'PhabricatorAuditComment');
$commit = $this->commit;
$actor = $this->getActor();
@@ -51,38 +38,26 @@
$commit->getPHID());
}
- $comment
- ->setActorPHID($actor->getPHID())
- ->setTargetPHID($commit->getPHID())
- ->save();
+ $content_blocks = array();
+ foreach ($comments as $comment) {
+ $content_blocks[] = $comment->getContent();
+ }
- $content_blocks = array($comment->getContent());
foreach ($inline_comments as $inline) {
$content_blocks[] = $inline->getContent();
}
- $ccs = $this->ccs;
- $auditors = $this->auditors;
-
- $metadata = $comment->getMetadata();
- $metacc = array();
-
// Find any "@mentions" in the content blocks.
$mention_ccs = PhabricatorMarkupEngine::extractPHIDsFromMentions(
$this->getActor(),
$content_blocks);
if ($mention_ccs) {
- $metacc = idx(
- $metadata,
- PhabricatorAuditComment::METADATA_ADDED_CCS,
- array());
- foreach ($mention_ccs as $cc_phid) {
- $metacc[] = $cc_phid;
- }
- }
-
- if ($metacc) {
- $ccs = array_merge($ccs, $metacc);
+ $comments[] = id(new PhabricatorAuditComment())
+ ->setAction(PhabricatorAuditActionConstants::ADD_CCS)
+ ->setMetadata(
+ array(
+ PhabricatorAuditComment::METADATA_ADDED_CCS => $mention_ccs,
+ ));
}
// When an actor submits an audit comment, we update all the audit requests
@@ -101,17 +76,28 @@
$action = $comment->getAction();
-
// TODO: We should validate the action, currently we allow anyone to, e.g.,
// close an audit if they muck with form parameters. I'll followup with this
// and handle the no-effect cases (e.g., closing and already-closed audit).
-
$actor_is_author = ($actor->getPHID() == $commit->getAuthorPHID());
+ // Pick a meaningful action, if we have one.
+ $action = PhabricatorAuditActionConstants::COMMENT;
+ foreach ($comments as $comment) {
+ switch ($comment->getAction()) {
+ case PhabricatorAuditActionConstants::CLOSE:
+ case PhabricatorAuditActionConstants::RESIGN:
+ case PhabricatorAuditActionConstants::ACCEPT:
+ case PhabricatorAuditActionConstants::CONCERN:
+ $action = $comment->getAction();
+ break;
+ }
+ }
+
if ($action == PhabricatorAuditActionConstants::CLOSE) {
if (!PhabricatorEnv::getEnvConfig('audit.can-author-close-audit')) {
- throw new Exception('Cannot Close Audit without enabling'.
+ throw new Exception('Cannot Close Audit without enabling'.
'audit.can-author-close-audit');
}
// "Close" means wipe out all the concerns.
@@ -219,32 +205,33 @@
}
}
- $requests_by_auditor = mpull($requests, null, 'getAuditorPHID');
- $requests_phids = array_keys($requests_by_auditor);
-
- $ccs = array_diff($ccs, $requests_phids);
- $auditors = array_diff($auditors, $requests_phids);
+ $auditors = array();
+ $ccs = array();
+ foreach ($comments as $comment) {
+ $meta = $comment->getMetadata();
- if ($action == PhabricatorAuditActionConstants::ADD_CCS) {
- if ($ccs) {
- $metadata[PhabricatorAuditComment::METADATA_ADDED_CCS] = $ccs;
- $comment->setMetaData($metadata);
- } else {
- $comment->setAction(PhabricatorAuditActionConstants::COMMENT);
+ $auditor_phids = idx(
+ $meta,
+ PhabricatorAuditComment::METADATA_ADDED_AUDITORS,
+ array());
+ foreach ($auditor_phids as $phid) {
+ $auditors[] = $phid;
}
- }
- if ($action == PhabricatorAuditActionConstants::ADD_AUDITORS) {
- if ($auditors) {
- $metadata[PhabricatorAuditComment::METADATA_ADDED_AUDITORS]
- = $auditors;
- $comment->setMetaData($metadata);
- } else {
- $comment->setAction(PhabricatorAuditActionConstants::COMMENT);
+ $cc_phids = idx(
+ $meta,
+ PhabricatorAuditComment::METADATA_ADDED_CCS,
+ array());
+ foreach ($cc_phids as $phid) {
+ $ccs[] = $phid;
}
}
- $comment->save();
+ $requests_by_auditor = mpull($requests, null, 'getAuditorPHID');
+ $requests_phids = array_keys($requests_by_auditor);
+
+ $ccs = array_diff($ccs, $requests_phids);
+ $auditors = array_diff($auditors, $requests_phids);
if ($auditors) {
foreach ($auditors as $auditor_phid) {
@@ -275,7 +262,13 @@
$commit->updateAuditStatus($requests);
$commit->save();
- $comments = array($comment);
+ foreach ($comments as $comment) {
+ $comment
+ ->setActorPHID($actor->getPHID())
+ ->setTargetPHID($commit->getPHID())
+ ->save();
+ }
+
foreach ($inline_comments as $inline) {
$xaction = id(new PhabricatorAuditComment())
->setAction(PhabricatorAuditActionConstants::INLINE)
@@ -307,7 +300,9 @@
$feed_dont_publish_phids = array_keys($feed_dont_publish_phids);
$feed_phids = array_diff($requests_phids, $feed_dont_publish_phids);
- $this->publishFeedStory($comment, $feed_phids);
+ foreach ($comments as $comment) {
+ $this->publishFeedStory($comment, $feed_phids);
+ }
id(new PhabricatorSearchIndexer())
->queueDocumentForIndexing($commit->getPHID());
diff --git a/src/applications/audit/mail/PhabricatorAuditReplyHandler.php b/src/applications/audit/mail/PhabricatorAuditReplyHandler.php
--- a/src/applications/audit/mail/PhabricatorAuditReplyHandler.php
+++ b/src/applications/audit/mail/PhabricatorAuditReplyHandler.php
@@ -45,7 +45,7 @@
$editor->setActor($actor);
$editor->setExcludeMailRecipientPHIDs(
$this->getExcludeMailRecipientPHIDs());
- $editor->addComment($comment);
+ $editor->addComments(array($comment));
}
}
diff --git a/src/applications/diffusion/conduit/DiffusionCreateCommentConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionCreateCommentConduitAPIMethod.php
--- a/src/applications/diffusion/conduit/DiffusionCreateCommentConduitAPIMethod.php
+++ b/src/applications/diffusion/conduit/DiffusionCreateCommentConduitAPIMethod.php
@@ -69,14 +69,23 @@
throw new ConduitException('ERR_BAD_ACTION');
}
- $comment = id(new PhabricatorAuditComment())
- ->setAction($action)
- ->setContent($message);
+ $comments = array();
+
+ if ($action != PhabricatorAuditActionConstants::COMMENT) {
+ $comments[] = id(new PhabricatorAuditComment())
+ ->setAction($action);
+ }
+
+ if (strlen($message)) {
+ $comments[] = id(new PhabricatorAuditComment())
+ ->setAction(PhabricatorAuditActionConstants::COMMENT)
+ ->setContent($message);
+ }
id(new PhabricatorAuditCommentEditor($commit))
->setActor($request->getUser())
->setNoEmail($request->getValue('silent'))
- ->addComment($comment);
+ ->addComments($comments);
return true;
// get the full uri of the comment?

File Metadata

Mime Type
text/plain
Expires
Fri, Mar 28, 6:02 PM (5 d, 3 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7381627
Default Alt Text
D10052.id24203.diff (14 KB)

Event Timeline

Phabricator · Built by Phacility