diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index e29082ea42..ec8eb4c4a8 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -270,6 +270,8 @@ phutil_register_library_map(array( 'DifferentialRevertPlanFieldSpecification' => 'applications/differential/field/specification/DifferentialRevertPlanFieldSpecification.php', 'DifferentialReviewRequestMail' => 'applications/differential/mail/DifferentialReviewRequestMail.php', 'DifferentialReviewedByFieldSpecification' => 'applications/differential/field/specification/DifferentialReviewedByFieldSpecification.php', + 'DifferentialReviewerStats' => 'applications/differential/stats/DifferentialReviewerStats.php', + 'DifferentialReviewerStatsTestCase' => 'applications/differential/stats/__tests__/DifferentialReviewerStatsTestCase.php', 'DifferentialReviewersFieldSpecification' => 'applications/differential/field/specification/DifferentialReviewersFieldSpecification.php', 'DifferentialRevision' => 'applications/differential/storage/DifferentialRevision.php', 'DifferentialRevisionCommentListView' => 'applications/differential/view/DifferentialRevisionCommentListView.php', @@ -1340,6 +1342,7 @@ phutil_register_library_map(array( 'DifferentialRevertPlanFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialReviewRequestMail' => 'DifferentialMail', 'DifferentialReviewedByFieldSpecification' => 'DifferentialFieldSpecification', + 'DifferentialReviewerStatsTestCase' => 'PhabricatorTestCase', 'DifferentialReviewersFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialRevision' => 'DifferentialDAO', 'DifferentialRevisionCommentListView' => 'AphrontView', diff --git a/src/applications/differential/stats/DifferentialReviewerStats.php b/src/applications/differential/stats/DifferentialReviewerStats.php new file mode 100644 index 0000000000..003def828d --- /dev/null +++ b/src/applications/differential/stats/DifferentialReviewerStats.php @@ -0,0 +1,137 @@ +since = $value; + return $this; + } + + public function setNow($value) { + $this->now = $value; + return $this; + } + + public function computeTimes( + DifferentialRevision $revision, + array $comments) { + assert_instances_of($comments, 'DifferentialComment'); + + $add_rev = DifferentialComment::METADATA_ADDED_REVIEWERS; + $rem_rev = DifferentialComment::METADATA_REMOVED_REVIEWERS; + + $date = $revision->getDateCreated(); + + // Find out original reviewers. + $reviewers = array_fill_keys($revision->getReviewers(), $date); + foreach (array_reverse($comments) as $comment) { + $metadata = $comment->getMetadata(); + foreach (idx($metadata, $add_rev, array()) as $phid) { + unset($reviewers[$phid]); + } + foreach (idx($metadata, $rem_rev, array()) as $phid) { + $reviewers[$phid] = $date; + } + } + + $reviewed = array(); + $not_reviewed = array(); + $status = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; + + foreach ($comments as $comment) { + $date = $comment->getDateCreated(); + $old_status = $status; + + switch ($comment->getAction()) { + case DifferentialAction::ACTION_UPDATE: + if ($status != ArcanistDifferentialRevisionStatus::CLOSED && + $status != ArcanistDifferentialRevisionStatus::ACCEPTED) { + $status = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; + } + break; + case DifferentialAction::ACTION_REQUEST: + case DifferentialAction::ACTION_RECLAIM: + $status = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; + break; + case DifferentialAction::ACTION_REJECT: + case DifferentialAction::ACTION_RETHINK: + $status = ArcanistDifferentialRevisionStatus::NEEDS_REVISION; + break; + case DifferentialAction::ACTION_ACCEPT: + $status = ArcanistDifferentialRevisionStatus::ACCEPTED; + break; + case DifferentialAction::ACTION_CLOSE: + $status = ArcanistDifferentialRevisionStatus::CLOSED; + break; + case DifferentialAction::ACTION_ABANDON: + $status = ArcanistDifferentialRevisionStatus::ABANDONED; + break; + } + + // Update current reviewers. + $metadata = $comment->getMetadata(); + foreach (idx($metadata, $add_rev, array()) as $phid) { + // If someone reviewed a revision without being its reviewer then give + // him zero response time. + $reviewers[$phid] = $date; + } + foreach (idx($metadata, $rem_rev, array()) as $phid) { + $start = idx($reviewers, $phid); + if ($start !== null) { + if ($date >= $this->since) { + $reviewed[$phid][] = $date - $start; + } + unset($reviewers[$phid]); + } + } + + // TODO: Respect workdays. + + if ($old_status != $status) { + if ($status == ArcanistDifferentialRevisionStatus::NEEDS_REVIEW) { + $reviewers = array_fill_keys(array_keys($reviewers), $date); + } else if ($date >= $this->since) { + if ($old_status == ArcanistDifferentialRevisionStatus::NEEDS_REVIEW) { + foreach ($reviewers as $phid => $start) { + if ($phid == $comment->getAuthorPHID()) { + $reviewed[$phid][] = $date - $start; + } else { + $not_reviewed[$phid][] = $date - $start; + } + } + } + } + } + } + + if ($status == ArcanistDifferentialRevisionStatus::NEEDS_REVIEW) { + $now = ($this->now !== null ? $this->now : time()); + if ($now >= $this->since) { + foreach ($reviewers as $phid => $start) { + $not_reviewed[$phid][] = $now - $start; + } + } + } + + return array($reviewed, $not_reviewed); + } + +} diff --git a/src/applications/differential/stats/__tests__/DifferentialReviewerStatsTestCase.php b/src/applications/differential/stats/__tests__/DifferentialReviewerStatsTestCase.php new file mode 100644 index 0000000000..532eb11dae --- /dev/null +++ b/src/applications/differential/stats/__tests__/DifferentialReviewerStatsTestCase.php @@ -0,0 +1,121 @@ +setDateCreated(1); + $revision->attachRelationships(array( + array('relation' => $revw, 'objectPHID' => 'R1'), + array('relation' => $revw, 'objectPHID' => 'R3'), + )); + + $comments = array( + $this->newComment(2, 'A', DifferentialAction::ACTION_COMMENT), + $this->newComment(4, 'A', DifferentialAction::ACTION_ADDREVIEWERS, + array(DifferentialComment::METADATA_ADDED_REVIEWERS => array('R3'))), + $this->newComment(8, 'R1', DifferentialAction::ACTION_REJECT), + $this->newComment(16, 'A', DifferentialAction::ACTION_COMMENT), + $this->newComment(32, 'A', DifferentialAction::ACTION_UPDATE), + $this->newComment(64, 'A', DifferentialAction::ACTION_UPDATE), + $this->newComment(128, 'A', DifferentialAction::ACTION_COMMENT), + $this->newComment(256, 'R2', DifferentialAction::ACTION_RESIGN, + array(DifferentialComment::METADATA_REMOVED_REVIEWERS => array('R2'))), + $this->newComment(512, 'R3', DifferentialAction::ACTION_ACCEPT), + $this->newComment(1024, 'A', DifferentialAction::ACTION_UPDATE), + // TODO: claim, abandon, reclaim + ); + + $stats = new DifferentialReviewerStats(); + list($reviewed, $not_reviewed) = $stats->computeTimes($revision, $comments); + + ksort($reviewed); + $this->assertEqual( + array( + 'R1' => array(8 - 1), + 'R2' => array(256 - 32), + 'R3' => array(512 - 32), + ), + $reviewed); + + ksort($not_reviewed); + $this->assertEqual( + array( + 'R1' => array(512 - 32), + 'R2' => array(8 - 1), + 'R3' => array(8 - 4), + ), + $not_reviewed); + } + + public function testReviewerStatsSince() { + $revision = new DifferentialRevision(); + $revision->setDateCreated(1); + $revision->attachRelationships(array( + array( + 'relation' => DifferentialRevision::RELATION_REVIEWER, + 'objectPHID' => 'R', + ), + )); + + $comments = array( + $this->newComment(2, 'R', DifferentialAction::ACTION_REJECT), + $this->newComment(4, 'A', DifferentialAction::ACTION_REQUEST), + $this->newComment(8, 'R', DifferentialAction::ACTION_ACCEPT), + ); + + $stats = new DifferentialReviewerStats(); + $stats->setSince(4); + list($reviewed, $not_reviewed) = $stats->computeTimes($revision, $comments); + + $this->assertEqual(array('R' => array(8 - 4)), $reviewed); + $this->assertEqual(array(), $not_reviewed); + } + + public function testReviewerStatsRequiresReview() { + $revision = new DifferentialRevision(); + $revision->setDateCreated(1); + $revision->attachRelationships(array( + array( + 'relation' => DifferentialRevision::RELATION_REVIEWER, + 'objectPHID' => 'R', + ), + )); + + $comments = array(); + + $stats = new DifferentialReviewerStats(); + $stats->setNow(2); + list($reviewed, $not_reviewed) = $stats->computeTimes($revision, $comments); + + $this->assertEqual(array(), $reviewed); + $this->assertEqual(array('R' => array(2 - 1)), $not_reviewed); + } + + private function newComment($date, $author, $action, $metadata = array()) { + return id(new DifferentialComment()) + ->setDateCreated($date) + ->setAuthorPHID($author) + ->setAction($action) + ->setMetadata($metadata); + } + +}