1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-02-16 16:58:38 +01:00

Begin transacitonalizing repository edits and provide a more sensible edit interface

Summary:
Ref T2231, T603. Plan of attack here is pretty much:

  - Built out a new (currently not linked in the UI) edit interface in Diffusion which is transaction-based and has a sensible layout.
  - Build out a new create interface based on PagedForm which dumps into the new edit interface.
  - Throw the old stuff away.
  - Everyone lives happily ever after.

Test Plan:
{F44163}
{F44164}

Reviewers: chad, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2231

Differential Revision: https://secure.phabricator.com/D6029
This commit is contained in:
epriestley 2013-05-24 12:37:42 -07:00
parent 27ad838939
commit 2fd018ad92
10 changed files with 436 additions and 1 deletions

View file

@ -0,0 +1,21 @@
CREATE TABLE {$NAMESPACE}_repository.repository_transaction (
id INT UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT,
phid VARCHAR(64) NOT NULL COLLATE utf8_bin,
authorPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
objectPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
viewPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin,
editPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin,
commentPHID VARCHAR(64) COLLATE utf8_bin,
commentVersion INT UNSIGNED NOT NULL,
transactionType VARCHAR(32) NOT NULL COLLATE utf8_bin,
oldValue LONGTEXT NOT NULL COLLATE utf8_bin,
newValue LONGTEXT NOT NULL COLLATE utf8_bin,
metadata LONGTEXT NOT NULL COLLATE utf8_bin,
contentSource LONGTEXT NOT NULL COLLATE utf8_bin,
dateCreated INT UNSIGNED NOT NULL,
dateModified INT UNSIGNED NOT NULL,
UNIQUE KEY `key_phid` (phid),
KEY `key_object` (objectPHID)
) ENGINE=InnoDB, COLLATE utf8_general_ci;

View file

@ -478,6 +478,8 @@ phutil_register_library_map(array(
'DiffusionRemarkupRule' => 'applications/diffusion/remarkup/DiffusionRemarkupRule.php',
'DiffusionRenameHistoryQuery' => 'applications/diffusion/query/DiffusionRenameHistoryQuery.php',
'DiffusionRepositoryController' => 'applications/diffusion/controller/DiffusionRepositoryController.php',
'DiffusionRepositoryEditBasicController' => 'applications/diffusion/controller/DiffusionRepositoryEditBasicController.php',
'DiffusionRepositoryEditController' => 'applications/diffusion/controller/DiffusionRepositoryEditController.php',
'DiffusionRepositoryPath' => 'applications/diffusion/data/DiffusionRepositoryPath.php',
'DiffusionRepositoryTag' => 'applications/diffusion/data/DiffusionRepositoryTag.php',
'DiffusionRequest' => 'applications/diffusion/request/DiffusionRequest.php',
@ -1320,6 +1322,7 @@ phutil_register_library_map(array(
'PhabricatorRepositoryDeleteController' => 'applications/repository/controller/PhabricatorRepositoryDeleteController.php',
'PhabricatorRepositoryDiscoveryEngine' => 'applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php',
'PhabricatorRepositoryEditController' => 'applications/repository/controller/PhabricatorRepositoryEditController.php',
'PhabricatorRepositoryEditor' => 'applications/repository/editor/PhabricatorRepositoryEditor.php',
'PhabricatorRepositoryEngine' => 'applications/repository/engine/PhabricatorRepositoryEngine.php',
'PhabricatorRepositoryGitCommitChangeParserWorker' => 'applications/repository/worker/commitchangeparser/PhabricatorRepositoryGitCommitChangeParserWorker.php',
'PhabricatorRepositoryGitCommitMessageParserWorker' => 'applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php',
@ -1339,6 +1342,8 @@ phutil_register_library_map(array(
'PhabricatorRepositorySvnCommitMessageParserWorker' => 'applications/repository/worker/commitmessageparser/PhabricatorRepositorySvnCommitMessageParserWorker.php',
'PhabricatorRepositorySymbol' => 'applications/repository/storage/PhabricatorRepositorySymbol.php',
'PhabricatorRepositoryTestCase' => 'applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php',
'PhabricatorRepositoryTransaction' => 'applications/repository/storage/PhabricatorRepositoryTransaction.php',
'PhabricatorRepositoryTransactionQuery' => 'applications/repository/query/PhabricatorRepositoryTransactionQuery.php',
'PhabricatorRepositoryType' => 'applications/repository/constants/PhabricatorRepositoryType.php',
'PhabricatorS3FileStorageEngine' => 'applications/files/engine/PhabricatorS3FileStorageEngine.php',
'PhabricatorSQLPatchList' => 'infrastructure/storage/patch/PhabricatorSQLPatchList.php',
@ -2274,6 +2279,8 @@ phutil_register_library_map(array(
'DiffusionRawDiffQuery' => 'DiffusionQuery',
'DiffusionRemarkupRule' => 'PhabricatorRemarkupRuleObject',
'DiffusionRepositoryController' => 'DiffusionController',
'DiffusionRepositoryEditBasicController' => 'DiffusionController',
'DiffusionRepositoryEditController' => 'DiffusionController',
'DiffusionSetupException' => 'AphrontUsageException',
'DiffusionStableCommitNameQuery' => 'DiffusionQuery',
'DiffusionSvnCommitParentsQuery' => 'DiffusionCommitParentsQuery',
@ -3090,6 +3097,7 @@ phutil_register_library_map(array(
'PhabricatorRepositoryDeleteController' => 'PhabricatorRepositoryController',
'PhabricatorRepositoryDiscoveryEngine' => 'PhabricatorRepositoryEngine',
'PhabricatorRepositoryEditController' => 'PhabricatorRepositoryController',
'PhabricatorRepositoryEditor' => 'PhabricatorApplicationTransactionEditor',
'PhabricatorRepositoryGitCommitChangeParserWorker' => 'PhabricatorRepositoryCommitChangeParserWorker',
'PhabricatorRepositoryGitCommitMessageParserWorker' => 'PhabricatorRepositoryCommitMessageParserWorker',
'PhabricatorRepositoryListController' => 'PhabricatorRepositoryController',
@ -3108,6 +3116,8 @@ phutil_register_library_map(array(
'PhabricatorRepositorySvnCommitMessageParserWorker' => 'PhabricatorRepositoryCommitMessageParserWorker',
'PhabricatorRepositorySymbol' => 'PhabricatorRepositoryDAO',
'PhabricatorRepositoryTestCase' => 'PhabricatorTestCase',
'PhabricatorRepositoryTransaction' => 'PhabricatorApplicationTransaction',
'PhabricatorRepositoryTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
'PhabricatorS3FileStorageEngine' => 'PhabricatorFileStorageEngine',
'PhabricatorSSHWorkflow' => 'PhutilArgumentWorkflow',
'PhabricatorSavedQuery' => 'PhabricatorSearchDAO',

View file

@ -62,6 +62,10 @@ final class PhabricatorApplicationDiffusion extends PhabricatorApplication {
=> 'DiffusionCommitTagsController',
'commit/(?P<commit>[a-z0-9]+)/edit/'
=> 'DiffusionCommitEditController',
'edit/' => array(
'' => 'DiffusionRepositoryEditController',
'basic/' => 'DiffusionRepositoryEditBasicController',
),
),
'inline/' => array(
'edit/(?P<phid>[^/]+)/' => 'DiffusionInlineCommentController',

View file

@ -334,4 +334,11 @@ abstract class DiffusionController extends PhabricatorController {
$method,
$params);
}
protected function getRepositoryControllerURI(
PhabricatorRepository $repository,
$path) {
return $this->getApplicationURI($repository->getCallsign().'/'.$path);
}
}

View file

@ -0,0 +1,115 @@
<?php
final class DiffusionRepositoryEditBasicController extends DiffusionController {
public function processRequest() {
$request = $this->getRequest();
$user = $request->getUser();
$drequest = $this->diffusionRequest;
$repository = $drequest->getRepository();
$repository = id(new PhabricatorRepositoryQuery())
->setViewer($user)
->requireCapabilities(
array(
PhabricatorPolicyCapability::CAN_VIEW,
PhabricatorPolicyCapability::CAN_EDIT,
))
->withIDs(array($repository->getID()))
->executeOne();
if (!$repository) {
return new Aphront404Response();
}
$edit_uri = $this->getRepositoryControllerURI($repository, 'edit/');
$v_name = $repository->getName();
$v_desc = $repository->getDetail('description');
$e_name = true;
$errors = array();
if ($request->isFormPost()) {
$v_name = $request->getStr('name');
$v_desc = $request->getStr('description');
if (!strlen($v_name)) {
$e_name = pht('Required');
$errors[] = pht('Repository name is required.');
} else {
$e_name = null;
}
if (!$errors) {
$xactions = array();
$template = id(new PhabricatorRepositoryTransaction());
$type_name = PhabricatorRepositoryTransaction::TYPE_NAME;
$type_desc = PhabricatorRepositoryTransaction::TYPE_DESCRIPTION;
$xactions[] = id(clone $template)
->setTransactionType($type_name)
->setNewValue($v_name);
$xactions[] = id(clone $template)
->setTransactionType($type_desc)
->setNewValue($v_desc);
id(new PhabricatorRepositoryEditor())
->setContinueOnNoEffect(true)
->setContentSourceFromRequest($request)
->setActor($user)
->applyTransactions($repository, $xactions);
return id(new AphrontRedirectResponse())->setURI($edit_uri);
}
}
$content = array();
$crumbs = $this->buildCrumbs();
$crumbs->addCrumb(
id(new PhabricatorCrumbView())
->setName(pht('Edit Basics')));
$content[] = $crumbs;
$title = pht('Edit %s', $repository->getName());
if ($errors) {
$content[] = id(new AphrontErrorView())
->setTitle(pht('Form Errors'))
->setErrors($errors);
}
$form = id(new AphrontFormView())
->setUser($user)
->setFlexible(true)
->appendChild(
id(new AphrontFormTextControl())
->setName('name')
->setLabel(pht('Name'))
->setValue($v_name)
->setError($e_name))
->appendChild(
id(new AphrontFormTextAreaControl())
->setName('description')
->setLabel(pht('Description'))
->setValue($v_desc))
->appendChild(
id(new AphrontFormSubmitControl())
->setValue(pht('Save'))
->addCancelButton($edit_uri));
$content[] = $form;
return $this->buildApplicationPage(
$content,
array(
'title' => $title,
'dust' => true,
'device' => true,
));
}
}

View file

@ -0,0 +1,112 @@
<?php
final class DiffusionRepositoryEditController extends DiffusionController {
public function processRequest() {
$request = $this->getRequest();
$user = $request->getUser();
$drequest = $this->diffusionRequest;
$repository = $drequest->getRepository();
$content = array();
$crumbs = $this->buildCrumbs();
$crumbs->addCrumb(
id(new PhabricatorCrumbView())
->setName(pht('Edit')));
$content[] = $crumbs;
$title = pht('Edit %s', $repository->getName());
$content[] = id(new PhabricatorHeaderView())
->setHeader($title);
$content[] = $this->buildBasicActions($repository);
$content[] = $this->buildBasicProperties($repository);
$content[] = id(new PhabricatorHeaderView())
->setHeader(pht('Edit History'));
$xactions = id(new PhabricatorRepositoryTransactionQuery())
->setViewer($user)
->withObjectPHIDs(array($repository->getPHID()))
->execute();
$engine = id(new PhabricatorMarkupEngine())
->setViewer($user);
foreach ($xactions as $xaction) {
if ($xaction->getComment()) {
$engine->addObject(
$xaction->getComment(),
PhabricatorApplicationTransactionComment::MARKUP_FIELD_COMMENT);
}
}
$engine->process();
$xaction_view = id(new PhabricatorApplicationTransactionView())
->setUser($user)
->setTransactions($xactions)
->setMarkupEngine($engine);
$content[] = $xaction_view;
return $this->buildApplicationPage(
$content,
array(
'title' => $title,
'device' => true,
'dust' => true,
));
}
private function buildBasicActions(PhabricatorRepository $repository) {
$user = $this->getRequest()->getUser();
$view = id(new PhabricatorActionListView())
->setUser($user);
$can_edit = PhabricatorPolicyFilter::hasCapability(
$user,
$repository,
PhabricatorPolicyCapability::CAN_EDIT);
$edit = id(new PhabricatorActionView())
->setIcon('edit')
->setName(pht('Edit Basic Information'))
->setHref($this->getRepositoryControllerURI($repository, 'edit/basic/'))
->setDisabled(!$can_edit);
$view->addAction($edit);
return $view;
}
private function buildBasicProperties(PhabricatorRepository $repository) {
$view = id(new PhabricatorPropertyListView())
->setUser($this->getRequest()->getUser())
->setObject($repository);
$view->addProperty(pht('Name'), $repository->getName());
$view->addProperty(pht('ID'), $repository->getID());
$view->addProperty(pht('PHID'), $repository->getPHID());
$type = PhabricatorRepositoryType::getNameForRepositoryType(
$repository->getVersionControlSystem());
$view->addProperty(pht('Type'), $type);
$view->addProperty(pht('Callsign'), $repository->getCallsign());
$description = $repository->getDetail('description');
if (!strlen($description)) {
$description = phutil_tag('em', array(), pht('None'));
}
$view->addProperty(pht('Description'), $description);
return $view;
}
}

View file

@ -0,0 +1,84 @@
<?php
final class PhabricatorRepositoryEditor
extends PhabricatorApplicationTransactionEditor {
public function getTransactionTypes() {
$types = parent::getTransactionTypes();
$types[] = PhabricatorRepositoryTransaction::TYPE_NAME;
$types[] = PhabricatorRepositoryTransaction::TYPE_DESCRIPTION;
return $types;
}
protected function getCustomTransactionOldValue(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
case PhabricatorRepositoryTransaction::TYPE_NAME:
return $object->getName();
case PhabricatorRepositoryTransaction::TYPE_DESCRIPTION:
return $object->getDetail('description');
}
}
protected function getCustomTransactionNewValue(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
case PhabricatorRepositoryTransaction::TYPE_NAME:
case PhabricatorRepositoryTransaction::TYPE_DESCRIPTION:
return $xaction->getNewValue();
}
}
protected function applyCustomInternalTransaction(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
case PhabricatorRepositoryTransaction::TYPE_NAME:
$object->setName($xaction->getNewValue());
break;
case PhabricatorRepositoryTransaction::TYPE_DESCRIPTION:
$object->setDetail('description', $xaction->getNewValue());
break;
}
}
protected function applyCustomExternalTransaction(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
return;
}
protected function mergeTransactions(
PhabricatorApplicationTransaction $u,
PhabricatorApplicationTransaction $v) {
$type = $u->getTransactionType();
switch ($type) {
}
return parent::mergeTransactions($u, $v);
}
protected function transactionHasEffect(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
$old = $xaction->getOldValue();
$new = $xaction->getNewValue();
$type = $xaction->getTransactionType();
switch ($type) {
}
return parent::transactionHasEffect($object, $xaction);
}
}

View file

@ -0,0 +1,10 @@
<?php
final class PhabricatorRepositoryTransactionQuery
extends PhabricatorApplicationTransactionQuery {
protected function getTemplateApplicationTransaction() {
return new PhabricatorRepositoryTransaction();
}
}

View file

@ -0,0 +1,69 @@
<?php
final class PhabricatorRepositoryTransaction
extends PhabricatorApplicationTransaction {
const TYPE_NAME = 'repo:name';
const TYPE_DESCRIPTION = 'repo:description';
public function getApplicationName() {
return 'repository';
}
public function getApplicationTransactionType() {
return PhabricatorPHIDConstants::PHID_TYPE_REPO;
}
public function getApplicationTransactionCommentObject() {
return null;
}
public function getApplicationObjectTypeName() {
return pht('repository');
}
public function getTitle() {
$author_phid = $this->getAuthorPHID();
$old = $this->getOldValue();
$new = $this->getNewValue();
switch ($this->getTransactionType()) {
case self::TYPE_NAME:
return pht(
'%s renamed this repository from "%s" to "%s".',
$this->renderHandleLink($author_phid),
$old,
$new);
case self::TYPE_DESCRIPTION:
return pht(
'%s updated the description of this repository.',
$this->renderHandleLink($author_phid));
}
return parent::getTitle();
}
public function hasChangeDetails() {
switch ($this->getTransactionType()) {
case self::TYPE_DESCRIPTION:
return true;
}
return parent::hasChangeDetails();
}
public function renderChangeDetails(PhabricatorUser $viewer) {
$old = $this->getOldValue();
$new = $this->getNewValue();
$view = id(new PhabricatorApplicationTransactionTextDiffDetailView())
->setUser($viewer)
->setOldText($old)
->setNewText($new);
return $view->render();
}
}

View file

@ -1314,7 +1314,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList {
'type' => 'sql',
'name' => $this->getPatchPath('20130523.maniphest_owners.sql'),
),
'20130524.repoxactions.sql' => array(
'type' => 'sql',
'name' => $this->getPatchPath('20130524.repoxactions.sql'),
),
);
}
}