Skip to content

Commit 24ced7e

Browse files
author
epriestley
committedFeb 6, 2013
Expose commit information via conduit instead of user information
Summary: After D4825, this information is often available to us in a safe way. Provide it explictly. This removes or reduces functionality in some cases, but I think we can plug those holes with Conpherence addresses and/or explicit user acknowledgement/config. Test Plan: Patched a commit with `arc patch` and got the original address out. Reviewers: btrahan, edward, vrana Reviewed By: btrahan CC: aran Differential Revision: https://secure.phabricator.com/D4828
1 parent 574bc3b commit 24ced7e

File tree

4 files changed

+7
-79
lines changed

4 files changed

+7
-79
lines changed
 

‎conf/default.conf.php

-11
Original file line numberDiff line numberDiff line change
@@ -1015,17 +1015,6 @@
10151015
// interact with the revisions.
10161016
'differential.anonymous-access' => false,
10171017

1018-
// If you set this to true, revision author email address information will
1019-
// be exposed in Conduit. This is useful for Arcanist.
1020-
//
1021-
// For example, consider the "arc patch DX" workflow which needs to ask
1022-
// Differential for the revision DX. This data often should contain
1023-
// the author's email address, eg "George Washington
1024-
// <gwashinton@example.com>" when DX is a git or mercurial revision. If this
1025-
// option is false, Differential defaults to the best it can, something like
1026-
// "George Washington" or "gwashington".
1027-
'differential.expose-emails-prudently' => false,
1028-
10291018
// List of file regexps that should be treated as if they are generated by
10301019
// an automatic process, and thus get hidden by default in differential.
10311020
'differential.generated-paths' => array(

‎src/applications/differential/conduit/ConduitAPI_differential_getdiff_Method.php

-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ protected function execute(ConduitAPIRequest $request) {
7272
$project_name = null;
7373
}
7474
$basic_dict['projectName'] = $project_name;
75-
$basic_dict['author'] = $diff->loadAuthorInformation();
7675

7776
return $basic_dict;
7877
}

‎src/applications/differential/config/PhabricatorDifferentialConfigOptions.php

-22
Original file line numberDiff line numberDiff line change
@@ -127,28 +127,6 @@ public function getOptions() {
127127
"If you set this to true, users won't need to login to view ".
128128
"Differential revisions. Anonymous users will have read-only ".
129129
"access and won't be able to interact with the revisions.")),
130-
$this->newOption('differential.expose-emails-prudently', 'bool', false)
131-
->setBoolOptions(
132-
array(
133-
pht("Expose revision author email address via Conduit"),
134-
pht("Don't expose revision author email address via Conduit"),
135-
))
136-
->setSummary(
137-
pht(
138-
"Determines whether or not the author's email address should be ".
139-
"exposed via Conduit."))
140-
->setDescription(
141-
pht(
142-
"If you set this to true, revision author email address ".
143-
"information will be exposed in Conduit. This is useful for ".
144-
"Arcanist.\n\n".
145-
"For example, consider the 'arc patch DX' workflow which needs ".
146-
"to ask Differential for the revision DX. This data often should ".
147-
"contain the author's email address, eg 'George Washington ".
148-
"<gwashinton@example.com>' when DX is a git or mercurial ".
149-
"revision. If this option is false, Differential defaults to the ".
150-
"best it can, something like 'George Washington' or ".
151-
"'gwashington'.")),
152130
$this->newOption('differential.generated-paths', 'list<string>', array())
153131
->setSummary(pht("File regexps to treat as automatically generated."))
154132
->setDescription(

‎src/applications/differential/storage/DifferentialDiff.php

+7-45
Original file line numberDiff line numberDiff line change
@@ -240,55 +240,17 @@ public function getDiffDict() {
240240
$this->getID());
241241
foreach ($properties as $property) {
242242
$dict['properties'][$property->getName()] = $property->getData();
243-
}
244-
245-
return $dict;
246-
}
247243

248-
/**
249-
* Figures out the right author information for a given diff based on the
250-
* repository and Phabricator configuration settings.
251-
*
252-
* Git is particularly finicky as it requires author information to be in
253-
* the format "George Washington <gwashington@example.com>" to
254-
* consistently work. If the Phabricator instance isn't configured to
255-
* expose emails prudently, then we are unable to get any author information
256-
* for git.
257-
*/
258-
public function loadAuthorInformation() {
259-
$author = id(new PhabricatorUser())
260-
->loadOneWhere('phid = %s', $this->getAuthorPHID());
261-
262-
$use_emails =
263-
PhabricatorEnv::getEnvConfig('differential.expose-emails-prudently');
264-
265-
switch ($this->getSourceControlSystem()) {
266-
case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
267-
if (!$use_emails) {
268-
$author_info = '';
269-
} else {
270-
$author_info = $this->getFullAuthorInfo($author);
244+
if ($property->getName() == 'local:commits') {
245+
foreach ($property->getData() as $commit) {
246+
$dict['authorName'] = $commit['author'];
247+
$dict['authorEmail'] = $commit['authorEmail'];
248+
break;
271249
}
272-
break;
273-
case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
274-
if (!$use_emails) {
275-
$author_info = $author->getUsername();
276-
} else {
277-
$author_info = $this->getFullAuthorInfo($author);
278-
}
279-
break;
280-
case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
281-
default:
282-
$author_info = $author->getUsername();
283-
break;
250+
}
284251
}
285252

286-
return $author_info;
253+
return $dict;
287254
}
288255

289-
private function getFullAuthorInfo(PhabricatorUser $author) {
290-
return sprintf('%s <%s>',
291-
$author->getRealName(),
292-
$author->loadPrimaryEmailAddress());
293-
}
294256
}

0 commit comments

Comments
 (0)
Failed to load comments.