mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-10 00:42:41 +01:00
Fix an issue with serializing reviewers over the wire
Fixes T10981. Ref T10939. `arc` currently has some odd, hard-coded checks (missing reviewers, all reviewers away) that depend on the field value being in a certain format. The recent changes swapped the field value from scalars (PHIDs) to dictionaries and broke this workflow. It worked fine in testing because we apply these checks very inconsistently (not on update or `--edit`). To get around this for now, serialize into "PHID!" and then unserialize on the other side. This is icky but keeps us from needing to require an `arc` upgrade. These checks are generally bad news and should move to the server side in the long run (T4631). (This probably prevents clean `arc diff`, so I'm just cowboy committing it.) Auditors: chad
This commit is contained in:
parent
de1a30efc7
commit
3aed39b8b0
1 changed files with 37 additions and 1 deletions
|
@ -180,7 +180,7 @@ final class DifferentialReviewersField
|
||||||
}
|
}
|
||||||
|
|
||||||
public function parseValueFromCommitMessage($value) {
|
public function parseValueFromCommitMessage($value) {
|
||||||
return $this->parseObjectList(
|
$results = $this->parseObjectList(
|
||||||
$value,
|
$value,
|
||||||
array(
|
array(
|
||||||
PhabricatorPeopleUserPHIDType::TYPECONST,
|
PhabricatorPeopleUserPHIDType::TYPECONST,
|
||||||
|
@ -189,6 +189,8 @@ final class DifferentialReviewersField
|
||||||
),
|
),
|
||||||
false,
|
false,
|
||||||
array('!'));
|
array('!'));
|
||||||
|
|
||||||
|
return $this->flattenReviewers($results);
|
||||||
}
|
}
|
||||||
|
|
||||||
public function getRequiredHandlePHIDsForCommitMessage() {
|
public function getRequiredHandlePHIDsForCommitMessage() {
|
||||||
|
@ -196,6 +198,8 @@ final class DifferentialReviewersField
|
||||||
}
|
}
|
||||||
|
|
||||||
public function readValueFromCommitMessage($value) {
|
public function readValueFromCommitMessage($value) {
|
||||||
|
$value = $this->inflateReviewers($value);
|
||||||
|
|
||||||
$reviewers = array();
|
$reviewers = array();
|
||||||
foreach ($value as $spec) {
|
foreach ($value as $spec) {
|
||||||
$phid = $spec['phid'];
|
$phid = $spec['phid'];
|
||||||
|
@ -287,4 +291,36 @@ final class DifferentialReviewersField
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function flattenReviewers(array $values) {
|
||||||
|
// NOTE: For now, `arc` relies on this field returning only scalars, so we
|
||||||
|
// need to reduce the results into scalars. See T10981.
|
||||||
|
$result = array();
|
||||||
|
|
||||||
|
foreach ($values as $value) {
|
||||||
|
$result[] = $value['phid'].implode('', array_keys($value['suffixes']));
|
||||||
|
}
|
||||||
|
|
||||||
|
return $result;
|
||||||
|
}
|
||||||
|
|
||||||
|
private function inflateReviewers(array $values) {
|
||||||
|
$result = array();
|
||||||
|
|
||||||
|
foreach ($values as $value) {
|
||||||
|
if (substr($value, -1) == '!') {
|
||||||
|
$value = substr($value, 0, -1);
|
||||||
|
$suffixes = array('!' => '!');
|
||||||
|
} else {
|
||||||
|
$suffixes = array();
|
||||||
|
}
|
||||||
|
|
||||||
|
$result[] = array(
|
||||||
|
'phid' => $value,
|
||||||
|
'suffixes' => $suffixes,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
return $result;
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue