1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 00:42:41 +01:00

Improve Differential handling of disabled users

Summary:
We currently allow you to assign code review to disabled users, but
should not.

Test Plan:
  - Created revisions with no reviewers and only disabled reviewers, was
appropriately warned.
  - Looked at a disabled user handle link, was clearly informed.
  - Tried to create a new revision with a disabled reviewer, was rebuffed.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D1429
This commit is contained in:
epriestley 2012-01-16 16:54:05 -08:00
parent 4faab06c3c
commit e2c75d5dc2
7 changed files with 118 additions and 75 deletions

View file

@ -330,6 +330,17 @@ celerity_register_resource_map(array(
),
'disk' => '/rsrc/js/javelin/lib/behavior.js',
),
0 =>
array(
'uri' => '/res/b6096fdd/rsrc/js/javelin/lib/__tests__/URI.js',
'type' => 'js',
'requires' =>
array(
0 => 'javelin-uri',
1 => 'javelin-php-serializer',
),
'disk' => '/rsrc/js/javelin/lib/__tests__/URI.js',
),
'javelin-behavior-aphront-basic-tokenizer' =>
array(
'uri' => '/res/9be30797/rsrc/js/application/core/behavior-tokenizer.js',
@ -1427,6 +1438,18 @@ celerity_register_resource_map(array(
),
'disk' => '/rsrc/css/application/objectselector/object-selector.css',
),
'phabricator-prefab' =>
array(
'uri' => '/res/5784a112/rsrc/js/application/core/Prefab.js',
'type' => 'js',
'requires' =>
array(
0 => 'javelin-install',
1 => 'javelin-util',
2 => 'javelin-dom',
),
'disk' => '/rsrc/js/application/core/Prefab.js',
),
'phabricator-profile-css' =>
array(
'uri' => '/res/9869d10b/rsrc/css/application/profile/profile-view.css',
@ -1445,29 +1468,6 @@ celerity_register_resource_map(array(
),
'disk' => '/rsrc/css/application/profile/profile-header-view.css',
),
0 =>
array(
'uri' => '/res/b6096fdd/rsrc/js/javelin/lib/__tests__/URI.js',
'type' => 'js',
'requires' =>
array(
0 => 'javelin-uri',
1 => 'javelin-php-serializer',
),
'disk' => '/rsrc/js/javelin/lib/__tests__/URI.js',
),
'phabricator-prefab' =>
array(
'uri' => '/res/5784a112/rsrc/js/application/core/Prefab.js',
'type' => 'js',
'requires' =>
array(
0 => 'javelin-install',
1 => 'javelin-util',
2 => 'javelin-dom',
),
'disk' => '/rsrc/js/application/core/Prefab.js',
),
'phabricator-remarkup-css' =>
array(
'uri' => '/res/39f358b8/rsrc/css/core/remarkup.css',
@ -1509,7 +1509,7 @@ celerity_register_resource_map(array(
),
'phabricator-standard-page-view' =>
array(
'uri' => '/res/827b93f8/rsrc/css/application/base/standard-page-view.css',
'uri' => '/res/ec5acaed/rsrc/css/application/base/standard-page-view.css',
'type' => 'css',
'requires' =>
array(
@ -1784,30 +1784,6 @@ celerity_register_resource_map(array(
'uri' => '/res/pkg/b164acea/javelin.pkg.js',
'type' => 'js',
),
'bb4d65a4' =>
array(
'name' => 'core.pkg.css',
'symbols' =>
array(
0 => 'phabricator-core-css',
1 => 'phabricator-core-buttons-css',
2 => 'phabricator-standard-page-view',
3 => 'aphront-dialog-view-css',
4 => 'aphront-form-view-css',
5 => 'aphront-panel-view-css',
6 => 'aphront-side-nav-view-css',
7 => 'aphront-table-view-css',
8 => 'aphront-crumbs-view-css',
9 => 'aphront-tokenizer-control-css',
10 => 'aphront-typeahead-control-css',
11 => 'aphront-list-filter-view-css',
12 => 'phabricator-directory-css',
13 => 'phabricator-remarkup-css',
14 => 'syntax-highlighting-css',
),
'uri' => '/res/pkg/bb4d65a4/core.pkg.css',
'type' => 'css',
),
'fdcba95b' =>
array(
'name' => 'differential.pkg.css',
@ -1829,19 +1805,43 @@ celerity_register_resource_map(array(
'uri' => '/res/pkg/fdcba95b/differential.pkg.css',
'type' => 'css',
),
16378540 =>
array(
'name' => 'core.pkg.css',
'symbols' =>
array(
0 => 'phabricator-core-css',
1 => 'phabricator-core-buttons-css',
2 => 'phabricator-standard-page-view',
3 => 'aphront-dialog-view-css',
4 => 'aphront-form-view-css',
5 => 'aphront-panel-view-css',
6 => 'aphront-side-nav-view-css',
7 => 'aphront-table-view-css',
8 => 'aphront-crumbs-view-css',
9 => 'aphront-tokenizer-control-css',
10 => 'aphront-typeahead-control-css',
11 => 'aphront-list-filter-view-css',
12 => 'phabricator-directory-css',
13 => 'phabricator-remarkup-css',
14 => 'syntax-highlighting-css',
),
'uri' => '/res/pkg/16378540/core.pkg.css',
'type' => 'css',
),
),
'reverse' =>
array(
'aphront-crumbs-view-css' => 'bb4d65a4',
'aphront-dialog-view-css' => 'bb4d65a4',
'aphront-form-view-css' => 'bb4d65a4',
'aphront-crumbs-view-css' => '16378540',
'aphront-dialog-view-css' => '16378540',
'aphront-form-view-css' => '16378540',
'aphront-headsup-action-list-view-css' => 'fdcba95b',
'aphront-list-filter-view-css' => 'bb4d65a4',
'aphront-panel-view-css' => 'bb4d65a4',
'aphront-side-nav-view-css' => 'bb4d65a4',
'aphront-table-view-css' => 'bb4d65a4',
'aphront-tokenizer-control-css' => 'bb4d65a4',
'aphront-typeahead-control-css' => 'bb4d65a4',
'aphront-list-filter-view-css' => '16378540',
'aphront-panel-view-css' => '16378540',
'aphront-side-nav-view-css' => '16378540',
'aphront-table-view-css' => '16378540',
'aphront-tokenizer-control-css' => '16378540',
'aphront-typeahead-control-css' => '16378540',
'differential-changeset-view-css' => 'fdcba95b',
'differential-core-view-css' => 'fdcba95b',
'differential-inline-comment-editor' => 'a6562582',
@ -1890,16 +1890,16 @@ celerity_register_resource_map(array(
'javelin-vector' => 'b164acea',
'javelin-workflow' => '46547a92',
'phabricator-content-source-view-css' => 'fdcba95b',
'phabricator-core-buttons-css' => 'bb4d65a4',
'phabricator-core-css' => 'bb4d65a4',
'phabricator-directory-css' => 'bb4d65a4',
'phabricator-core-buttons-css' => '16378540',
'phabricator-core-css' => '16378540',
'phabricator-directory-css' => '16378540',
'phabricator-drag-and-drop-file-upload' => 'a6562582',
'phabricator-keyboard-shortcut' => '46547a92',
'phabricator-keyboard-shortcut-manager' => '46547a92',
'phabricator-object-selector-css' => 'fdcba95b',
'phabricator-remarkup-css' => 'bb4d65a4',
'phabricator-remarkup-css' => '16378540',
'phabricator-shaped-request' => 'a6562582',
'phabricator-standard-page-view' => 'bb4d65a4',
'syntax-highlighting-css' => 'bb4d65a4',
'phabricator-standard-page-view' => '16378540',
'syntax-highlighting-css' => '16378540',
),
));

View file

@ -91,13 +91,11 @@ abstract class AphrontResponse {
$this->formatEpochTimestampForHTTPHeader($this->lastModified));
}
$headers[] = array(
// IE has a feature where it may override an explicit Content-Type
// declaration by inferring a content type. This can be a security risk
// and we always explicitly transmit the correct Content-Type header, so
// prevent IE from using inferred content types.
array('X-Content-Type-Options', 'nosniff'),
);
// IE has a feature where it may override an explicit Content-Type
// declaration by inferring a content type. This can be a security risk
// and we always explicitly transmit the correct Content-Type header, so
// prevent IE from using inferred content types.
$headers[] = array('X-Content-Type-Options', 'nosniff');
return $headers;
}

View file

@ -134,6 +134,28 @@ class DifferentialRevisionViewController extends DifferentialController {
$aux_field->setHandles(array_select_keys($handles, $aux_phids[$key]));
}
$reviewer_warning = null;
$has_live_reviewer = false;
foreach ($revision->getReviewers() as $reviewer) {
if (!$handles[$reviewer]->isDisabled()) {
$has_live_reviewer = true;
}
}
if (!$has_live_reviewer) {
$reviewer_warning = new AphrontErrorView();
$reviewer_warning->setSeverity(AphrontErrorView::SEVERITY_WARNING);
$reviewer_warning->setTitle('No Active Reviewers');
if ($revision->getReviewers()) {
$reviewer_warning->appendChild(
'<p>All specified reviewers are disabled. You may want to add '.
'some new reviewers.</p>');
} else {
$reviewer_warning->appendChild(
'<p>This revision has no specified reviewers. You may want to '.
'add some.</p>');
}
}
$request_uri = $request->getRequestURI();
$limit = 100;
@ -272,6 +294,7 @@ class DifferentialRevisionViewController extends DifferentialController {
$page_pane = id(new DifferentialPrimaryPaneView())
->setLineWidthFromChangesets($changesets)
->setID($pane_id)
->appendChild($reviewer_warning)
->appendChild(
$revision_detail->render().
$comment_view->render().

View file

@ -1,7 +1,7 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
* Copyright 2012 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -577,9 +577,12 @@ abstract class DifferentialFieldSpecification {
$object_map = array();
$users = id(new PhabricatorUser())->loadAllWhere(
'(username IN (%Ls)) OR (email IN (%Ls))',
'((username IN (%Ls)) OR (email IN (%Ls)))
AND isDisabled = 0
AND isSystemAgent = 0',
$value,
$value);
$user_map = mpull($users, 'getPHID', 'getUsername');
foreach ($user_map as $username => $phid) {
// Usernames may have uppercase letters in them. Put both names in the
@ -620,7 +623,8 @@ abstract class DifferentialFieldSpecification {
? "users and mailing lists"
: "users";
throw new DifferentialFieldParseException(
"Commit message references nonexistent {$what}: {$invalid}.");
"Commit message references disabled or nonexistent {$what}: ".
"{$invalid}.");
}
return array_unique($results);

View file

@ -137,6 +137,14 @@ class PhabricatorPeopleProfileController extends PhabricatorPeopleController {
$nav->addFilter(null, 'Edit Profile...', '/settings/page/profile/');
}
if ($viewer->getIsAdmin()) {
$nav->addSpacer();
$nav->addFilter(
null,
'Administrate User...',
'/people/edit/'.$user->getID().'/');
}
return $this->buildStandardPageResponse(
$header,
array(

View file

@ -1,7 +1,7 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
* Copyright 2012 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -205,8 +205,13 @@ class PhabricatorObjectHandle {
}
$class = null;
if ($this->status != PhabricatorObjectHandleStatus::STATUS_OPEN) {
$class = 'handle-status-'.$this->status;
$class .= ' handle-status-'.$this->status;
}
if ($this->disabled) {
$class .= ' handle-disabled';
}
return phutil_render_tag(

View file

@ -172,6 +172,11 @@ td.phabricator-login-details {
text-decoration: line-through;
}
a.handle-disabled {
background: #999999;
color: #cccccc;
}
.PhabricatorMonospaced {
font-family: "Menlo", "Consolas", "Monaco", monospace;
font-size: 10px;