mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-19 03:50:54 +01:00
Differential - refine selecting a repository diff --> revision workflow
Summary: Fixes T6200. Ref T6237. When creating a diff from the web view, allow the user to select the repository at that time. When viewing a diff that has no associated revision and then creating a revision, pass along the repository phid to the create revision controller. Within the create revision controller, default the repository selector to this repository phid. Finally, in the editor, stop aggressively resetting the repository phid for every TYPE_UPDATE; rather, do so if its not a new object -- the diff should reign supreme in that case -- or if there's no repository -- let the diff be the guide. Test Plan: - made a diff with an associated repo, made a revision from the diff, saw the associated repo and it stuck on save! - made a diff with an associated repo, made a revision from the diff but changed the repo and it stuck on save! - made a diff with an associated repo, made a revision from the diff but changed the repo to nothing and it stuck on save! Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T6237, T6200 Differential Revision: https://secure.phabricator.com/D10872
This commit is contained in:
parent
ffe0765b50
commit
7ef236c9a6
6 changed files with 67 additions and 8 deletions
|
@ -61,6 +61,7 @@ final class DifferentialCreateRawDiffConduitAPIMethod
|
||||||
id(new DifferentialDiffEditor())
|
id(new DifferentialDiffEditor())
|
||||||
->setActor($viewer)
|
->setActor($viewer)
|
||||||
->setContentSourceFromConduitRequest($request)
|
->setContentSourceFromConduitRequest($request)
|
||||||
|
->setLookupRepository(false) // respect user choice
|
||||||
->applyTransactions($diff, $xactions);
|
->applyTransactions($diff, $xactions);
|
||||||
|
|
||||||
return $this->buildDiffInfoDictionary($diff);
|
return $this->buildDiffInfoDictionary($diff);
|
||||||
|
|
|
@ -7,12 +7,20 @@ final class DifferentialDiffCreateController extends DifferentialController {
|
||||||
$request = $this->getRequest();
|
$request = $this->getRequest();
|
||||||
|
|
||||||
$diff = null;
|
$diff = null;
|
||||||
|
$repository_phid = null;
|
||||||
|
$repository_value = array();
|
||||||
$errors = array();
|
$errors = array();
|
||||||
$e_diff = null;
|
$e_diff = null;
|
||||||
$e_file = null;
|
$e_file = null;
|
||||||
$validation_exception = null;
|
$validation_exception = null;
|
||||||
if ($request->isFormPost()) {
|
if ($request->isFormPost()) {
|
||||||
|
|
||||||
|
$repository_tokenizer = $request->getArr(
|
||||||
|
id(new DifferentialRepositoryField())->getFieldKey());
|
||||||
|
if ($repository_tokenizer) {
|
||||||
|
$repository_phid = reset($repository_tokenizer);
|
||||||
|
}
|
||||||
|
|
||||||
if ($request->getFileExists('diff-file')) {
|
if ($request->getFileExists('diff-file')) {
|
||||||
$diff = PhabricatorFile::readUploadedFileData($_FILES['diff-file']);
|
$diff = PhabricatorFile::readUploadedFileData($_FILES['diff-file']);
|
||||||
} else {
|
} else {
|
||||||
|
@ -33,7 +41,7 @@ final class DifferentialDiffCreateController extends DifferentialController {
|
||||||
'differential.createrawdiff',
|
'differential.createrawdiff',
|
||||||
array(
|
array(
|
||||||
'diff' => $diff,
|
'diff' => $diff,
|
||||||
));
|
'repositoryPHID' => $repository_phid,));
|
||||||
$call->setUser($request->getUser());
|
$call->setUser($request->getUser());
|
||||||
$result = $call->execute();
|
$result = $call->execute();
|
||||||
$path = id(new PhutilURI($result['uri']))->getPath();
|
$path = id(new PhutilURI($result['uri']))->getPath();
|
||||||
|
@ -56,6 +64,10 @@ final class DifferentialDiffCreateController extends DifferentialController {
|
||||||
|
|
||||||
$cancel_uri = $this->getApplicationURI();
|
$cancel_uri = $this->getApplicationURI();
|
||||||
|
|
||||||
|
if ($repository_phid) {
|
||||||
|
$repository_value = $this->loadViewerHandles(array($repository_phid));
|
||||||
|
}
|
||||||
|
|
||||||
$form
|
$form
|
||||||
->setAction('/differential/diff/create/')
|
->setAction('/differential/diff/create/')
|
||||||
->setEncType('multipart/form-data')
|
->setEncType('multipart/form-data')
|
||||||
|
@ -81,6 +93,13 @@ final class DifferentialDiffCreateController extends DifferentialController {
|
||||||
->setLabel(pht('Raw Diff From File'))
|
->setLabel(pht('Raw Diff From File'))
|
||||||
->setName('diff-file')
|
->setName('diff-file')
|
||||||
->setError($e_file))
|
->setError($e_file))
|
||||||
|
->appendChild(
|
||||||
|
id(new AphrontFormTokenizerControl())
|
||||||
|
->setName(id(new DifferentialRepositoryField())->getFieldKey())
|
||||||
|
->setLabel(pht('Repository'))
|
||||||
|
->setDatasource(new DiffusionRepositoryDatasource())
|
||||||
|
->setValue($repository_value)
|
||||||
|
->setLimit(1))
|
||||||
->appendChild(
|
->appendChild(
|
||||||
id(new AphrontFormSubmitControl())
|
id(new AphrontFormSubmitControl())
|
||||||
->addCancelButton($cancel_uri)
|
->addCancelButton($cancel_uri)
|
||||||
|
|
|
@ -80,6 +80,9 @@ final class DifferentialDiffViewController extends DifferentialController {
|
||||||
->setAction('/differential/revision/edit/')
|
->setAction('/differential/revision/edit/')
|
||||||
->addHiddenInput('diffID', $diff->getID())
|
->addHiddenInput('diffID', $diff->getID())
|
||||||
->addHiddenInput('viaDiffView', 1)
|
->addHiddenInput('viaDiffView', 1)
|
||||||
|
->addHiddenInput(
|
||||||
|
id(new DifferentialRepositoryField())->getFieldKey(),
|
||||||
|
$diff->getRepositoryPHID())
|
||||||
->appendRemarkupInstructions(
|
->appendRemarkupInstructions(
|
||||||
pht(
|
pht(
|
||||||
'Review the diff for correctness. When you are satisfied, either '.
|
'Review the diff for correctness. When you are satisfied, either '.
|
||||||
|
|
|
@ -71,16 +71,41 @@ final class DifferentialRevisionEditController
|
||||||
->setViewer($viewer)
|
->setViewer($viewer)
|
||||||
->readFieldsFromStorage($revision);
|
->readFieldsFromStorage($revision);
|
||||||
|
|
||||||
|
if ($request->getStr('viaDiffView') && $diff) {
|
||||||
|
$repo_key = id(new DifferentialRepositoryField())->getFieldKey();
|
||||||
|
$repository_field = idx(
|
||||||
|
$field_list->getFields(),
|
||||||
|
$repo_key);
|
||||||
|
if ($repository_field) {
|
||||||
|
$repository_field->setValue($request->getStr($repo_key));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
$validation_exception = null;
|
$validation_exception = null;
|
||||||
if ($request->isFormPost() && !$request->getStr('viaDiffView')) {
|
if ($request->isFormPost() && !$request->getStr('viaDiffView')) {
|
||||||
|
|
||||||
|
$editor = id(new DifferentialTransactionEditor())
|
||||||
|
->setActor($viewer)
|
||||||
|
->setContentSourceFromRequest($request)
|
||||||
|
->setContinueOnNoEffect(true);
|
||||||
|
|
||||||
$xactions = $field_list->buildFieldTransactionsFromRequest(
|
$xactions = $field_list->buildFieldTransactionsFromRequest(
|
||||||
new DifferentialTransaction(),
|
new DifferentialTransaction(),
|
||||||
$request);
|
$request);
|
||||||
|
|
||||||
if ($diff) {
|
if ($diff) {
|
||||||
|
$repository_phid = null;
|
||||||
|
$repository_tokenizer = $request->getArr(
|
||||||
|
id(new DifferentialRepositoryField())->getFieldKey());
|
||||||
|
if ($repository_tokenizer) {
|
||||||
|
$repository_phid = reset($repository_tokenizer);
|
||||||
|
}
|
||||||
|
|
||||||
$xactions[] = id(new DifferentialTransaction())
|
$xactions[] = id(new DifferentialTransaction())
|
||||||
->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
|
->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
|
||||||
->setNewValue($diff->getPHID());
|
->setNewValue($diff->getPHID());
|
||||||
|
|
||||||
|
$editor->setRepositoryPHIDOverride($repository_phid);
|
||||||
}
|
}
|
||||||
|
|
||||||
$comments = $request->getStr('comments');
|
$comments = $request->getStr('comments');
|
||||||
|
@ -92,11 +117,6 @@ final class DifferentialRevisionEditController
|
||||||
->setContent($comments));
|
->setContent($comments));
|
||||||
}
|
}
|
||||||
|
|
||||||
$editor = id(new DifferentialTransactionEditor())
|
|
||||||
->setActor($viewer)
|
|
||||||
->setContentSourceFromRequest($request)
|
|
||||||
->setContinueOnNoEffect(true);
|
|
||||||
|
|
||||||
try {
|
try {
|
||||||
$editor->applyTransactions($revision, $xactions);
|
$editor->applyTransactions($revision, $xactions);
|
||||||
$revision_uri = '/D'.$revision->getID();
|
$revision_uri = '/D'.$revision->getID();
|
||||||
|
|
|
@ -4,6 +4,12 @@ final class DifferentialDiffEditor
|
||||||
extends PhabricatorApplicationTransactionEditor {
|
extends PhabricatorApplicationTransactionEditor {
|
||||||
|
|
||||||
private $diffDataDict;
|
private $diffDataDict;
|
||||||
|
private $lookupRepository = true;
|
||||||
|
|
||||||
|
public function setLookupRepository($bool) {
|
||||||
|
$this->lookupRepository = $bool;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
public function getEditorApplicationClass() {
|
public function getEditorApplicationClass() {
|
||||||
return 'PhabricatorDifferentialApplication';
|
return 'PhabricatorDifferentialApplication';
|
||||||
|
@ -80,7 +86,7 @@ final class DifferentialDiffEditor
|
||||||
// is old, or couldn't figure out which repository the working copy
|
// is old, or couldn't figure out which repository the working copy
|
||||||
// belongs to), apply heuristics to try to figure it out.
|
// belongs to), apply heuristics to try to figure it out.
|
||||||
|
|
||||||
if (!$object->getRepositoryPHID()) {
|
if ($this->lookupRepository && !$object->getRepositoryPHID()) {
|
||||||
$repository = id(new DifferentialRepositoryLookup())
|
$repository = id(new DifferentialRepositoryLookup())
|
||||||
->setDiff($object)
|
->setDiff($object)
|
||||||
->setViewer($this->getActor())
|
->setViewer($this->getActor())
|
||||||
|
|
|
@ -6,6 +6,7 @@ final class DifferentialTransactionEditor
|
||||||
private $heraldEmailPHIDs;
|
private $heraldEmailPHIDs;
|
||||||
private $changedPriorToCommitURI;
|
private $changedPriorToCommitURI;
|
||||||
private $isCloseByCommit;
|
private $isCloseByCommit;
|
||||||
|
private $repositoryPHIDOverride = false;
|
||||||
|
|
||||||
public function getEditorApplicationClass() {
|
public function getEditorApplicationClass() {
|
||||||
return 'PhabricatorDifferentialApplication';
|
return 'PhabricatorDifferentialApplication';
|
||||||
|
@ -45,6 +46,11 @@ final class DifferentialTransactionEditor
|
||||||
return $this->changedPriorToCommitURI;
|
return $this->changedPriorToCommitURI;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function setRepositoryPHIDOverride($phid_or_null) {
|
||||||
|
$this->repositoryPHIDOverride = $phid_or_null;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
public function getTransactionTypes() {
|
public function getTransactionTypes() {
|
||||||
$types = parent::getTransactionTypes();
|
$types = parent::getTransactionTypes();
|
||||||
|
|
||||||
|
@ -205,7 +211,11 @@ final class DifferentialTransactionEditor
|
||||||
$diff = $this->requireDiff($xaction->getNewValue());
|
$diff = $this->requireDiff($xaction->getNewValue());
|
||||||
|
|
||||||
$object->setLineCount($diff->getLineCount());
|
$object->setLineCount($diff->getLineCount());
|
||||||
|
if ($this->repositoryPHIDOverride !== false) {
|
||||||
|
$object->setRepositoryPHID($this->repositoryPHIDOverride);
|
||||||
|
} else {
|
||||||
$object->setRepositoryPHID($diff->getRepositoryPHID());
|
$object->setRepositoryPHID($diff->getRepositoryPHID());
|
||||||
|
}
|
||||||
$object->setArcanistProjectPHID($diff->getArcanistProjectPHID());
|
$object->setArcanistProjectPHID($diff->getArcanistProjectPHID());
|
||||||
$object->attachActiveDiff($diff);
|
$object->attachActiveDiff($diff);
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue