From a2a19e29aaceb6eee8ae1214cdd18b143dea7065 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Sat, 17 Sep 2016 21:23:54 +0000 Subject: [PATCH] Remove TYPE_FILES from Conpherence Summary: I believe these are left over from widgets, when we added a "Files" widget that kept track of everything added to the window. Test Plan: Added files to a Conpherece, Set an image when editing. Anything else? Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D16567 --- .../__tests__/ConpherenceRoomTestCase.php | 19 ----- ...ConpherenceQueryThreadConduitAPIMethod.php | 4 +- ...onpherenceUpdateThreadConduitAPIMethod.php | 3 +- .../ConpherenceUpdateController.php | 1 - .../conpherence/editor/ConpherenceEditor.php | 70 ------------------- .../query/ConpherenceThreadQuery.php | 22 ------ .../conpherence/storage/ConpherenceThread.php | 10 --- .../storage/ConpherenceTransaction.php | 27 ------- .../view/ConpherenceTransactionView.php | 3 - 9 files changed, 2 insertions(+), 157 deletions(-) diff --git a/src/applications/conpherence/__tests__/ConpherenceRoomTestCase.php b/src/applications/conpherence/__tests__/ConpherenceRoomTestCase.php index a4eeb3ed86..f34dd25110 100644 --- a/src/applications/conpherence/__tests__/ConpherenceRoomTestCase.php +++ b/src/applications/conpherence/__tests__/ConpherenceRoomTestCase.php @@ -112,25 +112,6 @@ final class ConpherenceRoomTestCase extends ConpherenceTestCase { } } - public function testAddMessageWithFileAttachments() { - $creator = $this->generateNewTestUser(); - $friend_1 = $this->generateNewTestUser(); - - $participant_map = array( - $creator->getPHID() => $creator, - $friend_1->getPHID() => $friend_1, - ); - - $conpherence = $this->createRoom( - $creator, - array_keys($participant_map)); - - foreach ($participant_map as $phid => $user) { - $xactions = $this->addMessageWithFile($user, $conpherence); - $this->assertEqual(2, count($xactions)); - } - } - private function createRoom( PhabricatorUser $creator, array $participant_phids) { diff --git a/src/applications/conpherence/conduit/ConpherenceQueryThreadConduitAPIMethod.php b/src/applications/conpherence/conduit/ConpherenceQueryThreadConduitAPIMethod.php index cd5fa1bff7..c66a90f585 100644 --- a/src/applications/conpherence/conduit/ConpherenceQueryThreadConduitAPIMethod.php +++ b/src/applications/conpherence/conduit/ConpherenceQueryThreadConduitAPIMethod.php @@ -37,8 +37,7 @@ final class ConpherenceQueryThreadConduitAPIMethod $query = id(new ConpherenceThreadQuery()) ->setViewer($user) - ->needParticipantCache(true) - ->needFilePHIDs(true); + ->needParticipantCache(true); if ($ids) { $conpherences = $query @@ -73,7 +72,6 @@ final class ConpherenceQueryThreadConduitAPIMethod 'conpherenceTitle' => $conpherence->getTitle(), 'messageCount' => $conpherence->getMessageCount(), 'recentParticipantPHIDs' => $conpherence->getRecentParticipantPHIDs(), - 'filePHIDs' => $conpherence->getFilePHIDs(), 'conpherenceURI' => $this->getConpherenceURI($conpherence), ); } diff --git a/src/applications/conpherence/conduit/ConpherenceUpdateThreadConduitAPIMethod.php b/src/applications/conpherence/conduit/ConpherenceUpdateThreadConduitAPIMethod.php index ebcf7f9a04..01b86f9a42 100644 --- a/src/applications/conpherence/conduit/ConpherenceUpdateThreadConduitAPIMethod.php +++ b/src/applications/conpherence/conduit/ConpherenceUpdateThreadConduitAPIMethod.php @@ -44,8 +44,7 @@ final class ConpherenceUpdateThreadConduitAPIMethod $id = $request->getValue('id'); $phid = $request->getValue('phid'); $query = id(new ConpherenceThreadQuery()) - ->setViewer($user) - ->needFilePHIDs(true); + ->setViewer($user); if ($id) { $query->withIDs(array($id)); } else if ($phid) { diff --git a/src/applications/conpherence/controller/ConpherenceUpdateController.php b/src/applications/conpherence/controller/ConpherenceUpdateController.php index f8602a15cf..5a1ad4e8fa 100644 --- a/src/applications/conpherence/controller/ConpherenceUpdateController.php +++ b/src/applications/conpherence/controller/ConpherenceUpdateController.php @@ -36,7 +36,6 @@ final class ConpherenceUpdateController $conpherence = id(new ConpherenceThreadQuery()) ->setViewer($user) ->withIDs(array($conpherence_id)) - ->needFilePHIDs(true) ->needOrigPics(true) ->needCropPics(true) ->needParticipants($need_participants) diff --git a/src/applications/conpherence/editor/ConpherenceEditor.php b/src/applications/conpherence/editor/ConpherenceEditor.php index b7b37a3e6d..8507e9817f 100644 --- a/src/applications/conpherence/editor/ConpherenceEditor.php +++ b/src/applications/conpherence/editor/ConpherenceEditor.php @@ -22,7 +22,6 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { $topic) { $conpherence = ConpherenceThread::initializeNewRoom($creator); - $files = array(); $errors = array(); if (empty($participant_phids)) { $errors[] = self::ERROR_EMPTY_PARTICIPANTS; @@ -35,26 +34,11 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { $errors[] = self::ERROR_EMPTY_MESSAGE; } - $file_phids = PhabricatorMarkupEngine::extractFilePHIDsFromEmbeddedFiles( - $creator, - array($message)); - if ($file_phids) { - $files = id(new PhabricatorFileQuery()) - ->setViewer($creator) - ->withPHIDs($file_phids) - ->execute(); - } - if (!$errors) { $xactions = array(); $xactions[] = id(new ConpherenceTransaction()) ->setTransactionType(ConpherenceTransaction::TYPE_PARTICIPANTS) ->setNewValue(array('+' => $participant_phids)); - if ($files) { - $xactions[] = id(new ConpherenceTransaction()) - ->setTransactionType(ConpherenceTransaction::TYPE_FILES) - ->setNewValue(array('+' => mpull($files, 'getPHID'))); - } if ($title) { $xactions[] = id(new ConpherenceTransaction()) ->setTransactionType(ConpherenceTransaction::TYPE_TITLE) @@ -88,27 +72,7 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { ConpherenceThread $conpherence, $text) { - $files = array(); - $file_phids = PhabricatorMarkupEngine::extractFilePHIDsFromEmbeddedFiles( - $viewer, - array($text)); - // Since these are extracted from text, we might be re-including the - // same file -- e.g. a mock under discussion. Filter files we - // already have. - $existing_file_phids = $conpherence->getFilePHIDs(); - $file_phids = array_diff($file_phids, $existing_file_phids); - if ($file_phids) { - $files = id(new PhabricatorFileQuery()) - ->setViewer($this->getActor()) - ->withPHIDs($file_phids) - ->execute(); - } $xactions = array(); - if ($files) { - $xactions[] = id(new ConpherenceTransaction()) - ->setTransactionType(ConpherenceTransaction::TYPE_FILES) - ->setNewValue(array('+' => mpull($files, 'getPHID'))); - } $xactions[] = id(new ConpherenceTransaction()) ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) ->attachComment( @@ -126,7 +90,6 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { $types[] = ConpherenceTransaction::TYPE_TITLE; $types[] = ConpherenceTransaction::TYPE_TOPIC; $types[] = ConpherenceTransaction::TYPE_PARTICIPANTS; - $types[] = ConpherenceTransaction::TYPE_FILES; $types[] = ConpherenceTransaction::TYPE_PICTURE; $types[] = ConpherenceTransaction::TYPE_PICTURE_CROP; $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; @@ -154,8 +117,6 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { return array(); } return $object->getParticipantPHIDs(); - case ConpherenceTransaction::TYPE_FILES: - return $object->getFilePHIDs(); } } @@ -172,7 +133,6 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { $file = $xaction->getNewValue(); return $file->getPHID(); case ConpherenceTransaction::TYPE_PARTICIPANTS: - case ConpherenceTransaction::TYPE_FILES: return $this->getPHIDTransactionNewValue($xaction); } } @@ -335,27 +295,6 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { - case ConpherenceTransaction::TYPE_FILES: - $editor = new PhabricatorEdgeEditor(); - $edge_type = PhabricatorObjectHasFileEdgeType::EDGECONST; - $old = array_fill_keys($xaction->getOldValue(), true); - $new = array_fill_keys($xaction->getNewValue(), true); - $add_edges = array_keys(array_diff_key($new, $old)); - $remove_edges = array_keys(array_diff_key($old, $new)); - foreach ($add_edges as $file_phid) { - $editor->addEdge( - $object->getPHID(), - $edge_type, - $file_phid); - } - foreach ($remove_edges as $file_phid) { - $editor->removeEdge( - $object->getPHID(), - $edge_type, - $file_phid); - } - $editor->save(); - break; case ConpherenceTransaction::TYPE_PARTICIPANTS: if ($this->getIsNewObject()) { continue; @@ -488,14 +427,6 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { PhabricatorPolicyCapability::CAN_EDIT); } break; - // This is similar to PhabricatorTransactions::TYPE_COMMENT so - // use CAN_VIEW - case ConpherenceTransaction::TYPE_FILES: - PhabricatorPolicyFilter::requireCapability( - $this->requireActor(), - $object, - PhabricatorPolicyCapability::CAN_VIEW); - break; case ConpherenceTransaction::TYPE_TITLE: case ConpherenceTransaction::TYPE_TOPIC: PhabricatorPolicyFilter::requireCapability( @@ -514,7 +445,6 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { switch ($type) { case ConpherenceTransaction::TYPE_TITLE: return $v; - case ConpherenceTransaction::TYPE_FILES: case ConpherenceTransaction::TYPE_PARTICIPANTS: return $this->mergePHIDOrEdgeTransactions($u, $v); } diff --git a/src/applications/conpherence/query/ConpherenceThreadQuery.php b/src/applications/conpherence/query/ConpherenceThreadQuery.php index dc1e761c43..c3d5322c15 100644 --- a/src/applications/conpherence/query/ConpherenceThreadQuery.php +++ b/src/applications/conpherence/query/ConpherenceThreadQuery.php @@ -13,17 +13,11 @@ final class ConpherenceThreadQuery private $needOrigPics; private $needTransactions; private $needParticipantCache; - private $needFilePHIDs; private $afterTransactionID; private $beforeTransactionID; private $transactionLimit; private $fulltext; - public function needFilePHIDs($need_file_phids) { - $this->needFilePHIDs = $need_file_phids; - return $this; - } - public function needParticipantCache($participant_cache) { $this->needParticipantCache = $participant_cache; return $this; @@ -116,9 +110,6 @@ final class ConpherenceThreadQuery if ($this->needTransactions) { $this->loadTransactionsAndHandles($conpherences); } - if ($this->needFilePHIDs) { - $this->loadFilePHIDs($conpherences); - } if ($this->needOrigPics || $this->needCropPics) { $this->initImages($conpherences); } @@ -275,19 +266,6 @@ final class ConpherenceThreadQuery return $this; } - private function loadFilePHIDs(array $conpherences) { - $edge_type = PhabricatorObjectHasFileEdgeType::EDGECONST; - $file_edges = id(new PhabricatorEdgeQuery()) - ->withSourcePHIDs(array_keys($conpherences)) - ->withEdgeTypes(array($edge_type)) - ->execute(); - foreach ($file_edges as $conpherence_phid => $data) { - $conpherence = $conpherences[$conpherence_phid]; - $conpherence->attachFilePHIDs(array_keys($data[$edge_type])); - } - return $this; - } - private function loadOrigPics(array $conpherences) { return $this->loadPics( $conpherences, diff --git a/src/applications/conpherence/storage/ConpherenceThread.php b/src/applications/conpherence/storage/ConpherenceThread.php index ec8449f87f..80efc92f5a 100644 --- a/src/applications/conpherence/storage/ConpherenceThread.php +++ b/src/applications/conpherence/storage/ConpherenceThread.php @@ -20,7 +20,6 @@ final class ConpherenceThread extends ConpherenceDAO private $participants = self::ATTACHABLE; private $transactions = self::ATTACHABLE; private $handles = self::ATTACHABLE; - private $filePHIDs = self::ATTACHABLE; private $images = self::ATTACHABLE; public static function initializeNewRoom(PhabricatorUser $sender) { @@ -31,7 +30,6 @@ final class ConpherenceThread extends ConpherenceDAO ->setTitle('') ->setTopic('') ->attachParticipants(array()) - ->attachFilePHIDs(array()) ->attachImages(array()) ->setViewPolicy($default_policy) ->setEditPolicy($default_policy) @@ -158,14 +156,6 @@ final class ConpherenceThread extends ConpherenceDAO $amount); } - public function attachFilePHIDs(array $file_phids) { - $this->filePHIDs = $file_phids; - return $this; - } - public function getFilePHIDs() { - return $this->assertAttached($this->filePHIDs); - } - public function loadImageURI($size) { $file = $this->getImage($size); diff --git a/src/applications/conpherence/storage/ConpherenceTransaction.php b/src/applications/conpherence/storage/ConpherenceTransaction.php index 1090be43c8..29d23dfbc3 100644 --- a/src/applications/conpherence/storage/ConpherenceTransaction.php +++ b/src/applications/conpherence/storage/ConpherenceTransaction.php @@ -2,7 +2,6 @@ final class ConpherenceTransaction extends PhabricatorApplicationTransaction { - const TYPE_FILES = 'files'; const TYPE_TITLE = 'title'; const TYPE_TOPIC = 'topic'; const TYPE_PARTICIPANTS = 'participants'; @@ -44,8 +43,6 @@ final class ConpherenceTransaction extends PhabricatorApplicationTransaction { case self::TYPE_PICTURE: case self::TYPE_DATE_MARKER: return false; - case self::TYPE_FILES: - return true; case self::TYPE_PICTURE_CROP: return true; } @@ -68,29 +65,6 @@ final class ConpherenceTransaction extends PhabricatorApplicationTransaction { case self::TYPE_PICTURE: return $this->getRoomTitle(); break; - case self::TYPE_FILES: - $add = array_diff($new, $old); - $rem = array_diff($old, $new); - - if ($add && $rem) { - $title = pht( - '%s edited files(s), added %d and removed %d.', - $this->renderHandleLink($author_phid), - count($add), - count($rem)); - } else if ($add) { - $title = pht( - '%s added %s files(s).', - $this->renderHandleLink($author_phid), - phutil_count($add)); - } else { - $title = pht( - '%s removed %s file(s).', - $this->renderHandleLink($author_phid), - phutil_count($rem)); - } - return $title; - break; case self::TYPE_PARTICIPANTS: $add = array_diff($new, $old); $rem = array_diff($old, $new); @@ -252,7 +226,6 @@ final class ConpherenceTransaction extends PhabricatorApplicationTransaction { switch ($this->getTransactionType()) { case self::TYPE_TITLE: case self::TYPE_PICTURE: - case self::TYPE_FILES: case self::TYPE_DATE_MARKER: break; case self::TYPE_PARTICIPANTS: diff --git a/src/applications/conpherence/view/ConpherenceTransactionView.php b/src/applications/conpherence/view/ConpherenceTransactionView.php index 467ddf1411..e99013fda7 100644 --- a/src/applications/conpherence/view/ConpherenceTransactionView.php +++ b/src/applications/conpherence/view/ConpherenceTransactionView.php @@ -227,9 +227,6 @@ final class ConpherenceTransactionView extends AphrontView { $content = null; $handles = $this->getHandles(); switch ($transaction->getTransactionType()) { - case ConpherenceTransaction::TYPE_FILES: - $content = $transaction->getTitle(); - break; case ConpherenceTransaction::TYPE_TITLE: case ConpherenceTransaction::TYPE_TOPIC: case ConpherenceTransaction::TYPE_PICTURE: