Fix file PHID extraction in Owners and Differential

Summary:
Ref T9787. To fix this, I want to change how file PHIDs are extracted slightly: specifically, I'm going to extract them later in the editing process.

Before doing this, clean up a couple of bad implementations:

  - Owners extracts its description as a file PHID. This is an error.
    - Extract the description as a remarkup block instead.
    - Add an edge table so stuff like file attachment works properly.
  - Differential has a no-op extract method. This is presumably just a copy/paste issue from long ago.

Test Plan:
  - Edited a revision in Differential.
  - Dropped a file into the description of an Owners package.
    - Before change: this did not attach the file.
    - After change: the file now attaches properly and shows up as "Attached" in the file details.

Reviewers: chad, joshuaspence

Reviewed By: joshuaspence

Subscribers: joshuaspence

Maniphest Tasks: T9787

Differential Revision: https://secure.phabricator.com/D14493
This commit is contained in:
epriestley
2015-11-16 09:53:01 -08:00
parent 26a235ab8a
commit 5aae89babb
6 changed files with 40 additions and 21 deletions

View File

@@ -2563,6 +2563,7 @@ phutil_register_library_map(array(
'PhabricatorOwnersPackageTransactionQuery' => 'applications/owners/query/PhabricatorOwnersPackageTransactionQuery.php',
'PhabricatorOwnersPath' => 'applications/owners/storage/PhabricatorOwnersPath.php',
'PhabricatorOwnersPathsController' => 'applications/owners/controller/PhabricatorOwnersPathsController.php',
'PhabricatorOwnersSchemaSpec' => 'applications/owners/storage/PhabricatorOwnersSchemaSpec.php',
'PhabricatorOwnersSearchField' => 'applications/owners/searchfield/PhabricatorOwnersSearchField.php',
'PhabricatorPHDConfigOptions' => 'applications/config/option/PhabricatorPHDConfigOptions.php',
'PhabricatorPHID' => 'applications/phid/storage/PhabricatorPHID.php',
@@ -6718,6 +6719,7 @@ phutil_register_library_map(array(
'PhabricatorOwnersPackageTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
'PhabricatorOwnersPath' => 'PhabricatorOwnersDAO',
'PhabricatorOwnersPathsController' => 'PhabricatorOwnersController',
'PhabricatorOwnersSchemaSpec' => 'PhabricatorConfigSchemaSpec',
'PhabricatorOwnersSearchField' => 'PhabricatorSearchTokenizerField',
'PhabricatorPHDConfigOptions' => 'PhabricatorApplicationConfigOptions',
'PhabricatorPHID' => 'Phobject',

View File

@@ -1298,15 +1298,6 @@ final class DifferentialTransactionEditor
return true;
}
protected function extractFilePHIDsFromCustomTransaction(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {}
return parent::extractFilePHIDsFromCustomTransaction($object, $xaction);
}
protected function expandCustomRemarkupBlockTransactions(
PhabricatorLiskDAO $object,
array $xactions,

View File

@@ -205,18 +205,6 @@ final class PhabricatorOwnersPackageTransactionEditor
return $errors;
}
protected function extractFilePHIDsFromCustomTransaction(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION:
return array($xaction->getNewValue());
}
return parent::extractFilePHIDsFromCustomTransaction($object, $xaction);
}
protected function shouldSendMail(
PhabricatorLiskDAO $object,
array $xactions) {

View File

@@ -208,4 +208,16 @@ final class PhabricatorOwnersPackageTransaction
return parent::renderChangeDetails($viewer);
}
public function getRemarkupBlocks() {
$blocks = parent::getRemarkupBlocks();
switch ($this->getTransactionType()) {
case self::TYPE_DESCRIPTION:
$blocks[] = $this->getNewValue();
break;
}
return $blocks;
}
}

View File

@@ -0,0 +1,10 @@
<?php
final class PhabricatorOwnersSchemaSpec
extends PhabricatorConfigSchemaSpec {
public function buildSchemata() {
$this->buildEdgeSchemata(new PhabricatorOwnersPackage());
}
}