From 515f9a36ab7ab7114fbe441099382360522e93fe Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sun, 6 Oct 2013 17:07:55 -0700
Subject: [PATCH] When editing objects which use files, attach the files to the
objects
Summary: Ref T603. Fixes T3921. Tightens up policy controls for file/object relationships in existing applications.
Test Plan:
- Uploaded new project image, verified it got an edge to the project.
- Uploaded new profile image, verified it got an edge to me.
- Uploaded new macro image, verified it got an edge to the macro.
- Uploaded new paste via web UI and conduit, verified it got attached.
- Replaced, added images to a mock, verified they got edges.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3921, T603
Differential Revision: https://secure.phabricator.com/D7254
---
.../files/storage/PhabricatorFile.php | 19 +++++++++++++++
.../macro/editor/PhabricatorMacroEditor.php | 12 ++++++++++
.../ConduitAPI_paste_create_Method.php | 4 ++++
.../paste/editor/PhabricatorPasteEditor.php | 22 ++++++++++++++---
.../storage/PhabricatorPasteTransaction.php | 3 ---
...bricatorPeopleProfilePictureController.php | 1 +
.../pholio/editor/PholioMockEditor.php | 20 ++++++++++++++++
...habricatorProjectProfileEditController.php | 3 +++
...habricatorApplicationTransactionEditor.php | 24 +++++++++++++++----
9 files changed, 98 insertions(+), 10 deletions(-)
diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php
index fd50087c95..fdd2a0641f 100644
--- a/src/applications/files/storage/PhabricatorFile.php
+++ b/src/applications/files/storage/PhabricatorFile.php
@@ -860,6 +860,25 @@ final class PhabricatorFile extends PhabricatorFileDAO
return idx($this->metadata, self::METADATA_IMAGE_WIDTH);
}
+ /**
+ * Write the policy edge between this file and some object.
+ *
+ * @param PhabricatorUser Acting user.
+ * @param phid Object PHID to attach to.
+ * @return this
+ */
+ public function attachToObject(PhabricatorUser $actor, $phid) {
+ $edge_type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_FILE;
+
+ id(new PhabricatorEdgeEditor())
+ ->setActor($actor)
+ ->setSuppressEvents(true)
+ ->addEdge($phid, $edge_type, $this->getPHID())
+ ->save();
+
+ return $this;
+ }
+
/* -( PhabricatorPolicyInterface Implementation )-------------------------- */
diff --git a/src/applications/macro/editor/PhabricatorMacroEditor.php b/src/applications/macro/editor/PhabricatorMacroEditor.php
index d37e111e66..cbf7ccec83 100644
--- a/src/applications/macro/editor/PhabricatorMacroEditor.php
+++ b/src/applications/macro/editor/PhabricatorMacroEditor.php
@@ -77,6 +77,18 @@ final class PhabricatorMacroEditor
return;
}
+ protected function extractFilePHIDsFromCustomTransaction(
+ PhabricatorLiskDAO $object,
+ PhabricatorApplicationTransaction $xaction) {
+
+ switch ($xaction->getTransactionType()) {
+ case PhabricatorMacroTransactionType::TYPE_FILE:
+ return array($xaction->getNewValue());
+ }
+
+ return array();
+ }
+
protected function mergeTransactions(
PhabricatorApplicationTransaction $u,
PhabricatorApplicationTransaction $v) {
diff --git a/src/applications/paste/conduit/ConduitAPI_paste_create_Method.php b/src/applications/paste/conduit/ConduitAPI_paste_create_Method.php
index 114aea698f..a8cbe4c5f1 100644
--- a/src/applications/paste/conduit/ConduitAPI_paste_create_Method.php
+++ b/src/applications/paste/conduit/ConduitAPI_paste_create_Method.php
@@ -49,6 +49,8 @@ final class ConduitAPI_paste_create_Method extends ConduitAPI_paste_Method {
'authorPHID' => $user->getPHID(),
));
+ // TODO: This should use PhabricatorPasteEditor.
+
$paste = new PhabricatorPaste();
$paste->setTitle($title);
$paste->setLanguage($language);
@@ -57,6 +59,8 @@ final class ConduitAPI_paste_create_Method extends ConduitAPI_paste_Method {
$paste->setViewPolicy(PhabricatorPolicies::POLICY_USER);
$paste->save();
+ $paste_file->attachToObject($user, $paste->getPHID());
+
$paste->attachRawContent($content);
return $this->buildPasteInfoDictionary($paste);
diff --git a/src/applications/paste/editor/PhabricatorPasteEditor.php b/src/applications/paste/editor/PhabricatorPasteEditor.php
index 20da125257..cb3d8a5e71 100644
--- a/src/applications/paste/editor/PhabricatorPasteEditor.php
+++ b/src/applications/paste/editor/PhabricatorPasteEditor.php
@@ -1,11 +1,10 @@
<?php
-/**
- * @group paste
- */
final class PhabricatorPasteEditor
extends PhabricatorApplicationTransactionEditor {
+ private $pasteFile;
+
public function getTransactionTypes() {
$types = parent::getTransactionTypes();
@@ -95,11 +94,28 @@ final class PhabricatorPasteEditor
'authorPHID' => $this->getActor()->getPHID(),
));
$object->setFilePHID($paste_file->getPHID());
+
+ $this->pasteFile = $paste_file;
break;
}
}
}
+ protected function applyFinalEffects(
+ PhabricatorLiskDAO $object,
+ array $xactions) {
+
+ // TODO: This should use extractFilePHIDs() instead, but the way
+ // the transactions work right now makes pretty messy.
+
+ if ($this->pasteFile) {
+ $this->pasteFile->attachToObject(
+ $this->getActor(),
+ $object->getPHID());
+ }
+ }
+
+
protected function shouldSendMail(
PhabricatorLiskDAO $object,
array $xactions) {
diff --git a/src/applications/paste/storage/PhabricatorPasteTransaction.php b/src/applications/paste/storage/PhabricatorPasteTransaction.php
index 96e5e5827f..453c13802c 100644
--- a/src/applications/paste/storage/PhabricatorPasteTransaction.php
+++ b/src/applications/paste/storage/PhabricatorPasteTransaction.php
@@ -1,8 +1,5 @@
<?php
-/**
- * @group paste
- */
final class PhabricatorPasteTransaction
extends PhabricatorApplicationTransaction {
diff --git a/src/applications/people/controller/PhabricatorPeopleProfilePictureController.php b/src/applications/people/controller/PhabricatorPeopleProfilePictureController.php
index d16a4ab4a6..169b90911a 100644
--- a/src/applications/people/controller/PhabricatorPeopleProfilePictureController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfilePictureController.php
@@ -82,6 +82,7 @@ final class PhabricatorPeopleProfilePictureController
$user->setProfileImagePHID(null);
} else {
$user->setProfileImagePHID($xformed->getPHID());
+ $xformed->attachToObject($viewer, $user->getPHID());
}
$user->save();
return id(new AphrontRedirectResponse())->setURI($profile_uri);
diff --git a/src/applications/pholio/editor/PholioMockEditor.php b/src/applications/pholio/editor/PholioMockEditor.php
index 30e82b8fe2..8cf5784cc4 100644
--- a/src/applications/pholio/editor/PholioMockEditor.php
+++ b/src/applications/pholio/editor/PholioMockEditor.php
@@ -106,6 +106,26 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor {
}
}
+ protected function extractFilePHIDsFromCustomTransaction(
+ PhabricatorLiskDAO $object,
+ PhabricatorApplicationTransaction $xaction) {
+
+ switch ($xaction->getTransactionType()) {
+ case PholioTransactionType::TYPE_IMAGE_FILE:
+ $new = $xaction->getNewValue();
+ $phids = array();
+ foreach ($new as $key => $images) {
+ $phids[] = mpull($images, 'getFilePHID');
+ }
+ return array_mergev($phids);
+ case PholioTransactionType::TYPE_IMAGE_REPLACE:
+ return array($xaction->getNewValue()->getFilePHID());
+ }
+
+ return array();
+ }
+
+
protected function transactionHasEffect(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
diff --git a/src/applications/project/controller/PhabricatorProjectProfileEditController.php b/src/applications/project/controller/PhabricatorProjectProfileEditController.php
index d482afbc5c..5f7aa0824f 100644
--- a/src/applications/project/controller/PhabricatorProjectProfileEditController.php
+++ b/src/applications/project/controller/PhabricatorProjectProfileEditController.php
@@ -107,7 +107,10 @@ final class PhabricatorProjectProfileEditController
$file,
$x = 50,
$y = 50);
+
$profile->setProfileImagePHID($xformed->getPHID());
+ $xformed->attachToObject($user, $project->getPHID());
+
} else {
$e_image = pht('Not Supported');
$errors[] =
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
index 5cb35a926c..f1bb0db57c 100644
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -1575,13 +1575,20 @@ abstract class PhabricatorApplicationTransactionEditor
}
$blocks = array_mergev($blocks);
- if (!$blocks) {
- return array();
+
+ $phids = array();
+ if ($blocks) {
+ $phids[] = PhabricatorMarkupEngine::extractFilePHIDsFromEmbeddedFiles(
+ $blocks);
}
- $phids = PhabricatorMarkupEngine::extractFilePHIDsFromEmbeddedFiles(
- $blocks);
+ foreach ($xactions as $xaction) {
+ $phids[] = $this->extractFilePHIDsFromCustomTransaction(
+ $object,
+ $xaction);
+ }
+ $phids = array_unique(array_filter(array_mergev($phids)));
if (!$phids) {
return array();
}
@@ -1598,6 +1605,15 @@ abstract class PhabricatorApplicationTransactionEditor
return mpull($files, 'getPHID');
}
+ /**
+ * @task files
+ */
+ protected function extractFilePHIDsFromCustomTransaction(
+ PhabricatorLiskDAO $object,
+ PhabricatorApplicationTransaction $xaction) {
+ return array();
+ }
+
/**
* @task files