Page MenuHomePhabricator

D8476.diff
No OneTemporary

D8476.diff

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
@@ -345,7 +345,6 @@
'DifferentialChangesetViewController' => 'applications/differential/controller/DifferentialChangesetViewController.php',
'DifferentialComment' => 'applications/differential/storage/DifferentialComment.php',
'DifferentialCommentPreviewController' => 'applications/differential/controller/DifferentialCommentPreviewController.php',
- 'DifferentialCommentQuery' => 'applications/differential/query/DifferentialCommentQuery.php',
'DifferentialCommentSaveController' => 'applications/differential/controller/DifferentialCommentSaveController.php',
'DifferentialCommitMessageParser' => 'applications/differential/parser/DifferentialCommitMessageParser.php',
'DifferentialCommitMessageParserTestCase' => 'applications/differential/parser/__tests__/DifferentialCommitMessageParserTestCase.php',
@@ -354,11 +353,11 @@
'DifferentialController' => 'applications/differential/controller/DifferentialController.php',
'DifferentialCoreCustomField' => 'applications/differential/customfield/DifferentialCoreCustomField.php',
'DifferentialCustomField' => 'applications/differential/customfield/DifferentialCustomField.php',
- 'DifferentialCustomFieldDependsOnParser' => 'applications/differential/field/parser/DifferentialCustomFieldDependsOnParser.php',
- 'DifferentialCustomFieldDependsOnParserTestCase' => 'applications/differential/field/parser/__tests__/DifferentialCustomFieldDependsOnParserTestCase.php',
+ 'DifferentialCustomFieldDependsOnParser' => 'applications/differential/parser/DifferentialCustomFieldDependsOnParser.php',
+ 'DifferentialCustomFieldDependsOnParserTestCase' => 'applications/differential/parser/__tests__/DifferentialCustomFieldDependsOnParserTestCase.php',
'DifferentialCustomFieldNumericIndex' => 'applications/differential/storage/DifferentialCustomFieldNumericIndex.php',
- 'DifferentialCustomFieldRevertsParser' => 'applications/differential/field/parser/DifferentialCustomFieldRevertsParser.php',
- 'DifferentialCustomFieldRevertsParserTestCase' => 'applications/differential/field/parser/__tests__/DifferentialCustomFieldRevertsParserTestCase.php',
+ 'DifferentialCustomFieldRevertsParser' => 'applications/differential/parser/DifferentialCustomFieldRevertsParser.php',
+ 'DifferentialCustomFieldRevertsParserTestCase' => 'applications/differential/parser/__tests__/DifferentialCustomFieldRevertsParserTestCase.php',
'DifferentialCustomFieldStorage' => 'applications/differential/storage/DifferentialCustomFieldStorage.php',
'DifferentialCustomFieldStringIndex' => 'applications/differential/storage/DifferentialCustomFieldStringIndex.php',
'DifferentialDAO' => 'applications/differential/storage/DifferentialDAO.php',
@@ -374,10 +373,9 @@
'DifferentialDoorkeeperRevisionFeedStoryPublisher' => 'applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php',
'DifferentialDraft' => 'applications/differential/storage/DifferentialDraft.php',
'DifferentialEditPolicyField' => 'applications/differential/customfield/DifferentialEditPolicyField.php',
- 'DifferentialException' => 'applications/differential/exception/DifferentialException.php',
'DifferentialExceptionMail' => 'applications/differential/mail/DifferentialExceptionMail.php',
- 'DifferentialFieldParseException' => 'applications/differential/field/exception/DifferentialFieldParseException.php',
- 'DifferentialFieldValidationException' => 'applications/differential/field/exception/DifferentialFieldValidationException.php',
+ 'DifferentialFieldParseException' => 'applications/differential/exception/DifferentialFieldParseException.php',
+ 'DifferentialFieldValidationException' => 'applications/differential/exception/DifferentialFieldValidationException.php',
'DifferentialGetWorkingCopy' => 'applications/differential/DifferentialGetWorkingCopy.php',
'DifferentialGitSVNIDField' => 'applications/differential/customfield/DifferentialGitSVNIDField.php',
'DifferentialHostField' => 'applications/differential/customfield/DifferentialHostField.php',
@@ -442,7 +440,6 @@
'DifferentialStoredCustomField' => 'applications/differential/customfield/DifferentialStoredCustomField.php',
'DifferentialSubscribersField' => 'applications/differential/customfield/DifferentialSubscribersField.php',
'DifferentialSummaryField' => 'applications/differential/customfield/DifferentialSummaryField.php',
- 'DifferentialTasksAttacher' => 'applications/differential/DifferentialTasksAttacher.php',
'DifferentialTestPlanField' => 'applications/differential/customfield/DifferentialTestPlanField.php',
'DifferentialTitleField' => 'applications/differential/customfield/DifferentialTitleField.php',
'DifferentialTransaction' => 'applications/differential/storage/DifferentialTransaction.php',
@@ -2869,7 +2866,6 @@
'DifferentialChangesetViewController' => 'DifferentialController',
'DifferentialComment' => 'PhabricatorMarkupInterface',
'DifferentialCommentPreviewController' => 'DifferentialController',
- 'DifferentialCommentQuery' => 'PhabricatorOffsetPagedQuery',
'DifferentialCommentSaveController' => 'DifferentialController',
'DifferentialCommitMessageParserTestCase' => 'PhabricatorTestCase',
'DifferentialCommitsField' => 'DifferentialCustomField',
@@ -2902,7 +2898,6 @@
'DifferentialDoorkeeperRevisionFeedStoryPublisher' => 'DoorkeeperFeedStoryPublisher',
'DifferentialDraft' => 'DifferentialDAO',
'DifferentialEditPolicyField' => 'DifferentialCoreCustomField',
- 'DifferentialException' => 'Exception',
'DifferentialExceptionMail' => 'DifferentialMail',
'DifferentialFieldParseException' => 'Exception',
'DifferentialFieldValidationException' => 'Exception',
diff --git a/src/applications/differential/DifferentialTasksAttacher.php b/src/applications/differential/DifferentialTasksAttacher.php
deleted file mode 100644
--- a/src/applications/differential/DifferentialTasksAttacher.php
+++ /dev/null
@@ -1,24 +0,0 @@
-<?php
-
-abstract class DifferentialTasksAttacher {
- /**
- * Implementation of this function should attach given tasks to
- * the given revision. The function is called when 'arc' has task
- * ids defined in the commit message.
- */
- abstract public function attachTasksToRevision(
- $user_phid,
- DifferentialRevision $revision,
- array $task_ids);
-
- /**
- * This method will be called with a task and its original and new
- * associated revisions. Implementation of this method should update
- * the affected revisions to maintain the new associations.
- */
- abstract public function updateTaskRevisionAssoc(
- $task_phid,
- array $orig_rev_phids,
- array $new_rev_phids);
-
-}
diff --git a/src/applications/differential/conduit/ConduitAPI_differential_getrevisioncomments_Method.php b/src/applications/differential/conduit/ConduitAPI_differential_getrevisioncomments_Method.php
--- a/src/applications/differential/conduit/ConduitAPI_differential_getrevisioncomments_Method.php
+++ b/src/applications/differential/conduit/ConduitAPI_differential_getrevisioncomments_Method.php
@@ -32,6 +32,7 @@
}
protected function execute(ConduitAPIRequest $request) {
+ $viewer = $request->getUser();
$results = array();
$revision_ids = $request->getValue('ids');
@@ -40,7 +41,7 @@
}
$revisions = id(new DifferentialRevisionQuery())
- ->setViewer($request->getUser())
+ ->setViewer($viewer)
->withIDs($revision_ids)
->execute();
@@ -48,24 +49,36 @@
return $results;
}
- $comments = id(new DifferentialCommentQuery())
- ->withRevisionPHIDs(mpull($revisions, 'getPHID'))
+ $xactions = id(new DifferentialTransactionQuery())
+ ->setViewer($viewer)
+ ->withObjectPHIDs(mpull($revisions, 'getPHID'))
->execute();
$revisions = mpull($revisions, null, 'getPHID');
- foreach ($comments as $comment) {
- $revision = idx($revisions, $comment->getRevisionPHID());
+ foreach ($xactions as $xaction) {
+ $revision = idx($revisions, $xaction->getObjectPHID());
if (!$revision) {
continue;
}
+ $type = $xaction->getTransactionType();
+ if ($type == DifferentialTransaction::TYPE_ACTION) {
+ $action = $xaction->getNewValue();
+ } else if ($type == PhabricatorTransactions::TYPE_COMMENT) {
+ $action = 'comment';
+ } else {
+ $action = 'none';
+ }
+
$result = array(
'revisionID' => $revision->getID(),
- 'action' => $comment->getAction(),
- 'authorPHID' => $comment->getAuthorPHID(),
- 'dateCreated' => $comment->getDateCreated(),
- 'content' => $comment->getContent(),
+ 'action' => $action,
+ 'authorPHID' => $xaction->getAuthorPHID(),
+ 'dateCreated' => $xaction->getDateCreated(),
+ 'content' => ($xaction->hasComment()
+ ? $xaction->getComment()->getContent()
+ : null),
);
$results[$revision->getID()][] = $result;
diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -84,8 +84,6 @@
$target_manual->getID());
$props = mpull($props, 'getData', 'getName');
- $comments = $revision->loadComments();
-
$all_changesets = $changesets;
$inlines = $this->loadInlineComments(
$revision,
@@ -98,14 +96,7 @@
array(
$revision->getAuthorPHID(),
$user->getPHID(),
- ),
- mpull($comments, 'getAuthorPHID'));
-
- foreach ($comments as $comment) {
- foreach ($comment->getRequiredHandlePHIDs() as $phid) {
- $object_phids[] = $phid;
- }
- }
+ ));
foreach ($revision->getAttached() as $type => $phids) {
foreach ($phids as $phid => $info) {
diff --git a/src/applications/differential/exception/DifferentialException.php b/src/applications/differential/exception/DifferentialException.php
deleted file mode 100644
--- a/src/applications/differential/exception/DifferentialException.php
+++ /dev/null
@@ -1,5 +0,0 @@
-<?php
-
-abstract class DifferentialException extends Exception {
-
-}
diff --git a/src/applications/differential/field/exception/DifferentialFieldParseException.php b/src/applications/differential/exception/DifferentialFieldParseException.php
rename from src/applications/differential/field/exception/DifferentialFieldParseException.php
rename to src/applications/differential/exception/DifferentialFieldParseException.php
diff --git a/src/applications/differential/field/exception/DifferentialFieldValidationException.php b/src/applications/differential/exception/DifferentialFieldValidationException.php
rename from src/applications/differential/field/exception/DifferentialFieldValidationException.php
rename to src/applications/differential/exception/DifferentialFieldValidationException.php
diff --git a/src/applications/differential/field/parser/DifferentialCustomFieldDependsOnParser.php b/src/applications/differential/parser/DifferentialCustomFieldDependsOnParser.php
rename from src/applications/differential/field/parser/DifferentialCustomFieldDependsOnParser.php
rename to src/applications/differential/parser/DifferentialCustomFieldDependsOnParser.php
diff --git a/src/applications/differential/field/parser/DifferentialCustomFieldRevertsParser.php b/src/applications/differential/parser/DifferentialCustomFieldRevertsParser.php
rename from src/applications/differential/field/parser/DifferentialCustomFieldRevertsParser.php
rename to src/applications/differential/parser/DifferentialCustomFieldRevertsParser.php
diff --git a/src/applications/differential/field/parser/__tests__/DifferentialCustomFieldDependsOnParserTestCase.php b/src/applications/differential/parser/__tests__/DifferentialCustomFieldDependsOnParserTestCase.php
rename from src/applications/differential/field/parser/__tests__/DifferentialCustomFieldDependsOnParserTestCase.php
rename to src/applications/differential/parser/__tests__/DifferentialCustomFieldDependsOnParserTestCase.php
diff --git a/src/applications/differential/field/parser/__tests__/DifferentialCustomFieldRevertsParserTestCase.php b/src/applications/differential/parser/__tests__/DifferentialCustomFieldRevertsParserTestCase.php
rename from src/applications/differential/field/parser/__tests__/DifferentialCustomFieldRevertsParserTestCase.php
rename to src/applications/differential/parser/__tests__/DifferentialCustomFieldRevertsParserTestCase.php
diff --git a/src/applications/differential/query/DifferentialCommentQuery.php b/src/applications/differential/query/DifferentialCommentQuery.php
deleted file mode 100644
--- a/src/applications/differential/query/DifferentialCommentQuery.php
+++ /dev/null
@@ -1,34 +0,0 @@
-<?php
-
-/**
- * Temporary wrapper for transitioning Differential to ApplicationTransactions.
- */
-final class DifferentialCommentQuery
- extends PhabricatorOffsetPagedQuery {
-
- private $revisionPHIDs;
-
- public function withRevisionPHIDs(array $phids) {
- $this->revisionPHIDs = $phids;
- return $this;
- }
-
- public function execute() {
- // TODO: We're getting rid of this, it is the bads.
- $viewer = PhabricatorUser::getOmnipotentUser();
-
- $xactions = id(new DifferentialTransactionQuery())
- ->setViewer($viewer)
- ->withObjectPHIDs($this->revisionPHIDs)
- ->needComments(true)
- ->execute();
-
- $results = array();
- foreach ($xactions as $xaction) {
- $results[] = DifferentialComment::newFromModernTransaction($xaction);
- }
-
- return $results;
- }
-
-}
diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php
--- a/src/applications/differential/storage/DifferentialRevision.php
+++ b/src/applications/differential/storage/DifferentialRevision.php
@@ -160,15 +160,6 @@
DifferentialPHIDTypeRevision::TYPECONST);
}
- public function loadComments() {
- if (!$this->getID()) {
- return array();
- }
- return id(new DifferentialCommentQuery())
- ->withRevisionPHIDs(array($this->getPHID()))
- ->execute();
- }
-
public function loadActiveDiff() {
return id(new DifferentialDiff())->loadOneWhere(
'revisionID = %d ORDER BY id DESC LIMIT 1',
@@ -200,13 +191,6 @@
self::TABLE_COMMIT,
$this->getID());
- $comments = id(new DifferentialCommentQuery())
- ->withRevisionPHIDs(array($this->getPHID()))
- ->execute();
- foreach ($comments as $comment) {
- $comment->delete();
- }
-
$inlines = id(new DifferentialInlineCommentQuery())
->withRevisionIDs(array($this->getID()))
->execute();
@@ -295,27 +279,6 @@
return $last;
}
- public function loadReviewedBy() {
- $reviewer = null;
-
- if ($this->status == ArcanistDifferentialRevisionStatus::ACCEPTED ||
- $this->status == ArcanistDifferentialRevisionStatus::CLOSED) {
- $comments = $this->loadComments();
- foreach ($comments as $comment) {
- $action = $comment->getAction();
- if ($action == DifferentialAction::ACTION_ACCEPT) {
- $reviewer = $comment->getAuthorPHID();
- } else if ($action == DifferentialAction::ACTION_REJECT ||
- $action == DifferentialAction::ACTION_ABANDON ||
- $action == DifferentialAction::ACTION_RETHINK) {
- $reviewer = null;
- }
- }
- }
-
- return $reviewer;
- }
-
public function getHashes() {
return $this->assertAttached($this->hashes);
}
diff --git a/src/applications/herald/adapter/HeraldCommitAdapter.php b/src/applications/herald/adapter/HeraldCommitAdapter.php
--- a/src/applications/herald/adapter/HeraldCommitAdapter.php
+++ b/src/applications/herald/adapter/HeraldCommitAdapter.php
@@ -442,14 +442,14 @@
if (!$revision) {
return null;
}
- // after a revision is accepted, it can be closed (say via arc land)
- // so use this function to figure out if it was accepted at one point
- // *and* not later rejected... what a function!
- $reviewed_by = $revision->loadReviewedBy();
- if (!$reviewed_by) {
- return null;
+
+ switch ($revision->getStatus()) {
+ case ArcanistDifferentialRevisionStatus::ACCEPTED:
+ case ArcanistDifferentialRevisionStatus::CLOSED:
+ return $revision->getPHID();
}
- return $revision->getPHID();
+
+ return null;
case self::FIELD_DIFFERENTIAL_REVIEWERS:
$revision = $this->loadDifferentialRevision();
if (!$revision) {
diff --git a/src/applications/releeph/field/specification/ReleephDiffChurnFieldSpecification.php b/src/applications/releeph/field/specification/ReleephDiffChurnFieldSpecification.php
--- a/src/applications/releeph/field/specification/ReleephDiffChurnFieldSpecification.php
+++ b/src/applications/releeph/field/specification/ReleephDiffChurnFieldSpecification.php
@@ -23,24 +23,33 @@
}
$diff_rev = $this->getReleephRequest()->loadDifferentialRevision();
- $comments = id(new DifferentialCommentQuery())
- ->withRevisionPHIDs(array($diff_rev->getPHID()))
+
+ $xactions = id(new DifferentialTransactionQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->withObjectPHIDs(array($diff_rev->getPHID()))
->execute();
- $counts = array();
- foreach ($comments as $comment) {
- $action = $comment->getAction();
- if (!isset($counts[$action])) {
- $counts[$action] = 0;
+ $rejections = 0;
+ $comments = 0;
+ $updates = 0;
+ foreach ($xactions as $xaction) {
+ switch ($xaction->getTransactionType()) {
+ case PhabricatorTransactions::TYPE_COMMENT:
+ $comments++;
+ break;
+ case DifferentialTransaction::TYPE_UPDATE:
+ $updates++;
+ break;
+ case DifferentialTransaction::TYPE_ACTION:
+ switch ($xaction->getNewValue()) {
+ case DifferentialAction::ACTION_REJECT:
+ $rejections++;
+ break;
+ }
+ break;
}
- $counts[$action] += 1;
}
- // 'none' action just means a plain comment
- $comments = idx($counts, 'none', 0);
- $rejections = idx($counts, 'reject', 0);
- $updates = idx($counts, 'update', 0);
-
$points =
self::REJECTIONS_WEIGHT * $rejections +
self::COMMENTS_WEIGHT * $comments +
diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
--- a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
+++ b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
@@ -100,15 +100,10 @@
$revision = id(new DifferentialRevision())->load($revision_id);
if ($revision) {
$revision_author_phid = $revision->getAuthorPHID();
- $revision_reviewedby_phid = $revision->loadReviewedBy();
$commit_reviewedby_phid = $data->getCommitDetail('reviewerPHID');
if ($revision_author_phid !== $commit_author_phid) {
$reasons[] = "Author Not Matching with Revision";
}
- if ($revision_reviewedby_phid !== $commit_reviewedby_phid) {
- $reasons[] = "ReviewedBy Not Matching with Revision";
- }
-
} else {
$reasons[] = "Revision Not Found";
}
diff --git a/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php b/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php
--- a/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php
+++ b/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php
@@ -186,4 +186,14 @@
return phutil_utf8ize($string);
}
+ public function delete() {
+
+ // TODO: We should make some reasonable effort to destroy related
+ // infrastructure objects here, like edges, transactions, custom field
+ // storage, flags, Phrequent tracking, tokens, etc. This doesn't need to
+ // be exhaustive, but we can get a lot of it pretty easily.
+
+ return parent::delete();
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
Sat, Nov 9, 9:08 AM (1 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6713018
Default Alt Text
D8476.diff (20 KB)

Event Timeline