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 @@ -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 @@ -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(); + } + }