mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-23 07:12:41 +01:00
Highlight audited paths in commit detail
Summary: This doesn't indicate which path is part of which package - I think it would be too heavy. It just highlights the paths in a similar way as audits are highlighted. Maybe we can use different colors for highlighting different packages and use them also in paths. We can mix the colors if one path is part of more packages :-). Test Plan: Viewed commit with 9 files and 4 packages where I am responsible only for one of them. Verified that the only file in my package is highlighted. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin, haugen Maniphest Tasks: T1226 Differential Revision: https://secure.phabricator.com/D2982
This commit is contained in:
parent
6dd448602d
commit
91d8b3c8ab
3 changed files with 68 additions and 19 deletions
|
@ -26,6 +26,8 @@ final class PhabricatorAuditListView extends AphrontView {
|
|||
private $user;
|
||||
private $showDescriptions = true;
|
||||
|
||||
private $highlightedAudits;
|
||||
|
||||
public function setAudits(array $audits) {
|
||||
assert_instances_of($audits, 'PhabricatorRepositoryAuditRequest');
|
||||
$this->audits = $audits;
|
||||
|
@ -98,11 +100,36 @@ final class PhabricatorAuditListView extends AphrontView {
|
|||
return $commit->getCommitData()->getSummary();
|
||||
}
|
||||
|
||||
public function getHighlightedAudits() {
|
||||
if ($this->highlightedAudits === null) {
|
||||
$this->highlightedAudits = array();
|
||||
|
||||
$user = $this->user;
|
||||
$authority = array_fill_keys($this->authorityPHIDs, true);
|
||||
|
||||
foreach ($this->audits as $audit) {
|
||||
$has_authority = !empty($authority[$audit->getAuditorPHID()]);
|
||||
if ($has_authority) {
|
||||
$commit_phid = $audit->getCommitPHID();
|
||||
$commit_author = $this->commits[$commit_phid]->getAuthorPHID();
|
||||
|
||||
// You don't have authority over package and project audits on your
|
||||
// own commits.
|
||||
|
||||
$auditor_is_user = ($audit->getAuditorPHID() == $user->getPHID());
|
||||
$user_is_author = ($commit_author == $user->getPHID());
|
||||
|
||||
if ($auditor_is_user || !$user_is_author) {
|
||||
$this->highlightedAudits[$audit->getID()] = $audit;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return $this->highlightedAudits;
|
||||
}
|
||||
|
||||
public function render() {
|
||||
$user = $this->user;
|
||||
|
||||
$authority = array_fill_keys($this->authorityPHIDs, true);
|
||||
|
||||
$rowc = array();
|
||||
|
||||
$last = null;
|
||||
|
@ -137,22 +164,9 @@ final class PhabricatorAuditListView extends AphrontView {
|
|||
);
|
||||
|
||||
$row_class = null;
|
||||
|
||||
$has_authority = !empty($authority[$audit->getAuditorPHID()]);
|
||||
if ($has_authority) {
|
||||
$commit_author = $this->commits[$commit_phid]->getAuthorPHID();
|
||||
|
||||
// You don't have authority over package and project audits on your own
|
||||
// commits.
|
||||
|
||||
$auditor_is_user = ($audit->getAuditorPHID() == $user->getPHID());
|
||||
$user_is_author = ($commit_author == $user->getPHID());
|
||||
|
||||
if ($auditor_is_user || !$user_is_author) {
|
||||
$row_class = 'highlighted';
|
||||
}
|
||||
if (array_key_exists($audit->getID(), $this->getHighlightedAudits())) {
|
||||
$row_class = 'highlighted';
|
||||
}
|
||||
|
||||
$rowc[] = $row_class;
|
||||
}
|
||||
|
||||
|
|
|
@ -21,6 +21,7 @@ final class DiffusionCommitController extends DiffusionController {
|
|||
const CHANGES_LIMIT = 100;
|
||||
|
||||
private $auditAuthorityPHIDs;
|
||||
private $highlightedAudits;
|
||||
|
||||
public function willProcessRequest(array $data) {
|
||||
// This controller doesn't use blob/path stuff, just pass the dictionary
|
||||
|
@ -119,9 +120,23 @@ final class DiffusionCommitController extends DiffusionController {
|
|||
$changes = array_slice($changes, 0, self::CHANGES_LIMIT);
|
||||
}
|
||||
|
||||
$owners_paths = array();
|
||||
if ($this->highlightedAudits) {
|
||||
$packages = id(new PhabricatorOwnersPackage())->loadAllWhere(
|
||||
'phid IN (%Ls)',
|
||||
mpull($this->highlightedAudits, 'getAuditorPHID'));
|
||||
if ($packages) {
|
||||
$owners_paths = id(new PhabricatorOwnersPath())->loadAllWhere(
|
||||
'repositoryPHID = %s AND packageID IN (%Ld)',
|
||||
$repository->getPHID(),
|
||||
mpull($packages, 'getID'));
|
||||
}
|
||||
}
|
||||
|
||||
$change_table = new DiffusionCommitChangeTableView();
|
||||
$change_table->setDiffusionRequest($drequest);
|
||||
$change_table->setPathChanges($changes);
|
||||
$change_table->setOwnersPaths($owners_paths);
|
||||
|
||||
$count = count($changes);
|
||||
|
||||
|
@ -404,6 +419,7 @@ final class DiffusionCommitController extends DiffusionController {
|
|||
$handles = id(new PhabricatorObjectHandleData($phids))->loadHandles();
|
||||
$view->setHandles($handles);
|
||||
$view->setAuthorityPHIDs($this->auditAuthorityPHIDs);
|
||||
$this->highlightedAudits = $view->getHighlightedAudits();
|
||||
|
||||
$panel = new AphrontPanelView();
|
||||
$panel->setHeader('Audits');
|
||||
|
|
|
@ -19,6 +19,7 @@
|
|||
final class DiffusionCommitChangeTableView extends DiffusionView {
|
||||
|
||||
private $pathChanges;
|
||||
private $ownersPaths = array();
|
||||
|
||||
public function setPathChanges(array $path_changes) {
|
||||
assert_instances_of($path_changes, 'DiffusionPathChange');
|
||||
|
@ -26,8 +27,15 @@ final class DiffusionCommitChangeTableView extends DiffusionView {
|
|||
return $this;
|
||||
}
|
||||
|
||||
public function setOwnersPaths(array $owners_paths) {
|
||||
assert_instances_of($owners_paths, 'PhabricatorOwnersPath');
|
||||
$this->ownersPaths = $owners_paths;
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function render() {
|
||||
$rows = array();
|
||||
$rowc = array();
|
||||
|
||||
// TODO: Experiment with path stack rendering.
|
||||
|
||||
|
@ -56,6 +64,16 @@ final class DiffusionCommitChangeTableView extends DiffusionView {
|
|||
$change->getPath()),
|
||||
$path_column,
|
||||
);
|
||||
|
||||
$row_class = null;
|
||||
foreach ($this->ownersPaths as $owners_path) {
|
||||
$owners_path = $owners_path->getPath();
|
||||
if (strncmp('/'.$path, $owners_path, strlen($owners_path)) == 0) {
|
||||
$row_class = 'highlighted';
|
||||
break;
|
||||
}
|
||||
}
|
||||
$rowc[] = $row_class;
|
||||
}
|
||||
|
||||
$view = new AphrontTableView($rows);
|
||||
|
@ -73,6 +91,7 @@ final class DiffusionCommitChangeTableView extends DiffusionView {
|
|||
'',
|
||||
'wide',
|
||||
));
|
||||
$view->setRowClasses($rowc);
|
||||
$view->setNoDataString('This change has not been fully parsed yet.');
|
||||
|
||||
return $view->render();
|
||||
|
|
Loading…
Reference in a new issue