1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-23 22:10:55 +01:00

Remove willWriteRevision/didWriteRevision hooks

Summary:
Ref T2222. DifferentialRevisionEditor has no remaining callsites, but it has a bit of functionality which still needs to be ported forward. I'm going to rip it apart piece by piece.

This removes the willWriteRevision/didWriteRevision hooks. They are completely encapsulated by transactions now, except for a unique piece of branch/task logic, which I migrated forward.

Test Plan:
  - Lots of `grep`.
  - Created a new revision on branch `T25`, saw it associate with the task.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D8454
This commit is contained in:
epriestley 2014-03-08 12:03:15 -08:00
parent fbaa12440e
commit a19f49632f
15 changed files with 35 additions and 212 deletions

View file

@ -161,8 +161,6 @@ final class DifferentialRevisionEditor extends PhabricatorEditor {
$revision->loadRelationships(); $revision->loadRelationships();
$this->willWriteRevision();
if ($this->reviewers === null) { if ($this->reviewers === null) {
$this->reviewers = $revision->getReviewers(); $this->reviewers = $revision->getReviewers();
} }
@ -437,8 +435,6 @@ final class DifferentialRevisionEditor extends PhabricatorEditor {
break; break;
} }
$this->didWriteRevision();
$event_data = array( $event_data = array(
'revision_id' => $revision->getID(), 'revision_id' => $revision->getID(),
'revision_phid' => $revision->getPHID(), 'revision_phid' => $revision->getPHID(),
@ -690,42 +686,6 @@ final class DifferentialRevisionEditor extends PhabricatorEditor {
} }
} }
private function willWriteRevision() {
foreach ($this->auxiliaryFields as $aux_field) {
$aux_field->willWriteRevision($this);
}
$this->dispatchEvent(
PhabricatorEventType::TYPE_DIFFERENTIAL_WILLEDITREVISION);
}
private function didWriteRevision() {
foreach ($this->auxiliaryFields as $aux_field) {
$aux_field->didWriteRevision($this);
}
$this->dispatchEvent(
PhabricatorEventType::TYPE_DIFFERENTIAL_DIDEDITREVISION);
}
private function dispatchEvent($type) {
$event = new PhabricatorEvent(
$type,
array(
'revision' => $this->revision,
'new' => $this->isCreate,
));
$event->setUser($this->getActor());
$request = $this->getAphrontRequestForEventDispatch();
if ($request) {
$event->setAphrontRequest($request);
}
PhutilEventEngine::dispatchEvent($event);
}
/** /**
* Update the table which links Differential revisions to paths they affect, * Update the table which links Differential revisions to paths they affect,
* so Diffusion can efficiently find pending revisions for a given file. * so Diffusion can efficiently find pending revisions for a given file.

View file

@ -226,7 +226,9 @@ final class DifferentialTransactionEditor
$actor = $this->getActor(); $actor = $this->getActor();
$actor_phid = $actor->getPHID(); $actor_phid = $actor->getPHID();
$type_edge = PhabricatorTransactions::TYPE_EDGE; $type_edge = PhabricatorTransactions::TYPE_EDGE;
$edge_reviewer = PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER; $edge_reviewer = PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER;
$edge_ref_task = PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK;
$results = parent::expandTransaction($object, $xaction); $results = parent::expandTransaction($object, $xaction);
switch ($xaction->getTransactionType()) { switch ($xaction->getTransactionType()) {
@ -266,6 +268,38 @@ final class DifferentialTransactionEditor
->setIgnoreOnNoEffect(true) ->setIgnoreOnNoEffect(true)
->setNewValue(array('+' => $edits)); ->setNewValue(array('+' => $edits));
} }
// When a revision is updated and the diff comes from a branch named
// "T123" or similar, automatically associate the commit with the
// task that the branch names.
$maniphest = 'PhabricatorApplicationManiphest';
if (PhabricatorApplication::isClassInstalled($maniphest)) {
$diff = $this->loadDiff($xaction->getNewValue());
if ($diff) {
$branch = $diff->getBranch();
// No "$", to allow for branches like T123_demo.
$match = null;
if (preg_match('/^T(\d+)/i', $branch, $match)) {
$task_id = $match[1];
$tasks = id(new ManiphestTaskQuery())
->setViewer($this->getActor())
->withIDs(array($task_id))
->execute();
if ($tasks) {
$task = head($tasks);
$task_phid = $task->getPHID();
$results[] = id(new DifferentialTransaction())
->setTransactionType($type_edge)
->setMetadataValue('edge:type', $edge_ref_task)
->setIgnoreOnNoEffect(true)
->setNewValue(array('+' => array($task_phid => $task_phid)));
}
}
}
}
break; break;
case PhabricatorTransactions::TYPE_COMMENT: case PhabricatorTransactions::TYPE_COMMENT:
@ -905,7 +939,7 @@ final class DifferentialTransactionEditor
switch ($strongest->getTransactionType()) { switch ($strongest->getTransactionType()) {
case DifferentialTransaction::TYPE_UPDATE: case DifferentialTransaction::TYPE_UPDATE:
$count = new PhutilNumber($object->getLineCount()); $count = new PhutilNumber($object->getLineCount());
$action = pht('%s, %s line(s)', $action, $count); $action = pht('%s, %d line(s)', $action, $count);
break; break;
} }

View file

@ -43,32 +43,6 @@ final class DifferentialBranchFieldSpecification
return null; return null;
} }
public function didWriteRevision(DifferentialRevisionEditor $editor) {
$maniphest = 'PhabricatorApplicationManiphest';
if (!PhabricatorApplication::isClassInstalled($maniphest)) {
return;
}
$branch = $this->getDiff()->getBranch();
$match = null;
if (preg_match('/^T(\d+)/i', $branch, $match)) { // No $ to allow T123_demo.
list(, $task_id) = $match;
$task = id(new ManiphestTaskQuery())
->setViewer($editor->requireActor())
->withIDs(array($task_id))
->executeOne();
if ($task) {
id(new PhabricatorEdgeEditor())
->setActor($this->getUser())
->addEdge(
$this->getRevision()->getPHID(),
PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK,
$task->getPHID())
->save();
}
}
}
public function getCommitMessageTips() { public function getCommitMessageTips() {
return array( return array(
'Name branch "T123" to attach the diff to a task.', 'Name branch "T123" to attach the diff to a task.',

View file

@ -60,10 +60,6 @@ final class DifferentialCCsFieldSpecification
->setValue($cc_map); ->setValue($cc_map);
} }
public function willWriteRevision(DifferentialRevisionEditor $editor) {
$editor->setCCPHIDs($this->ccs);
}
public function shouldAppearOnCommitMessage() { public function shouldAppearOnCommitMessage() {
return true; return true;
} }

View file

@ -35,8 +35,4 @@ final class DifferentialEditPolicyFieldSpecification
->setName('editPolicy'); ->setName('editPolicy');
} }
public function willWriteRevision(DifferentialRevisionEditor $editor) {
$this->getRevision()->setEditPolicy($this->value);
}
} }

View file

@ -192,38 +192,6 @@ abstract class DifferentialFieldSpecification {
return false; return false;
} }
/**
* Hook for applying revision changes via the editor. Normally, you should
* not implement this, but a number of builtin fields use the revision object
* itself as storage. If you need to do something similar for whatever reason,
* this method gives you an opportunity to interact with the editor or
* revision before changes are saved (for example, you can write the field's
* value into some property of the revision).
*
* @param DifferentialRevisionEditor Active editor which is applying changes
* to the revision.
* @return void
* @task edit
*/
public function willWriteRevision(DifferentialRevisionEditor $editor) {
return;
}
/**
* Hook after an edit operation has completed. This allows you to update
* link tables or do other write operations which should happen after the
* revision is saved. Normally you don't need to implement this.
*
*
* @param DifferentialRevisionEditor Active editor which has just applied
* changes to the revision.
* @return void
* @task edit
*/
public function didWriteRevision(DifferentialRevisionEditor $editor) {
return;
}
/* -( Extending the Revision View Interface )------------------------------ */ /* -( Extending the Revision View Interface )------------------------------ */

View file

@ -63,34 +63,6 @@ abstract class DifferentialFreeformFieldSpecification
return $result; return $result;
} }
public function didWriteRevision(DifferentialRevisionEditor $editor) {
$message = $this->renderValueForCommitMessage(false);
$tasks = $this->findMentionedTasks($message);
if ($tasks) {
$tasks = id(new ManiphestTaskQuery())
->setViewer($editor->getActor())
->withIDs(array_keys($tasks))
->execute();
$this->saveFieldEdges(
$editor->getRevision(),
PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK,
mpull($tasks, 'getPHID'));
}
$dependents = $this->findDependentRevisions($message);
if ($dependents) {
$dependents = id(new DifferentialRevisionQuery())
->setViewer($editor->getActor())
->withIDs($dependents)
->execute();
$this->saveFieldEdges(
$editor->getRevision(),
PhabricatorEdgeConfig::TYPE_DREV_DEPENDS_ON_DREV,
mpull($dependents, 'getPHID'));
}
}
private function saveFieldEdges( private function saveFieldEdges(
DifferentialRevision $revision, DifferentialRevision $revision,
$edge_type, $edge_type,

View file

@ -167,29 +167,4 @@ final class DifferentialJIRAIssuesFieldSpecification
return $xobjs; return $xobjs;
} }
public function didWriteRevision(DifferentialRevisionEditor $editor) {
$revision = $editor->getRevision();
$revision_phid = $revision->getPHID();
$edge_type = PhabricatorEdgeConfig::TYPE_PHOB_HAS_JIRAISSUE;
$edge_dsts = mpull($this->loadDoorkeeperExternalObjects(), 'getPHID');
$edges = PhabricatorEdgeQuery::loadDestinationPHIDs(
$revision_phid,
$edge_type);
$editor = id(new PhabricatorEdgeEditor())
->setActor($this->getUser());
foreach (array_diff($edges, $edge_dsts) as $rem_edge) {
$editor->removeEdge($revision_phid, $edge_type, $rem_edge);
}
foreach (array_diff($edge_dsts, $edges) as $add_edge) {
$editor->addEdge($revision_phid, $edge_type, $add_edge);
}
$editor->save();
}
} }

View file

@ -43,34 +43,6 @@ final class DifferentialManiphestTasksFieldSpecification
PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK); PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK);
} }
/**
* Attach the revision to the task(s) and the task(s) to the revision.
*
* @return void
*/
public function didWriteRevision(DifferentialRevisionEditor $editor) {
$revision = $editor->getRevision();
$revision_phid = $revision->getPHID();
$edge_type = PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK;
$old_phids = $this->oldManiphestTasks;
$add_phids = $this->maniphestTasks;
$rem_phids = array_diff($old_phids, $add_phids);
$edge_editor = id(new PhabricatorEdgeEditor())
->setActor($this->getUser());
foreach ($add_phids as $phid) {
$edge_editor->addEdge($revision_phid, $edge_type, $phid);
}
foreach ($rem_phids as $phid) {
$edge_editor->removeEdge($revision_phid, $edge_type, $phid);
}
$edge_editor->save();
}
protected function didSetRevision() { protected function didSetRevision() {
$this->maniphestTasks = $this->getManiphestTaskPHIDs(); $this->maniphestTasks = $this->getManiphestTaskPHIDs();
$this->oldManiphestTasks = $this->maniphestTasks; $this->oldManiphestTasks = $this->maniphestTasks;

View file

@ -41,8 +41,4 @@ final class DifferentialRepositoryFieldSpecification
->setValue($value); ->setValue($value);
} }
public function willWriteRevision(DifferentialRevisionEditor $editor) {
$this->getRevision()->setRepositoryPHID($this->value);
}
} }

View file

@ -100,10 +100,6 @@ final class DifferentialReviewersFieldSpecification
->setError($this->error); ->setError($this->error);
} }
public function willWriteRevision(DifferentialRevisionEditor $editor) {
$editor->setReviewers($this->reviewers);
}
public function shouldAppearOnCommitMessage() { public function shouldAppearOnCommitMessage() {
return true; return true;
} }

View file

@ -38,10 +38,6 @@ final class DifferentialSummaryFieldSpecification
return true; return true;
} }
public function willWriteRevision(DifferentialRevisionEditor $editor) {
$this->getRevision()->setSummary($this->summary);
}
public function shouldAppearOnCommitMessage() { public function shouldAppearOnCommitMessage() {
return true; return true;
} }

View file

@ -42,10 +42,6 @@ final class DifferentialTestPlanFieldSpecification
return true; return true;
} }
public function willWriteRevision(DifferentialRevisionEditor $editor) {
$this->getRevision()->setTestPlan($this->plan);
}
public function validateField() { public function validateField() {
if ($this->isRequired()) { if ($this->isRequired()) {
if (!strlen($this->plan)) { if (!strlen($this->plan)) {

View file

@ -33,10 +33,6 @@ final class DifferentialTitleFieldSpecification
return true; return true;
} }
public function willWriteRevision(DifferentialRevisionEditor $editor) {
$this->getRevision()->setTitle($this->title);
}
public function validateField() { public function validateField() {
if (!strlen($this->title)) { if (!strlen($this->title)) {
$this->error = 'Required'; $this->error = 'Required';

View file

@ -35,8 +35,4 @@ final class DifferentialViewPolicyFieldSpecification
->setName('viewPolicy'); ->setName('viewPolicy');
} }
public function willWriteRevision(DifferentialRevisionEditor $editor) {
$this->getRevision()->setViewPolicy($this->value);
}
} }