diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d63b99d4a2..5d917db2c0 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -547,6 +547,7 @@ phutil_register_library_map(array( 'DifferentialRevisionHeraldField' => 'applications/differential/herald/DifferentialRevisionHeraldField.php', 'DifferentialRevisionHeraldFieldGroup' => 'applications/differential/herald/DifferentialRevisionHeraldFieldGroup.php', 'DifferentialRevisionIDCommitMessageField' => 'applications/differential/field/DifferentialRevisionIDCommitMessageField.php', + 'DifferentialRevisionInlinesController' => 'applications/differential/controller/DifferentialRevisionInlinesController.php', 'DifferentialRevisionLandController' => 'applications/differential/controller/DifferentialRevisionLandController.php', 'DifferentialRevisionListController' => 'applications/differential/controller/DifferentialRevisionListController.php', 'DifferentialRevisionListView' => 'applications/differential/view/DifferentialRevisionListView.php', @@ -1738,6 +1739,7 @@ phutil_register_library_map(array( 'PHUIDiffInlineCommentTableScaffold' => 'infrastructure/diff/view/PHUIDiffInlineCommentTableScaffold.php', 'PHUIDiffInlineCommentUndoView' => 'infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php', 'PHUIDiffInlineCommentView' => 'infrastructure/diff/view/PHUIDiffInlineCommentView.php', + 'PHUIDiffInlineThreader' => 'infrastructure/diff/view/PHUIDiffInlineThreader.php', 'PHUIDiffOneUpInlineCommentRowScaffold' => 'infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php', 'PHUIDiffRevealIconView' => 'infrastructure/diff/view/PHUIDiffRevealIconView.php', 'PHUIDiffTableOfContentsItemView' => 'infrastructure/diff/view/PHUIDiffTableOfContentsItemView.php', @@ -5516,6 +5518,7 @@ phutil_register_library_map(array( 'DifferentialRevisionHeraldField' => 'HeraldField', 'DifferentialRevisionHeraldFieldGroup' => 'HeraldFieldGroup', 'DifferentialRevisionIDCommitMessageField' => 'DifferentialCommitMessageField', + 'DifferentialRevisionInlinesController' => 'DifferentialController', 'DifferentialRevisionLandController' => 'DifferentialController', 'DifferentialRevisionListController' => 'DifferentialController', 'DifferentialRevisionListView' => 'AphrontView', @@ -6872,6 +6875,7 @@ phutil_register_library_map(array( 'PHUIDiffInlineCommentTableScaffold' => 'AphrontView', 'PHUIDiffInlineCommentUndoView' => 'PHUIDiffInlineCommentView', 'PHUIDiffInlineCommentView' => 'AphrontView', + 'PHUIDiffInlineThreader' => 'Phobject', 'PHUIDiffOneUpInlineCommentRowScaffold' => 'PHUIDiffInlineCommentRowScaffold', 'PHUIDiffRevealIconView' => 'AphrontView', 'PHUIDiffTableOfContentsItemView' => 'AphrontView', diff --git a/src/applications/audit/storage/PhabricatorAuditInlineComment.php b/src/applications/audit/storage/PhabricatorAuditInlineComment.php index b330368d1b..2534384f61 100644 --- a/src/applications/audit/storage/PhabricatorAuditInlineComment.php +++ b/src/applications/audit/storage/PhabricatorAuditInlineComment.php @@ -331,6 +331,14 @@ final class PhabricatorAuditInlineComment return $this->isGhost; } + public function getDateModified() { + return $this->proxy->getDateModified(); + } + + public function getDateCreated() { + return $this->proxy->getDateCreated(); + } + /* -( PhabricatorMarkupInterface Implementation )-------------------------- */ diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php index c4d1b47c40..1ecfdaf557 100644 --- a/src/applications/base/controller/PhabricatorController.php +++ b/src/applications/base/controller/PhabricatorController.php @@ -483,8 +483,10 @@ abstract class PhabricatorController extends AphrontController { // NOTE: Applications (objects of class PhabricatorApplication) can't // currently be set here, although they don't need any of the extensions // anyway. This should probably work differently than it does, though. - if ($object instanceof PhabricatorLiskDAO) { - $action_list->setObject($object); + if ($object) { + if ($object instanceof PhabricatorLiskDAO) { + $action_list->setObject($object); + } } $curtain = id(new PHUICurtainView()) diff --git a/src/applications/differential/application/PhabricatorDifferentialApplication.php b/src/applications/differential/application/PhabricatorDifferentialApplication.php index 1b6443a9e0..403e60c8a9 100644 --- a/src/applications/differential/application/PhabricatorDifferentialApplication.php +++ b/src/applications/differential/application/PhabricatorDifferentialApplication.php @@ -77,6 +77,8 @@ final class PhabricatorDifferentialApplication extends PhabricatorApplication { => 'DifferentialDiffCreateController', 'operation/(?P[1-9]\d*)/' => 'DifferentialRevisionOperationController', + 'inlines/(?P[1-9]\d*)/' + => 'DifferentialRevisionInlinesController', ), 'comment/' => array( 'preview/(?P[1-9]\d*)/' => 'DifferentialCommentPreviewController', diff --git a/src/applications/differential/controller/DifferentialRevisionInlinesController.php b/src/applications/differential/controller/DifferentialRevisionInlinesController.php new file mode 100644 index 0000000000..c5eb063e99 --- /dev/null +++ b/src/applications/differential/controller/DifferentialRevisionInlinesController.php @@ -0,0 +1,190 @@ +getViewer(); + $id = $request->getURIData('id'); + + $revision = id(new DifferentialRevisionQuery()) + ->withIDs(array($id)) + ->setViewer($viewer) + ->needDiffIDs(true) + ->executeOne(); + if (!$revision) { + return new Aphront404Response(); + } + + $revision_monogram = $revision->getMonogram(); + $revision_uri = $revision->getURI(); + $revision_title = $revision->getTitle(); + + $query = id(new DifferentialInlineCommentQuery()) + ->setViewer($viewer) + ->needHidden(true) + ->withRevisionPHIDs(array($revision->getPHID())); + $inlines = $query->execute(); + + $crumbs = $this->buildApplicationCrumbs(); + $crumbs->addTextCrumb($revision_monogram, $revision_uri); + $crumbs->addTextCrumb(pht('Inline Comments')); + $crumbs->setBorder(true); + + $content = $this->renderInlineTable($revision, $inlines); + $header = $this->buildHeader($revision); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter($content); + + return $this->newPage() + ->setTitle( + array( + "{$revision_monogram} {$revision_title}", + pht('Inlines'), + )) + ->setCrumbs($crumbs) + ->appendChild($view); + } + + private function buildHeader(DifferentialRevision $revision) { + $viewer = $this->getViewer(); + + $button = id(new PHUIButtonView()) + ->setTag('a') + ->setIcon('fa-chevron-left') + ->setHref($revision->getURI()) + ->setText(pht('Back to Revision')); + + return id(new PHUIHeaderView()) + ->setHeader($revision->getTitle()) + ->setUser($viewer) + ->setHeaderIcon('fa-cog') + ->addActionLink($button); + } + + private function renderInlineTable( + DifferentialRevision $revision, + array $inlines) { + + $viewer = $this->getViewer(); + $inlines = id(new PHUIDiffInlineThreader()) + ->reorderAndThreadCommments($inlines); + + $handle_phids = array(); + $changeset_ids = array(); + foreach ($inlines as $inline) { + $handle_phids[] = $inline->getAuthorPHID(); + $changeset_ids[] = $inline->getChangesetID(); + } + $handles = $viewer->loadHandles($handle_phids); + $handles = iterator_to_array($handles); + + if ($changeset_ids) { + $changesets = id(new DifferentialChangesetQuery()) + ->setViewer($viewer) + ->withIDs($changeset_ids) + ->execute(); + $changesets = mpull($changesets, null, 'getID'); + } else { + $changesets = array(); + } + + $current_changeset = head($revision->getDiffIDs()); + + $rows = array(); + foreach ($inlines as $inline) { + $status_icons = array(); + + $c_id = $inline->getChangesetID(); + $d_id = $changesets[$c_id]->getDiffID(); + + if ($d_id == $current_changeset) { + $diff_id = phutil_tag('strong', array(), pht('Current')); + } else { + $diff_id = pht('Diff %d', $d_id); + } + + $reviewer = $handles[$inline->getAuthorPHID()]->renderLink(); + $now = PhabricatorTime::getNow(); + $then = $inline->getDateModified(); + $datetime = phutil_format_relative_time($now - $then); + + $comment_href = $revision->getURI().'#inline-'.$inline->getID(); + $comment = phutil_tag( + 'a', + array( + 'href' => $comment_href, + ), + $inline->getContent()); + + $state = $inline->getFixedState(); + if ($state == PhabricatorInlineCommentInterface::STATE_DONE) { + $status_icons[] = id(new PHUIIconView()) + ->setIcon('fa-check green') + ->addClass('mmr'); + } else if ($inline->getReplyToCommentPHID() && + $inline->getAuthorPHID() == $revision->getAuthorPHID()) { + $status_icons[] = id(new PHUIIconView()) + ->setIcon('fa-commenting-o blue') + ->addClass('mmr'); + } else { + $status_icons[] = id(new PHUIIconView()) + ->setIcon('fa-circle-o grey') + ->addClass('mmr'); + } + + + if ($inline->getReplyToCommentPHID()) { + $reply_icon = id(new PHUIIconView()) + ->setIcon('fa-reply mmr darkgrey'); + $comment = array($reply_icon, $comment); + } + + $rows[] = array( + $diff_id, + $status_icons, + $reviewer, + AphrontTableView::renderSingleDisplayLine($comment), + $datetime, + ); + } + + $table = new AphrontTableView($rows); + $table->setHeaders( + array( + pht('Diff'), + pht('Status'), + pht('Reviewer'), + pht('Comment'), + pht('Created'), + )); + $table->setColumnClasses( + array( + '', + '', + '', + 'wide', + 'right', + )); + $table->setColumnVisibility( + array( + true, + true, + true, + true, + true, + )); + + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Inline Comments')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->setTable($table); + } + +} diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index 69e45b377d..0f3675f8f6 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -574,6 +574,12 @@ final class DifferentialRevisionViewController extends DifferentialController { ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit)); + $curtain->addAction( + id(new PhabricatorActionView()) + ->setIcon('fa-indent') + ->setHref("/differential/revision/inlines/{$revision_id}/") + ->setName(pht('List Inline Comments'))); + $curtain->addAction( id(new PhabricatorActionView()) ->setIcon('fa-upload') diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php index c745d8e42a..88303cf420 100644 --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -1045,7 +1045,8 @@ final class DifferentialChangesetParser extends Phobject { } } - $this->comments = $this->reorderAndThreadComments($this->comments); + $this->comments = id(new PHUIDiffInlineThreader()) + ->reorderAndThreadCommments($this->comments); foreach ($this->comments as $comment) { $final = $comment->getLineNumber() + @@ -1617,68 +1618,6 @@ final class DifferentialChangesetParser extends Phobject { return array($old_back, $new_back); } - private function reorderAndThreadComments(array $comments) { - $comments = msort($comments, 'getID'); - - // Build an empty map of all the comments we actually have. If a comment - // is a reply but the parent has gone missing, we don't want it to vanish - // completely. - $comment_phids = mpull($comments, 'getPHID'); - $replies = array_fill_keys($comment_phids, array()); - - // Now, remove all comments which are replies, leaving only the top-level - // comments. - foreach ($comments as $key => $comment) { - $reply_phid = $comment->getReplyToCommentPHID(); - if (isset($replies[$reply_phid])) { - $replies[$reply_phid][] = $comment; - unset($comments[$key]); - } - } - - // For each top level comment, add the comment, then add any replies - // to it. Do this recursively so threads are shown in threaded order. - $results = array(); - foreach ($comments as $comment) { - $results[] = $comment; - $phid = $comment->getPHID(); - $descendants = $this->getInlineReplies($replies, $phid, 1); - foreach ($descendants as $descendant) { - $results[] = $descendant; - } - } - - // If we have anything left, they were cyclic references. Just dump - // them in a the end. This should be impossible, but users are very - // creative. - foreach ($replies as $phid => $comments) { - foreach ($comments as $comment) { - $results[] = $comment; - } - } - - return $results; - } - - private function getInlineReplies(array &$replies, $phid, $depth) { - $comments = idx($replies, $phid, array()); - unset($replies[$phid]); - - $results = array(); - foreach ($comments as $comment) { - $results[] = $comment; - $descendants = $this->getInlineReplies( - $replies, - $comment->getPHID(), - $depth + 1); - foreach ($descendants as $descendant) { - $results[] = $descendant; - } - } - - return $results; - } - private function getOffset(array $map, $line) { if (!$map) { return null; diff --git a/src/applications/differential/storage/DifferentialInlineComment.php b/src/applications/differential/storage/DifferentialInlineComment.php index bdc231671f..cbe05663db 100644 --- a/src/applications/differential/storage/DifferentialInlineComment.php +++ b/src/applications/differential/storage/DifferentialInlineComment.php @@ -255,6 +255,13 @@ final class DifferentialInlineComment return $this; } + public function getDateModified() { + return $this->proxy->getDateModified(); + } + + public function getDateCreated() { + return $this->proxy->getDateCreated(); + } /* -( PhabricatorMarkupInterface Implementation )-------------------------- */ diff --git a/src/infrastructure/diff/interface/PhabricatorInlineCommentInterface.php b/src/infrastructure/diff/interface/PhabricatorInlineCommentInterface.php index 13bf3ad83b..5c2bafdc86 100644 --- a/src/infrastructure/diff/interface/PhabricatorInlineCommentInterface.php +++ b/src/infrastructure/diff/interface/PhabricatorInlineCommentInterface.php @@ -60,4 +60,7 @@ interface PhabricatorInlineCommentInterface extends PhabricatorMarkupInterface { public function supportsHiding(); public function isHidden(); + public function getDateModified(); + public function getDateCreated(); + } diff --git a/src/infrastructure/diff/view/PHUIDiffInlineThreader.php b/src/infrastructure/diff/view/PHUIDiffInlineThreader.php new file mode 100644 index 0000000000..5209f1f359 --- /dev/null +++ b/src/infrastructure/diff/view/PHUIDiffInlineThreader.php @@ -0,0 +1,66 @@ + $comment) { + $reply_phid = $comment->getReplyToCommentPHID(); + if (isset($replies[$reply_phid])) { + $replies[$reply_phid][] = $comment; + unset($comments[$key]); + } + } + + // For each top level comment, add the comment, then add any replies + // to it. Do this recursively so threads are shown in threaded order. + $results = array(); + foreach ($comments as $comment) { + $results[] = $comment; + $phid = $comment->getPHID(); + $descendants = $this->getInlineReplies($replies, $phid, 1); + foreach ($descendants as $descendant) { + $results[] = $descendant; + } + } + + // If we have anything left, they were cyclic references. Just dump + // them in a the end. This should be impossible, but users are very + // creative. + foreach ($replies as $phid => $comments) { + foreach ($comments as $comment) { + $results[] = $comment; + } + } + + return $results; + } + + private function getInlineReplies(array &$replies, $phid, $depth) { + $comments = idx($replies, $phid, array()); + unset($replies[$phid]); + + $results = array(); + foreach ($comments as $comment) { + $results[] = $comment; + $descendants = $this->getInlineReplies( + $replies, + $comment->getPHID(), + $depth + 1); + foreach ($descendants as $descendant) { + $results[] = $descendant; + } + } + + return $results; + } +}