From b471f6c07aefaaa0029e7b94b03fb51c54da0074 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 11 Jan 2017 14:44:33 -0800 Subject: [PATCH] Order inline comments in Diffusion consistently with Differential Summary: Fixes T8739. Currently, Diffusion inline comments in the timeline are sorted arbitrarily, mostly by creation order. Instead, sort them by line number, like Differential. Test Plan: Made comments in "C", "B", "A" order, saw them in line order after submit: {F2343032} Reviewers: chad Reviewed By: chad Maniphest Tasks: T8739 Differential Revision: https://secure.phabricator.com/D17184 --- .../view/PhabricatorAuditTransactionView.php | 82 +++++++++++-------- 1 file changed, 46 insertions(+), 36 deletions(-) diff --git a/src/applications/audit/view/PhabricatorAuditTransactionView.php b/src/applications/audit/view/PhabricatorAuditTransactionView.php index ac5653e278..897440d9d7 100644 --- a/src/applications/audit/view/PhabricatorAuditTransactionView.php +++ b/src/applications/audit/view/PhabricatorAuditTransactionView.php @@ -81,48 +81,58 @@ final class PhabricatorAuditTransactionView } } - if ($inlines) { - - // TODO: This should do something similar to sortAndGroupInlines() to get - // a stable ordering. - - $inlines_by_path = array(); - foreach ($inlines as $key => $inline) { - $comment = $inline->getComment(); - if (!$comment) { - // TODO: Migrate these away? They probably do not exist on normal - // non-development installs. - unset($inlines[$key]); - continue; - } - $path_id = $comment->getPathID(); - $inlines_by_path[$path_id][] = $inline; + $structs = array(); + foreach ($inlines as $key => $inline) { + $comment = $inline->getComment(); + if (!$comment) { + // TODO: Migrate these away? They probably do not exist on normal + // non-development installs. + unset($inlines[$key]); + continue; } - $inline_view = new PhabricatorInlineSummaryView(); - foreach ($inlines_by_path as $path_id => $group) { - $path = idx($this->pathMap, $path_id); - if ($path === null) { - continue; - } - - $items = array(); - foreach ($group as $inline) { - $comment = $inline->getComment(); - $item = array( - 'id' => $comment->getID(), - 'line' => $comment->getLineNumber(), - 'length' => $comment->getLineLength(), - 'content' => parent::renderTransactionContent($inline), - ); - $items[] = $item; - } - $inline_view->addCommentGroup($path, $items); + $path_id = $comment->getPathID(); + $path = idx($this->pathMap, $path_id); + if ($path === null) { + continue; } - $out[] = $inline_view; + $structs[] = array( + 'inline' => $inline, + 'path' => $path, + 'sort' => (string)id(new PhutilSortVector()) + ->addString($path) + ->addInt($comment->getLineNumber()) + ->addInt($comment->getLineLength()) + ->addInt($inline->getID()), + ); } + if (!$structs) { + return $out; + } + + $structs = isort($structs, 'sort'); + $structs = igroup($structs, 'path'); + + $inline_view = new PhabricatorInlineSummaryView(); + foreach ($structs as $path => $group) { + $inlines = ipull($group, 'inline'); + $items = array(); + foreach ($inlines as $inline) { + $comment = $inline->getComment(); + $items[] = array( + 'id' => $comment->getID(), + 'line' => $comment->getLineNumber(), + 'length' => $comment->getLineLength(), + 'content' => parent::renderTransactionContent($inline), + ); + } + $inline_view->addCommentGroup($path, $items); + } + + $out[] = $inline_view; + return $out; }