Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15440295
D10109.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
19 KB
Referenced Files
None
Subscribers
None
D10109.diff
View Options
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1141,6 +1141,7 @@
'PhabricatorAuditCommitStatusConstants' => 'applications/audit/constants/PhabricatorAuditCommitStatusConstants.php',
'PhabricatorAuditController' => 'applications/audit/controller/PhabricatorAuditController.php',
'PhabricatorAuditDAO' => 'applications/audit/storage/PhabricatorAuditDAO.php',
+ 'PhabricatorAuditEditor' => 'applications/audit/editor/PhabricatorAuditEditor.php',
'PhabricatorAuditInlineComment' => 'applications/audit/storage/PhabricatorAuditInlineComment.php',
'PhabricatorAuditListController' => 'applications/audit/controller/PhabricatorAuditListController.php',
'PhabricatorAuditListView' => 'applications/audit/view/PhabricatorAuditListView.php',
@@ -3934,6 +3935,7 @@
'PhabricatorAuditCommentEditor' => 'PhabricatorEditor',
'PhabricatorAuditController' => 'PhabricatorController',
'PhabricatorAuditDAO' => 'PhabricatorLiskDAO',
+ 'PhabricatorAuditEditor' => 'PhabricatorApplicationTransactionEditor',
'PhabricatorAuditInlineComment' => 'PhabricatorInlineCommentInterface',
'PhabricatorAuditListController' => 'PhabricatorAuditController',
'PhabricatorAuditListView' => 'AphrontView',
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
@@ -12,9 +12,10 @@
}
$commit_phid = $request->getStr('commit');
- $commit = id(new PhabricatorRepositoryCommit())->loadOneWhere(
- 'phid = %s',
- $commit_phid);
+ $commit = id(new DiffusionCommitQuery())
+ ->setViewer($user)
+ ->withPHIDs(array($commit_phid))
+ ->executeOne();
if (!$commit) {
return new Aphront404Response();
}
@@ -36,12 +37,11 @@
));
break;
case PhabricatorAuditActionConstants::ADD_CCS:
- $ccs = $request->getArr('ccs');
$comments[] = id(new PhabricatorAuditComment())
- ->setAction(PhabricatorAuditActionConstants::ADD_CCS)
- ->setMetadata(
+ ->setAction(PhabricatorTransactions::TYPE_SUBSCRIBERS)
+ ->setNewValue(
array(
- PhabricatorAuditComment::METADATA_ADDED_CCS => $ccs,
+ '+' => $request->getArr('ccs'),
));
break;
case PhabricatorAuditActionConstants::COMMENT:
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
@@ -37,8 +37,13 @@
$ccs = $request->getStrList('ccs');
if ($action == PhabricatorAuditActionConstants::ADD_CCS && $ccs) {
- $action_xaction->setTransactionType($action);
- $action_xaction->setNewValue(array_fuse($ccs));
+ $action_xaction->setTransactionType(
+ PhabricatorTransactions::TYPE_SUBSCRIBERS);
+
+ // NOTE: This doesn't get processed before use, so just provide fake
+ // values.
+ $action_xaction->setOldValue(array());
+ $action_xaction->setNewValue($ccs);
}
$xactions[] = $action_xaction;
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
@@ -38,28 +38,6 @@
$commit->getPHID());
}
- $content_blocks = array();
- foreach ($comments as $comment) {
- $content_blocks[] = $comment->getContent();
- }
-
- foreach ($inline_comments as $inline) {
- $content_blocks[] = $inline->getContent();
- }
-
- // Find any "@mentions" in the content blocks.
- $mention_ccs = PhabricatorMarkupEngine::extractPHIDsFromMentions(
- $this->getActor(),
- $content_blocks);
- if ($mention_ccs) {
- $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
// they have authority over to reflect the most recent status. The general
// idea here is that if audit has triggered for, e.g., several packages, but
@@ -93,8 +71,6 @@
}
}
- $add_self_cc = false;
-
if ($action == PhabricatorAuditActionConstants::CLOSE) {
if (!PhabricatorEnv::getEnvConfig('audit.can-author-close-audit')) {
throw new Exception('Cannot Close Audit without enabling'.
@@ -181,7 +157,6 @@
case PhabricatorAuditActionConstants::COMMENT:
case PhabricatorAuditActionConstants::ADD_AUDITORS:
case PhabricatorAuditActionConstants::ADD_CCS:
- $add_self_cc = true;
break;
case PhabricatorAuditActionConstants::ACCEPT:
$new_status = PhabricatorAuditStatusConstants::ACCEPTED;
@@ -208,11 +183,6 @@
}
$auditors = array();
- $ccs = array();
-
- if ($add_self_cc) {
- $ccs[] = $actor->getPHID();
- }
foreach ($comments as $comment) {
$meta = $comment->getMetadata();
@@ -224,20 +194,11 @@
foreach ($auditor_phids as $phid) {
$auditors[] = $phid;
}
-
- $cc_phids = idx(
- $meta,
- PhabricatorAuditComment::METADATA_ADDED_CCS,
- array());
- foreach ($cc_phids as $phid) {
- $ccs[] = $phid;
- }
}
$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) {
@@ -256,38 +217,31 @@
$commit->updateAuditStatus($requests);
$commit->save();
- if ($ccs) {
- id(new PhabricatorSubscriptionsEditor())
- ->setActor($actor)
- ->setObject($commit)
- ->subscribeExplicit($ccs)
- ->save();
- }
-
- $content_source = PhabricatorContentSource::newForSource(
- PhabricatorContentSource::SOURCE_LEGACY,
- array());
+ // Convert old comments into real transactions and apply them with a
+ // normal editor.
+ $xactions = array();
foreach ($comments as $comment) {
- $comment
- ->setActorPHID($actor->getPHID())
- ->setTargetPHID($commit->getPHID())
- ->setContentSource($content_source)
- ->save();
+ $xactions[] = $comment->getTransactionForSave();
}
foreach ($inline_comments as $inline) {
- $xaction = id(new PhabricatorAuditComment())
- ->setProxyComment($inline->getTransactionCommentForSave())
- ->setAction(PhabricatorAuditActionConstants::INLINE)
- ->setActorPHID($actor->getPHID())
- ->setTargetPHID($commit->getPHID())
- ->setContentSource($content_source)
- ->save();
-
- $comments[] = $xaction;
+ $xactions[] = id(new PhabricatorAuditTransaction())
+ ->setTransactionType(PhabricatorAuditActionConstants::INLINE)
+ ->attachComment($inline->getTransactionComment());
}
+ $content_source = PhabricatorContentSource::newForSource(
+ PhabricatorContentSource::SOURCE_LEGACY,
+ array());
+
+ $editor = id(new PhabricatorAuditEditor())
+ ->setActor($actor)
+ ->setContinueOnNoEffect(true)
+ ->setContinueOnMissingFields(true)
+ ->setContentSource($content_source)
+ ->applyTransactions($commit, $xactions);
+
$feed_dont_publish_phids = array();
foreach ($requests as $request) {
$status = $request->getAuditStatus();
@@ -396,8 +350,6 @@
assert_instances_of($other_comments, 'PhabricatorAuditComment');
assert_instances_of($inline_comments, 'PhabricatorInlineCommentInterface');
- $any_comment = head($comments);
-
$commit = $this->commit;
$data = $commit->loadCommitData();
@@ -421,7 +373,12 @@
PhabricatorAuditActionConstants::ADD_CCS => 'Added CCs',
PhabricatorAuditActionConstants::ADD_AUDITORS => 'Added Auditors',
);
- $verb = idx($map, $any_comment->getAction(), 'Commented On');
+ if ($comments) {
+ $any_action = head($comments)->getAction();
+ } else {
+ $any_action = null;
+ }
+ $verb = idx($map, $any_action, 'Commented On');
$reply_handler = self::newReplyHandlerForCommit($commit);
@@ -444,7 +401,7 @@
$email_to = array();
$email_cc = array();
- $email_to[$any_comment->getActorPHID()] = true;
+ $email_to[$this->getActor()->getPHID()] = true;
$author_phid = $data->getCommitDetail('authorPHID');
if ($author_phid) {
@@ -493,7 +450,7 @@
->setSubject("{$name}: {$summary}")
->setSubjectPrefix($prefix)
->setVarySubjectPrefix("[{$verb}]")
- ->setFrom($any_comment->getActorPHID())
+ ->setFrom($this->getActor()->getPHID())
->setThreadID($thread_id, $is_new)
->addHeader('Thread-Topic', $thread_topic)
->setRelatedPHID($commit->getPHID())
diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php
new file mode 100644
--- /dev/null
+++ b/src/applications/audit/editor/PhabricatorAuditEditor.php
@@ -0,0 +1,114 @@
+<?php
+
+final class PhabricatorAuditEditor
+ extends PhabricatorApplicationTransactionEditor {
+
+ public function getTransactionTypes() {
+ $types = parent::getTransactionTypes();
+
+ $types[] = PhabricatorTransactions::TYPE_COMMENT;
+
+ // TODO: These will get modernized eventually, but that can happen one
+ // at a time later on.
+ $types[] = PhabricatorAuditActionConstants::ACTION;
+ $types[] = PhabricatorAuditActionConstants::INLINE;
+ $types[] = PhabricatorAuditActionConstants::ADD_AUDITORS;
+
+ return $types;
+ }
+
+ protected function transactionHasEffect(
+ PhabricatorLiskDAO $object,
+ PhabricatorApplicationTransaction $xaction) {
+
+ switch ($xaction->getTransactionType()) {
+ case PhabricatorAuditActionConstants::INLINE:
+ return $xaction->hasComment();
+ }
+
+ return parent::transactionHasEffect($object, $xaction);
+ }
+
+ protected function getCustomTransactionOldValue(
+ PhabricatorLiskDAO $object,
+ PhabricatorApplicationTransaction $xaction) {
+ switch ($xaction->getTransactionType()) {
+ case PhabricatorAuditActionConstants::ACTION:
+ case PhabricatorAuditActionConstants::INLINE:
+ return null;
+ case PhabricatorAuditActionConstants::ADD_AUDITORS:
+ // TODO: For now, just record the added PHIDs. Eventually, turn these
+ // into real edge transactions, probably?
+ return array();
+ }
+
+ return parent::getCustomTransactionOldValue($object, $xaction);
+ }
+
+ protected function getCustomTransactionNewValue(
+ PhabricatorLiskDAO $object,
+ PhabricatorApplicationTransaction $xaction) {
+
+ switch ($xaction->getTransactionType()) {
+ case PhabricatorAuditActionConstants::ACTION:
+ case PhabricatorAuditActionConstants::INLINE:
+ case PhabricatorAuditActionConstants::ADD_AUDITORS:
+ return $xaction->getNewValue();
+ }
+
+ return parent::getCustomTransactionNewValue($object, $xaction);
+ }
+
+ protected function applyCustomInternalTransaction(
+ PhabricatorLiskDAO $object,
+ PhabricatorApplicationTransaction $xaction) {
+
+ switch ($xaction->getTransactionType()) {
+ case PhabricatorTransactions::TYPE_COMMENT:
+ case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+ case PhabricatorAuditActionConstants::ACTION:
+ case PhabricatorAuditActionConstants::INLINE:
+ case PhabricatorAuditActionConstants::ADD_AUDITORS:
+ return;
+ }
+
+ return parent::applyCustomInternalTransaction($object, $xaction);
+ }
+
+ protected function applyCustomExternalTransaction(
+ PhabricatorLiskDAO $object,
+ PhabricatorApplicationTransaction $xaction) {
+
+ switch ($xaction->getTransactionType()) {
+ case PhabricatorTransactions::TYPE_COMMENT:
+ case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+ case PhabricatorAuditActionConstants::ACTION:
+ case PhabricatorAuditActionConstants::INLINE:
+ return;
+ case PhabricatorAuditActionConstants::ADD_AUDITORS:
+ // TODO: For now, these are applied externally.
+ return;
+ }
+
+ return parent::applyCustomExternalTransaction($object, $xaction);
+ }
+
+ protected function sortTransactions(array $xactions) {
+ $xactions = parent::sortTransactions($xactions);
+
+ $head = array();
+ $tail = array();
+
+ foreach ($xactions as $xaction) {
+ $type = $xaction->getTransactionType();
+ if ($type == PhabricatorAuditActionConstants::INLINE) {
+ $tail[] = $xaction;
+ } else {
+ $head[] = $xaction;
+ }
+ }
+
+ return array_values(array_merge($head, $tail));
+ }
+
+}
diff --git a/src/applications/audit/storage/PhabricatorAuditComment.php b/src/applications/audit/storage/PhabricatorAuditComment.php
--- a/src/applications/audit/storage/PhabricatorAuditComment.php
+++ b/src/applications/audit/storage/PhabricatorAuditComment.php
@@ -4,7 +4,6 @@
implements PhabricatorMarkupInterface {
const METADATA_ADDED_AUDITORS = 'added-auditors';
- const METADATA_ADDED_CCS = 'added-ccs';
const MARKUP_FIELD_BODY = 'markup:body';
@@ -114,6 +113,7 @@
switch ($action) {
case PhabricatorAuditActionConstants::INLINE:
case PhabricatorAuditActionConstants::ADD_CCS:
+ case PhabricatorTransactions::TYPE_SUBSCRIBERS:
case PhabricatorAuditActionConstants::ADD_AUDITORS:
$this->proxy->setTransactionType($action);
break;
@@ -137,6 +137,7 @@
return PhabricatorAuditActionConstants::COMMENT;
case PhabricatorAuditActionConstants::INLINE:
case PhabricatorAuditActionConstants::ADD_CCS:
+ case PhabricatorTransactions::TYPE_SUBSCRIBERS:
case PhabricatorAuditActionConstants::ADD_AUDITORS:
return $type;
default:
@@ -151,9 +152,6 @@
$type = $this->proxy->getTransactionType();
switch ($type) {
- case PhabricatorAuditActionConstants::ADD_CCS:
- $raw_phids = idx($metadata, self::METADATA_ADDED_CCS, array());
- break;
case PhabricatorAuditActionConstants::ADD_AUDITORS:
$raw_phids = idx($metadata, self::METADATA_ADDED_AUDITORS, array());
break;
@@ -161,7 +159,6 @@
throw new Exception(pht('No metadata expected!'));
}
- $this->proxy->setOldValue(array());
$this->proxy->setNewValue(array_fuse($raw_phids));
return $this;
@@ -175,10 +172,6 @@
$type = $this->proxy->getTransactionType();
$new_value = $this->proxy->getNewValue();
switch ($type) {
- case PhabricatorAuditActionConstants::ADD_CCS:
- return array(
- self::METADATA_ADDED_CCS => array_keys($new_value),
- );
case PhabricatorAuditActionConstants::ADD_AUDITORS:
return array(
self::METADATA_ADDED_AUDITORS => array_keys($new_value),
@@ -189,28 +182,21 @@
}
public function save() {
- $this->proxy->openTransaction();
- $this->proxy
- ->setViewPolicy('public')
- ->setEditPolicy($this->getActorPHID())
- ->save();
-
- if (strlen($this->getContent())) {
- $this->getProxyComment()
- ->setAuthorPHID($this->getActorPHID())
- ->setViewPolicy('public')
- ->setEditPolicy($this->getActorPHID())
- ->setCommentVersion(1)
- ->setTransactionPHID($this->proxy->getPHID())
- ->save();
+ throw new Exception(
+ pht('This object can no longer be written to directly!'));
+ }
- $this->proxy
- ->setCommentVersion(1)
- ->setCommentPHID($this->getProxyComment()->getPHID())
- ->save();
- }
- $this->proxy->saveTransaction();
+ public function getTransactionForSave() {
+ $xaction = $this->proxy;
+ if (strlen($this->getContent())) {
+ $xaction->attachComment($this->getProxyComment());
+ }
+
+ return $xaction;
+ }
+ public function setNewValue($value) {
+ $this->proxy->setNewValue($value);
return $this;
}
diff --git a/src/applications/audit/storage/PhabricatorAuditInlineComment.php b/src/applications/audit/storage/PhabricatorAuditInlineComment.php
--- a/src/applications/audit/storage/PhabricatorAuditInlineComment.php
+++ b/src/applications/audit/storage/PhabricatorAuditInlineComment.php
@@ -18,6 +18,10 @@
return $this->proxy->getTransactionPHID();
}
+ public function getTransactionComment() {
+ return $this->proxy;
+ }
+
public function getTransactionCommentForSave() {
$content_source = PhabricatorContentSource::newForSource(
PhabricatorContentSource::SOURCE_LEGACY,
diff --git a/src/applications/audit/storage/PhabricatorAuditTransaction.php b/src/applications/audit/storage/PhabricatorAuditTransaction.php
--- a/src/applications/audit/storage/PhabricatorAuditTransaction.php
+++ b/src/applications/audit/storage/PhabricatorAuditTransaction.php
@@ -83,7 +83,9 @@
switch ($type) {
case PhabricatorAuditActionConstants::INLINE:
- break;
+ return pht(
+ '%s added inline comments.',
+ $author_handle);
case PhabricatorAuditActionConstants::ADD_CCS:
if ($add && $rem) {
return pht(
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
@@ -40,10 +40,10 @@
protected function execute(ConduitAPIRequest $request) {
$commit_phid = $request->getValue('phid');
- $commit = id(new PhabricatorRepositoryCommit())->loadOneWhere(
- 'phid = %s',
- $commit_phid);
-
+ $commit = id(new DiffusionCommitQuery())
+ ->setViewer($request->getUser())
+ ->withPHIDs(array($commit_phid))
+ ->executeOne();
if (!$commit) {
throw new ConduitException('ERR_BAD_COMMIT');
}
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php
@@ -79,7 +79,7 @@
if ($xaction->getTransactionType() == $type_comment) {
if ($comment->getPHID()) {
throw new Exception(
- 'Transaction comment must not yet have a PHID!');
+ 'Transaction comment must not yet have a PHID!');
}
}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Thu, Mar 27, 11:58 AM (1 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7390300
Default Alt Text
D10109.diff (19 KB)
Attached To
Mode
D10109: Provide a transaction editor to perform Audit row writes
Attached
Detach File
Event Timeline
Log In to Comment