diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -12,7 +12,7 @@ 'darkconsole.pkg.js' => 'df001cab', 'differential.pkg.css' => '4a93db37', 'differential.pkg.js' => '7528cfc9', - 'diffusion.pkg.css' => '471bc9eb', + 'diffusion.pkg.css' => '591664fa', 'diffusion.pkg.js' => 'bfc0737b', 'maniphest.pkg.css' => 'f5d89daf', 'maniphest.pkg.js' => 'df4aa49f', @@ -62,7 +62,6 @@ 'rsrc/css/application/differential/revision-history.css' => '0e8eb855', 'rsrc/css/application/differential/revision-list.css' => 'f3c47d33', 'rsrc/css/application/differential/table-of-contents.css' => '6bf8e1d2', - 'rsrc/css/application/diffusion/commit-view.css' => '92d1e8f9', 'rsrc/css/application/diffusion/diffusion-icons.css' => '9c5828da', 'rsrc/css/application/diffusion/diffusion-source.css' => '66fdf661', 'rsrc/css/application/feed/feed.css' => '4e544db4', @@ -534,7 +533,6 @@ 'differential-revision-history-css' => '0e8eb855', 'differential-revision-list-css' => 'f3c47d33', 'differential-table-of-contents-css' => '6bf8e1d2', - 'diffusion-commit-view-css' => '92d1e8f9', 'diffusion-icons-css' => '9c5828da', 'diffusion-source-css' => '66fdf661', 'diviner-shared-css' => '38813222', @@ -2137,7 +2135,6 @@ 'javelin-behavior-aphront-more', ), 'diffusion.pkg.css' => array( - 'diffusion-commit-view-css', 'diffusion-icons-css', ), 'diffusion.pkg.js' => array( diff --git a/resources/celerity/packages.php b/resources/celerity/packages.php --- a/resources/celerity/packages.php +++ b/resources/celerity/packages.php @@ -164,7 +164,6 @@ 'javelin-behavior-aphront-more', ), 'diffusion.pkg.css' => array( - 'diffusion-commit-view-css', 'diffusion-icons-css', ), 'diffusion.pkg.js' => array( 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 @@ -364,8 +364,6 @@ 'DiffusionBrowseSearchController' => 'applications/diffusion/controller/DiffusionBrowseSearchController.php', 'DiffusionBrowseTableView' => 'applications/diffusion/view/DiffusionBrowseTableView.php', 'DiffusionChangeController' => 'applications/diffusion/controller/DiffusionChangeController.php', - 'DiffusionCommentListView' => 'applications/diffusion/view/DiffusionCommentListView.php', - 'DiffusionCommentView' => 'applications/diffusion/view/DiffusionCommentView.php', 'DiffusionCommitBranchesController' => 'applications/diffusion/controller/DiffusionCommitBranchesController.php', 'DiffusionCommitChangeTableView' => 'applications/diffusion/view/DiffusionCommitChangeTableView.php', 'DiffusionCommitController' => 'applications/diffusion/controller/DiffusionCommitController.php', @@ -1153,6 +1151,7 @@ 'PhabricatorAuditTransaction' => 'applications/audit/storage/PhabricatorAuditTransaction.php', 'PhabricatorAuditTransactionComment' => 'applications/audit/storage/PhabricatorAuditTransactionComment.php', 'PhabricatorAuditTransactionQuery' => 'applications/audit/query/PhabricatorAuditTransactionQuery.php', + 'PhabricatorAuditTransactionView' => 'applications/audit/view/PhabricatorAuditTransactionView.php', 'PhabricatorAuthAccountView' => 'applications/auth/view/PhabricatorAuthAccountView.php', 'PhabricatorAuthApplication' => 'applications/auth/application/PhabricatorAuthApplication.php', 'PhabricatorAuthAuthFactorPHIDType' => 'applications/auth/phid/PhabricatorAuthAuthFactorPHIDType.php', @@ -3099,8 +3098,6 @@ 'DiffusionBrowseSearchController' => 'DiffusionBrowseController', 'DiffusionBrowseTableView' => 'DiffusionView', 'DiffusionChangeController' => 'DiffusionController', - 'DiffusionCommentListView' => 'AphrontView', - 'DiffusionCommentView' => 'AphrontView', 'DiffusionCommitBranchesController' => 'DiffusionController', 'DiffusionCommitChangeTableView' => 'DiffusionView', 'DiffusionCommitController' => 'DiffusionController', @@ -3944,6 +3941,7 @@ 'PhabricatorAuditTransaction' => 'PhabricatorApplicationTransaction', 'PhabricatorAuditTransactionComment' => 'PhabricatorApplicationTransactionComment', 'PhabricatorAuditTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'PhabricatorAuditTransactionView' => 'PhabricatorApplicationTransactionView', 'PhabricatorAuthAccountView' => 'AphrontView', 'PhabricatorAuthApplication' => 'PhabricatorApplication', 'PhabricatorAuthAuthFactorPHIDType' => 'PhabricatorPHIDType', diff --git a/src/applications/audit/controller/PhabricatorAuditPreviewController.php b/src/applications/audit/controller/PhabricatorAuditPreviewController.php --- a/src/applications/audit/controller/PhabricatorAuditPreviewController.php +++ b/src/applications/audit/controller/PhabricatorAuditPreviewController.php @@ -18,83 +18,66 @@ return new Aphront404Response(); } - $action = $request->getStr('action'); - - $phids = array( - $user->getPHID(), - $commit->getPHID(), - ); - - $comments = array(); + $xactions = array(); + $action = $request->getStr('action'); if ($action != PhabricatorAuditActionConstants::COMMENT) { - $action_comment = id(new PhabricatorAuditComment()) - ->setActorPHID($user->getPHID()) - ->setTargetPHID($commit->getPHID()) - ->setAction($action); + $action_xaction = id(new PhabricatorAuditTransaction()) + ->setAuthorPHID($user->getPHID()) + ->setObjectPHID($commit->getPHID()) + ->setTransactionType(PhabricatorAuditActionConstants::ACTION) + ->setNewValue($action); $auditors = $request->getStrList('auditors'); if ($action == PhabricatorAuditActionConstants::ADD_AUDITORS && $auditors) { - - $action_comment->setMetadata(array( - PhabricatorAuditComment::METADATA_ADDED_AUDITORS => $auditors)); - $phids = array_merge($phids, $auditors); + $action_xaction->setTransactionType($action); + $action_xaction->setNewValue(array_fuse($auditors)); } $ccs = $request->getStrList('ccs'); if ($action == PhabricatorAuditActionConstants::ADD_CCS && $ccs) { - $action_comment->setMetadata(array( - PhabricatorAuditComment::METADATA_ADDED_CCS => $ccs)); - $phids = array_merge($phids, $ccs); + $action_xaction->setTransactionType($action); + $action_xaction->setNewValue(array_fuse($ccs)); } - $comments[] = $action_comment; + $xactions[] = $action_xaction; } $content = $request->getStr('content'); if (strlen($content)) { - $comments[] = id(new PhabricatorAuditComment()) - ->setActorPHID($user->getPHID()) - ->setTargetPHID($commit->getPHID()) - ->setAction(PhabricatorAuditActionConstants::COMMENT) - ->setContent($content); + $xactions[] = id(new PhabricatorAuditTransaction()) + ->setAuthorPHID($user->getPHID()) + ->setObjectPHID($commit->getPHID()) + ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) + ->attachComment( + id(new PhabricatorAuditTransactionComment()) + ->setContent($content)); } - $engine = new PhabricatorMarkupEngine(); - $engine->setViewer($user); - foreach ($comments as $comment) { - $engine->addObject( - $comment, - PhabricatorAuditComment::MARKUP_FIELD_BODY); + $phids = array(); + foreach ($xactions as $xaction) { + $phids[] = $xaction->getRequiredHandlePHIDs(); } - $engine->process(); - - $views = array(); - foreach ($comments as $comment) { - $view = id(new DiffusionCommentView()) - ->setMarkupEngine($engine) - ->setUser($user) - ->setComment($comment) - ->setIsPreview(true); - - $phids = array_merge($phids, $view->getRequiredHandlePHIDs()); - $views[] = $view; - } - + $phids = array_mergev($phids); $handles = $this->loadViewerHandles($phids); - - foreach ($views as $view) { - $view->setHandles($handles); + foreach ($xactions as $xaction) { + $xaction->setHandles($handles); } + $view = id(new PhabricatorAuditTransactionView()) + ->setIsPreview(true) + ->setUser($user) + ->setObjectPHID($commit->getPHID()) + ->setTransactions($xactions); + id(new PhabricatorDraft()) ->setAuthorPHID($user->getPHID()) ->setDraftKey('diffusion-audit-'.$this->id) ->setDraft($content) ->replaceOrDelete(); - return id(new AphrontAjaxResponse())->setContent(hsprintf('%s', $views)); + return id(new AphrontAjaxResponse())->setContent(hsprintf('%s', $view)); } } diff --git a/src/applications/audit/storage/PhabricatorAuditTransaction.php b/src/applications/audit/storage/PhabricatorAuditTransaction.php --- a/src/applications/audit/storage/PhabricatorAuditTransaction.php +++ b/src/applications/audit/storage/PhabricatorAuditTransaction.php @@ -15,4 +15,144 @@ return new PhabricatorAuditTransactionComment(); } + public function getRequiredHandlePHIDs() { + $phids = parent::getRequiredHandlePHIDs(); + + $type = $this->getTransactionType(); + + switch ($type) { + case PhabricatorAuditActionConstants::ADD_CCS: + case PhabricatorAuditActionConstants::ADD_AUDITORS: + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + if (!is_array($old)) { + $old = array(); + } + if (!is_array($new)) { + $new = array(); + } + + foreach (array_keys($old + $new) as $phid) { + $phids[] = $phid; + } + break; + } + + return $phids; + } + + public function getColor() { + + $type = $this->getTransactionType(); + + switch ($type) { + case PhabricatorAuditActionConstants::ACTION: + switch ($this->getNewValue()) { + case PhabricatorAuditActionConstants::CONCERN: + return 'red'; + case PhabricatorAuditActionConstants::ACCEPT: + return 'green'; + } + } + + return parent::getColor(); + } + + public function getTitle() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + $author_handle = $this->getHandle($this->getAuthorPHID())->renderLink(); + + $type = $this->getTransactionType(); + + switch ($type) { + case PhabricatorAuditActionConstants::ADD_CCS: + case PhabricatorAuditActionConstants::ADD_AUDITORS: + if (!is_array($old)) { + $old = array(); + } + if (!is_array($new)) { + $new = array(); + } + $add = array_keys(array_diff_key($new, $old)); + $rem = array_keys(array_diff_key($old, $new)); + break; + } + + switch ($type) { + case PhabricatorAuditActionConstants::INLINE: + break; + case PhabricatorAuditActionConstants::ADD_CCS: + if ($add && $rem) { + return pht( + '%s edited subscribers; added: %s, removed: %s.', + $author_handle, + $this->renderHandleList($add), + $this->renderHandleList($rem)); + } else if ($add) { + return pht( + '%s added subscribers: %s.', + $author_handle, + $this->renderHandleList($add)); + } else if ($rem) { + return pht( + '%s removed subscribers: %s.', + $author_handle, + $this->renderHandleList($rem)); + } else { + return pht( + '%s added subscribers...', + $author_handle); + } + + case PhabricatorAuditActionConstants::ADD_AUDITORS: + if ($add && $rem) { + return pht( + '%s edited auditors; added: %s, removed: %s.', + $author_handle, + $this->renderHandleList($add), + $this->renderHandleList($rem)); + } else if ($add) { + return pht( + '%s added auditors: %s.', + $author_handle, + $this->renderHandleList($add)); + } else if ($rem) { + return pht( + '%s removed auditors: %s.', + $author_handle, + $this->renderHandleList($rem)); + } else { + return pht( + '%s added auditors...', + $author_handle); + } + + case PhabricatorAuditActionConstants::ACTION: + switch ($new) { + case PhabricatorAuditActionConstants::ACCEPT: + return pht( + '%s accepted this commit.', + $author_handle); + case PhabricatorAuditActionConstants::CONCERN: + return pht( + '%s raised a concern with this commit.', + $author_handle); + case PhabricatorAuditActionConstants::RESIGN: + return pht( + '%s resigned from this audit.', + $author_handle); + case PhabricatorAuditActionConstants::CLOSE: + return pht( + '%s closed this audit.', + $author_handle); + } + + } + + return parent::getTitle(); + } + } diff --git a/src/applications/audit/view/PhabricatorAuditTransactionView.php b/src/applications/audit/view/PhabricatorAuditTransactionView.php new file mode 100644 --- /dev/null +++ b/src/applications/audit/view/PhabricatorAuditTransactionView.php @@ -0,0 +1,124 @@ +pathMap = $path_map; + return $this; + } + + public function getPathMap() { + return $this->pathMap; + } + + // TODO: This shares a lot of code with Differential and Pholio and should + // probably be merged up. + + protected function shouldGroupTransactions( + PhabricatorApplicationTransaction $u, + PhabricatorApplicationTransaction $v) { + + if ($u->getAuthorPHID() != $v->getAuthorPHID()) { + // Don't group transactions by different authors. + return false; + } + + if (($v->getDateCreated() - $u->getDateCreated()) > 60) { + // Don't group if transactions that happened more than 60s apart. + return false; + } + + switch ($u->getTransactionType()) { + case PhabricatorTransactions::TYPE_COMMENT: + case PhabricatorAuditActionConstants::INLINE: + break; + default: + return false; + } + + switch ($v->getTransactionType()) { + case PhabricatorAuditActionConstants::INLINE: + return true; + } + + return parent::shouldGroupTransactions($u, $v); + } + + protected function renderTransactionContent( + PhabricatorApplicationTransaction $xaction) { + + $out = array(); + + $type_inline = PhabricatorAuditActionConstants::INLINE; + + $group = $xaction->getTransactionGroup(); + if ($xaction->getTransactionType() == $type_inline) { + array_unshift($group, $xaction); + } else { + $out[] = parent::renderTransactionContent($xaction); + } + + if (!$group) { + return $out; + } + + $inlines = array(); + foreach ($group as $xaction) { + switch ($xaction->getTransactionType()) { + case PhabricatorAuditActionConstants::INLINE: + $inlines[] = $xaction; + break; + default: + throw new Exception('Unknown grouped transaction type!'); + } + } + + 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; + } + + $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); + } + + $out[] = $inline_view; + } + + return $out; + } + +} diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -100,7 +100,6 @@ $engine = PhabricatorMarkupEngine::newDifferentialMarkupEngine(); $engine->setConfig('viewer', $user); - require_celerity_resource('diffusion-commit-view-css'); require_celerity_resource('phabricator-remarkup-css'); $parents = $this->callConduitWithDiffusionRequest( @@ -642,16 +641,23 @@ } private function buildComments(PhabricatorRepositoryCommit $commit) { - $user = $this->getRequest()->getUser(); - $comments = PhabricatorAuditComment::loadComments( - $user, - $commit->getPHID()); - - $inlines = PhabricatorAuditInlineComment::loadPublishedComments( - $user, - $commit->getPHID()); + $viewer = $this->getRequest()->getUser(); - $path_ids = mpull($inlines, 'getPathID'); + $xactions = id(new PhabricatorAuditTransactionQuery()) + ->setViewer($viewer) + ->withObjectPHIDs(array($commit->getPHID())) + ->needComments(true) + ->execute(); + + $path_ids = array(); + foreach ($xactions as $xaction) { + if ($xaction->hasComment()) { + $path_id = $xaction->getComment()->getPathID(); + if ($path_id) { + $path_ids[] = $path_id; + } + } + } $path_map = array(); if ($path_ids) { @@ -661,35 +667,11 @@ $path_map = ipull($path_map, 'path', 'id'); } - $engine = new PhabricatorMarkupEngine(); - $engine->setViewer($user); - - foreach ($comments as $comment) { - $engine->addObject( - $comment, - PhabricatorAuditComment::MARKUP_FIELD_BODY); - } - - foreach ($inlines as $inline) { - $engine->addObject( - $inline, - PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY); - } - - $engine->process(); - - $view = new DiffusionCommentListView(); - $view->setMarkupEngine($engine); - $view->setUser($user); - $view->setComments($comments); - $view->setInlineComments($inlines); - $view->setPathMap($path_map); - - $phids = $view->getRequiredHandlePHIDs(); - $handles = $this->loadViewerHandles($phids); - $view->setHandles($handles); - - return $view; + return id(new PhabricatorAuditTransactionView()) + ->setUser($viewer) + ->setObjectPHID($commit->getPHID()) + ->setPathMap($path_map) + ->setTransactions($xactions); } private function renderAddCommentPanel( diff --git a/src/applications/diffusion/view/DiffusionCommentListView.php b/src/applications/diffusion/view/DiffusionCommentListView.php deleted file mode 100644 --- a/src/applications/diffusion/view/DiffusionCommentListView.php +++ /dev/null @@ -1,96 +0,0 @@ -comments = $comments; - return $this; - } - - public function setInlineComments(array $inline_comments) { - assert_instances_of($inline_comments, 'PhabricatorInlineCommentInterface'); - $this->inlineComments = $inline_comments; - return $this; - } - - public function setPathMap(array $path_map) { - $this->pathMap = $path_map; - return $this; - } - - public function setMarkupEngine(PhabricatorMarkupEngine $markup_engine) { - $this->markupEngine = $markup_engine; - return $this; - } - - public function getMarkupEngine() { - return $this->markupEngine; - } - - public function getRequiredHandlePHIDs() { - $phids = array(); - foreach ($this->comments as $comment) { - $phids[$comment->getActorPHID()] = true; - $metadata = $comment->getMetaData(); - - $ccs_key = PhabricatorAuditComment::METADATA_ADDED_CCS; - $added_ccs = idx($metadata, $ccs_key, array()); - foreach ($added_ccs as $cc) { - $phids[$cc] = true; - } - $auditors_key = PhabricatorAuditComment::METADATA_ADDED_AUDITORS; - $added_auditors = idx($metadata, $auditors_key, array()); - foreach ($added_auditors as $auditor) { - $phids[$auditor] = true; - } - } - foreach ($this->inlineComments as $comment) { - $phids[$comment->getAuthorPHID()] = true; - } - return array_keys($phids); - } - - public function setHandles(array $handles) { - assert_instances_of($handles, 'PhabricatorObjectHandle'); - $this->handles = $handles; - return $this; - } - - public function render() { - - $inline_comments = mgroup($this->inlineComments, 'getTransactionPHID'); - - $num = 1; - - $comments = array(); - foreach ($this->comments as $comment) { - - $inlines = idx($inline_comments, $comment->getPHID(), array()); - - $view = id(new DiffusionCommentView()) - ->setMarkupEngine($this->getMarkupEngine()) - ->setComment($comment) - ->setInlineComments($inlines) - ->setCommentNumber($num) - ->setHandles($this->handles) - ->setPathMap($this->pathMap) - ->setUser($this->user); - - $comments[] = $view->render(); - ++$num; - } - - return phutil_tag( - 'div', - array('class' => 'diffusion-comment-list'), - $comments); - } - -} diff --git a/src/applications/diffusion/view/DiffusionCommentView.php b/src/applications/diffusion/view/DiffusionCommentView.php deleted file mode 100644 --- a/src/applications/diffusion/view/DiffusionCommentView.php +++ /dev/null @@ -1,209 +0,0 @@ -comment = $comment; - return $this; - } - - public function setCommentNumber($comment_number) { - $this->commentNumber = $comment_number; - return $this; - } - - public function setHandles(array $handles) { - assert_instances_of($handles, 'PhabricatorObjectHandle'); - $this->handles = $handles; - return $this; - } - - public function setIsPreview($is_preview) { - $this->isPreview = $is_preview; - return $this; - } - - public function setInlineComments(array $inline_comments) { - assert_instances_of($inline_comments, 'PhabricatorInlineCommentInterface'); - $this->inlineComments = $inline_comments; - return $this; - } - - public function setPathMap(array $path_map) { - $this->pathMap = $path_map; - return $this; - } - - public function setMarkupEngine(PhabricatorMarkupEngine $markup_engine) { - $this->markupEngine = $markup_engine; - return $this; - } - - public function getMarkupEngine() { - return $this->markupEngine; - } - - public function getRequiredHandlePHIDs() { - return array($this->comment->getActorPHID()); - } - - private function getHandle($phid) { - if (empty($this->handles[$phid])) { - throw new Exception("Unloaded handle '{$phid}'!"); - } - return $this->handles[$phid]; - } - - public function render() { - $comment = $this->comment; - $author = $this->getHandle($comment->getActorPHID()); - $author_link = $author->renderLink(); - - $actions = $this->renderActions(); - $content = $this->renderContent(); - $classes = $this->renderClasses(); - - $xaction_view = id(new PhabricatorTransactionView()) - ->setUser($this->user) - ->setImageURI($author->getImageURI()) - ->setActions($actions) - ->appendChild($content); - - if ($this->isPreview) { - $xaction_view->setIsPreview(true); - } else { - $xaction_view - ->setAnchor('comment-'.$this->commentNumber, '#'.$this->commentNumber) - ->setEpoch($comment->getDateCreated()); - } - - foreach ($classes as $class) { - $xaction_view->addClass($class); - } - - return $xaction_view->render(); - } - - private function renderActions() { - $comment = $this->comment; - $author = $this->getHandle($comment->getActorPHID()); - $author_link = $author->renderLink(); - - $action = $comment->getAction(); - $verb = PhabricatorAuditActionConstants::getActionPastTenseVerb($action); - - $metadata = $comment->getMetadata(); - $added_auditors = idx( - $metadata, - PhabricatorAuditComment::METADATA_ADDED_AUDITORS, - array()); - $added_ccs = idx( - $metadata, - PhabricatorAuditComment::METADATA_ADDED_CCS, - array()); - - $actions = array(); - if ($action == PhabricatorAuditActionConstants::ADD_CCS) { - $rendered_ccs = $this->renderHandleList($added_ccs); - $actions[] = pht('%s added CCs: %s.', $author_link, $rendered_ccs); - } else if ($action == PhabricatorAuditActionConstants::ADD_AUDITORS) { - $rendered_auditors = $this->renderHandleList($added_auditors); - $actions[] = pht( - '%s added auditors: %s.', - $author_link, - $rendered_auditors); - } else { - $actions[] = hsprintf('%s %s this commit.', $author_link, $verb); - } - - foreach ($actions as $key => $action) { - $actions[$key] = phutil_tag('div', array(), $action); - } - - return $actions; - } - - private function renderContent() { - $comment = $this->comment; - $engine = $this->getMarkupEngine(); - - if (!strlen($comment->getContent()) && empty($this->inlineComments)) { - return null; - } else { - return phutil_tag_div('phabricator-remarkup', array( - $engine->getOutput( - $comment, - PhabricatorAuditComment::MARKUP_FIELD_BODY), - $this->renderInlines(), - )); - } - } - - private function renderInlines() { - if (!$this->inlineComments) { - return null; - } - - $engine = $this->getMarkupEngine(); - - $inlines_by_path = mgroup($this->inlineComments, 'getPathID'); - - $view = new PhabricatorInlineSummaryView(); - foreach ($inlines_by_path as $path_id => $inlines) { - $path = idx($this->pathMap, $path_id); - if ($path === null) { - continue; - } - - $items = array(); - foreach ($inlines as $inline) { - $items[] = array( - 'id' => $inline->getID(), - 'line' => $inline->getLineNumber(), - 'length' => $inline->getLineLength(), - 'content' => $engine->getOutput( - $inline, - PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY), - ); - } - - $view->addCommentGroup($path, $items); - } - - return $view; - } - - private function renderHandleList(array $phids) { - $result = array(); - foreach ($phids as $phid) { - $result[] = $this->handles[$phid]->renderLink(); - } - return phutil_implode_html(', ', $result); - } - - private function renderClasses() { - $comment = $this->comment; - - $classes = array(); - switch ($comment->getAction()) { - case PhabricatorAuditActionConstants::ACCEPT: - $classes[] = 'audit-accept'; - break; - case PhabricatorAuditActionConstants::CONCERN: - $classes[] = 'audit-concern'; - break; - } - - return $classes; - } - -} diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -264,9 +264,10 @@ if (empty($this->handles[$phid])) { throw new Exception( pht( - 'Transaction ("%s") requires a handle ("%s") that it did not '. - 'load.', + 'Transaction ("%s", of type "%s") requires a handle ("%s") that it '. + 'did not load.', $this->getPHID(), + $this->getTransactionType(), $phid)); } return $this->handles[$phid]; diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php --- a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php @@ -159,17 +159,7 @@ } if ($this->getShowEditActions()) { - $list_id = celerity_generate_unique_node_id(); - - $view->setID($list_id); - - Javelin::initBehavior( - 'phabricator-transaction-list', - array( - 'listID' => $list_id, - 'objectPHID' => $this->getObjectPHID(), - 'nextAnchor' => $this->anchorOffset + count($events), - )); + Javelin::initBehavior('phabricator-transaction-list'); } return $view->render(); diff --git a/src/view/phui/PHUITimelineEventView.php b/src/view/phui/PHUITimelineEventView.php --- a/src/view/phui/PHUITimelineEventView.php +++ b/src/view/phui/PHUITimelineEventView.php @@ -495,8 +495,20 @@ $xaction_phid = $this->getTransactionPHID(); $items = array(); - if ($this->getQuoteTargetID()) { + if ($this->getIsEditable()) { + $items[] = id(new PhabricatorActionView()) + ->setIcon('fa-pencil') + ->setHref('/transactions/edit/'.$xaction_phid.'/') + ->setName(pht('Edit Comment')) + ->addSigil('transaction-edit') + ->setMetadata( + array( + 'anchor' => $anchor, + )); + } + + if ($this->getQuoteTargetID()) { $ref = null; if ($this->getQuoteRef()) { $ref = $this->getQuoteRef(); @@ -505,18 +517,6 @@ } } - if ($this->getIsEditable()) { - $items[] = id(new PhabricatorActionView()) - ->setIcon('fa-pencil') - ->setHref('/transactions/edit/'.$xaction_phid.'/') - ->setName(pht('Edit Comment')) - ->addSigil('transaction-edit') - ->setMetadata( - array( - 'anchor' => $anchor, - )); - } - $items[] = id(new PhabricatorActionView()) ->setIcon('fa-quote-left') ->setHref('#') diff --git a/webroot/rsrc/css/application/diffusion/commit-view.css b/webroot/rsrc/css/application/diffusion/commit-view.css deleted file mode 100644 --- a/webroot/rsrc/css/application/diffusion/commit-view.css +++ /dev/null @@ -1,15 +0,0 @@ -/** - * @provides diffusion-commit-view-css - */ - -.diffusion-comment-list { - margin: 2em; -} - -.phabricator-transaction-view .audit-accept { - border-color: #009933; -} - -.phabricator-transaction-view .audit-concern { - border-color: #aa0000; -}