From f75f3a0c3b32bf4bcc26e5971451b936cbe9299d Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Thu, 25 Jul 2013 16:59:25 -0700 Subject: [PATCH] Pholio - add concept of replacing images and primitive history view Summary: Now you can actually replace an image! Ref T3572. This ended up needing a wee bit of infrastructure to work... - add replace image transaction to pholio - add replacesImagePHID to PholioImage - tweaks to editor to properly update images with respect to replacement - add edges to track replacement - expose getNodes on graph query infrastructure to query the entire graph of who replaced who - move pholio image to new phid infrastructure Still TODO - the history view should get chopped out a bit from the current view - no more inline comments / generally less functionality plus maybe a tweak or two to make this more sensical. Test Plan: replaced images and played with history controller a little. works okay. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T3572 Differential Revision: https://secure.phabricator.com/D6560 --- .../sql/patches/20130722.pholioreplace.sql | 2 + src/__phutil_library_map__.php | 7 + .../phid/PhabricatorObjectHandle.php | 1 - .../phid/PhabricatorPHIDConstants.php | 1 - .../handle/PhabricatorObjectHandleData.php | 24 --- .../PhabricatorApplicationPholio.php | 1 + .../constants/PholioTransactionType.php | 1 + .../PholioImageHistoryController.php | 96 ++++++++++++ .../controller/PholioMockEditController.php | 47 ++++-- .../controller/PholioMockViewController.php | 40 +++++ .../pholio/editor/PholioMockEditor.php | 35 ++++- ...PhabricatorPholioMockTestDataGenerator.php | 1 + .../pholio/phid/PholioPHIDTypeImage.php | 47 ++++++ .../pholio/query/PholioImageQuery.php | 148 ++++++++++++++++++ .../pholio/query/PholioMockQuery.php | 39 ++--- .../pholio/storage/PholioImage.php | 56 +++++-- .../pholio/storage/PholioMock.php | 61 ++++++-- .../pholio/storage/PholioTransaction.php | 15 ++ .../pholio/view/PholioMockImagesView.php | 11 +- .../patch/PhabricatorBuiltinPatchList.php | 4 + .../pholio/behavior-pholio-mock-view.js | 7 + 21 files changed, 544 insertions(+), 100 deletions(-) create mode 100644 resources/sql/patches/20130722.pholioreplace.sql create mode 100644 src/applications/pholio/controller/PholioImageHistoryController.php create mode 100644 src/applications/pholio/phid/PholioPHIDTypeImage.php create mode 100644 src/applications/pholio/query/PholioImageQuery.php diff --git a/resources/sql/patches/20130722.pholioreplace.sql b/resources/sql/patches/20130722.pholioreplace.sql new file mode 100644 index 0000000000..132537ea05 --- /dev/null +++ b/resources/sql/patches/20130722.pholioreplace.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_pholio.pholio_image + ADD COLUMN replacesImagePHID VARCHAR(64) NULL COLLATE utf8_bin; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 3f1bc3d78b..f143e452c4 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1743,6 +1743,8 @@ phutil_register_library_map(array( 'PholioController' => 'applications/pholio/controller/PholioController.php', 'PholioDAO' => 'applications/pholio/storage/PholioDAO.php', 'PholioImage' => 'applications/pholio/storage/PholioImage.php', + 'PholioImageHistoryController' => 'applications/pholio/controller/PholioImageHistoryController.php', + 'PholioImageQuery' => 'applications/pholio/query/PholioImageQuery.php', 'PholioImageUploadController' => 'applications/pholio/controller/PholioImageUploadController.php', 'PholioInlineCommentEditView' => 'applications/pholio/view/PholioInlineCommentEditView.php', 'PholioInlineCommentSaveView' => 'applications/pholio/view/PholioInlineCommentSaveView.php', @@ -1764,6 +1766,7 @@ phutil_register_library_map(array( 'PholioMockQuery' => 'applications/pholio/query/PholioMockQuery.php', 'PholioMockSearchEngine' => 'applications/pholio/query/PholioMockSearchEngine.php', 'PholioMockViewController' => 'applications/pholio/controller/PholioMockViewController.php', + 'PholioPHIDTypeImage' => 'applications/pholio/phid/PholioPHIDTypeImage.php', 'PholioPHIDTypeMock' => 'applications/pholio/phid/PholioPHIDTypeMock.php', 'PholioRemarkupRule' => 'applications/pholio/remarkup/PholioRemarkupRule.php', 'PholioReplyHandler' => 'applications/pholio/mail/PholioReplyHandler.php', @@ -3795,7 +3798,10 @@ phutil_register_library_map(array( array( 0 => 'PholioDAO', 1 => 'PhabricatorMarkupInterface', + 2 => 'PhabricatorPolicyInterface', ), + 'PholioImageHistoryController' => 'PholioController', + 'PholioImageQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PholioImageUploadController' => 'PholioController', 'PholioInlineCommentEditView' => 'AphrontView', 'PholioInlineCommentSaveView' => 'AphrontView', @@ -3829,6 +3835,7 @@ phutil_register_library_map(array( 'PholioMockQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PholioMockSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PholioMockViewController' => 'PholioController', + 'PholioPHIDTypeImage' => 'PhabricatorPHIDType', 'PholioPHIDTypeMock' => 'PhabricatorPHIDType', 'PholioRemarkupRule' => 'PhabricatorRemarkupRuleObject', 'PholioReplyHandler' => 'PhabricatorMailReplyHandler', diff --git a/src/applications/phid/PhabricatorObjectHandle.php b/src/applications/phid/PhabricatorObjectHandle.php index 6b3fa3b544..ec8f2d35e0 100644 --- a/src/applications/phid/PhabricatorObjectHandle.php +++ b/src/applications/phid/PhabricatorObjectHandle.php @@ -109,7 +109,6 @@ final class PhabricatorObjectHandle static $map = array( PhabricatorPHIDConstants::PHID_TYPE_USER => 'User', - PhabricatorPHIDConstants::PHID_TYPE_PIMG => 'Pholio Image', PhabricatorPHIDConstants::PHID_TYPE_BLOG => 'Blog', PhabricatorPHIDConstants::PHID_TYPE_POST => 'Post', PhabricatorPHIDConstants::PHID_TYPE_LEGD => 'Legalpad Document', diff --git a/src/applications/phid/PhabricatorPHIDConstants.php b/src/applications/phid/PhabricatorPHIDConstants.php index bbba0c95a8..e5fd7a3ac1 100644 --- a/src/applications/phid/PhabricatorPHIDConstants.php +++ b/src/applications/phid/PhabricatorPHIDConstants.php @@ -17,7 +17,6 @@ final class PhabricatorPHIDConstants { const PHID_TYPE_TOBJ = 'TOBJ'; const PHID_TYPE_BLOG = 'BLOG'; const PHID_TYPE_ANSW = 'ANSW'; - const PHID_TYPE_PIMG = 'PIMG'; const PHID_TYPE_CONP = 'CONP'; const PHID_TYPE_ACNT = 'ACNT'; const PHID_TYPE_PDCT = 'PDCT'; diff --git a/src/applications/phid/handle/PhabricatorObjectHandleData.php b/src/applications/phid/handle/PhabricatorObjectHandleData.php index b4387efa0b..29e87c40d7 100644 --- a/src/applications/phid/handle/PhabricatorObjectHandleData.php +++ b/src/applications/phid/handle/PhabricatorObjectHandleData.php @@ -68,11 +68,6 @@ final class PhabricatorObjectHandleData { $phids); return mpull($projects, null, 'getPHID'); - case PhabricatorPHIDConstants::PHID_TYPE_PIMG: - $images = id(new PholioImage()) - ->loadAllWhere('phid IN (%Ls)', $phids); - return mpull($images, null, 'getPHID'); - case PhabricatorPHIDConstants::PHID_TYPE_XACT: $subtypes = array(); foreach ($phids as $phid) { @@ -301,25 +296,6 @@ final class PhabricatorObjectHandleData { } break; - case PhabricatorPHIDConstants::PHID_TYPE_PIMG: - foreach ($phids as $phid) { - $handle = new PhabricatorObjectHandle(); - $handle->setPHID($phid); - $handle->setType($type); - if (empty($objects[$phid])) { - $handle->setName('Unknown Image'); - } else { - $image = $objects[$phid]; - $handle->setName($image->getName()); - $handle->setFullName($image->getName()); - $handle->setURI( - '/M'.$image->getMockID().'/'.$image->getID().'/'); - $handle->setComplete(true); - } - $handles[$phid] = $handle; - } - break; - case PhabricatorPHIDConstants::PHID_TYPE_LEGD: foreach ($phids as $phid) { $handle = new PhabricatorObjectHandle(); diff --git a/src/applications/pholio/application/PhabricatorApplicationPholio.php b/src/applications/pholio/application/PhabricatorApplicationPholio.php index a5d5cb27f2..f3d460dd5c 100644 --- a/src/applications/pholio/application/PhabricatorApplicationPholio.php +++ b/src/applications/pholio/application/PhabricatorApplicationPholio.php @@ -58,6 +58,7 @@ final class PhabricatorApplicationPholio extends PhabricatorApplication { ), 'image/' => array( 'upload/' => 'PholioImageUploadController', + 'history/(?P\d+)/' => 'PholioImageHistoryController', ), ), ); diff --git a/src/applications/pholio/constants/PholioTransactionType.php b/src/applications/pholio/constants/PholioTransactionType.php index 77ea82462f..42713a0b12 100644 --- a/src/applications/pholio/constants/PholioTransactionType.php +++ b/src/applications/pholio/constants/PholioTransactionType.php @@ -13,6 +13,7 @@ final class PholioTransactionType extends PholioConstants { const TYPE_IMAGE_FILE = 'image-file'; const TYPE_IMAGE_NAME= 'image-name'; const TYPE_IMAGE_DESCRIPTION = 'image-description'; + const TYPE_IMAGE_REPLACE = 'image-replace'; /* your witty commentary at the mock : image : x,y level */ const TYPE_INLINE = 'inline'; diff --git a/src/applications/pholio/controller/PholioImageHistoryController.php b/src/applications/pholio/controller/PholioImageHistoryController.php new file mode 100644 index 0000000000..186a930057 --- /dev/null +++ b/src/applications/pholio/controller/PholioImageHistoryController.php @@ -0,0 +1,96 @@ +imageID = $data['id']; + } + + public function processRequest() { + $request = $this->getRequest(); + $user = $request->getUser(); + + $image = id(new PholioImageQuery()) + ->setViewer($user) + ->withIDs(array($this->imageID)) + ->executeOne(); + + if (!$image) { + return new Aphront404Response(); + } + + // note while we have a mock object, its missing images we need to show + // the history of what's happened here. + // fetch the real deal + // + $mock = id(new PholioMockQuery()) + ->setViewer($user) + ->needImages(true) + ->withIDs(array($image->getMockID())) + ->executeOne(); + + $phids = array($mock->getAuthorPHID()); + $this->loadHandles($phids); + + $engine = id(new PhabricatorMarkupEngine()) + ->setViewer($user); + $engine->addObject($mock, PholioMock::MARKUP_FIELD_DESCRIPTION); + $engine->process(); + + + $images = $mock->getImageHistorySet($this->imageID); + // TODO - this is a hack until we specialize the view object + $mock->attachImages($images); + $latest_image = last($images); + + $title = pht( + 'Image history for "%s" from the mock "%s."', + $latest_image->getName(), + $mock->getName()); + + $header = id(new PhabricatorHeaderView()) + ->setHeader($title); + + require_celerity_resource('pholio-css'); + require_celerity_resource('pholio-inline-comments-css'); + + $comment_form_id = celerity_generate_unique_node_id(); + $output = id(new PholioMockImagesView()) + ->setRequestURI($request->getRequestURI()) + ->setCommentFormID($comment_form_id) + ->setUser($user) + ->setMock($mock) + ->setImageID($this->imageID); + + $crumbs = $this->buildApplicationCrumbs(); + $crumbs + ->addCrumb( + id(new PhabricatorCrumbView()) + ->setName('M'.$mock->getID()) + ->setHref('/M'.$mock->getID())) + ->addCrumb( + id(new PhabricatorCrumbView()) + ->setName('Image History') + ->setHref($request->getRequestURI())); + + $content = array( + $crumbs, + $header, + $output->render(), + ); + + return $this->buildApplicationPage( + $content, + array( + 'title' => 'M'.$mock->getID().' '.$title, + 'device' => true, + 'pageObjects' => array($mock->getPHID()), + )); + } + +} diff --git a/src/applications/pholio/controller/PholioMockEditController.php b/src/applications/pholio/controller/PholioMockEditController.php index 40b6c514fa..7b938076b3 100644 --- a/src/applications/pholio/controller/PholioMockEditController.php +++ b/src/applications/pholio/controller/PholioMockEditController.php @@ -111,12 +111,41 @@ final class PholioMockEditController extends PholioController { } $sequence = 0; + $replaces = $request->getArr('replaces'); + $replaces_map = array_flip($replaces); + + /** + * Foreach file posted, check to see whether we are replacing an image, + * adding an image, or simply updating image metadata. Create + * transactions for these cases as appropos. + */ foreach ($files as $file_phid => $file) { - $mock_image = idx($mock_images, $file_phid); + $replaces_image_phid = null; + if (isset($replaces_map[$file_phid])) { + $old_file_phid = $replaces_map[$file_phid]; + $old_image = idx($mock_images, $old_file_phid); + if ($old_image) { + $replaces_image_phid = $old_image->getPHID(); + } + } + + $existing_image = idx($mock_images, $file_phid); + $title = (string)$request->getStr('title_'.$file_phid); $description = (string)$request->getStr('description_'.$file_phid); - if (!$mock_image) { - // this is an add + + if ($replaces_image_phid) { + $replace_image = id(new PholioImage()) + ->setReplacesImagePHID($replaces_image_phid) + ->setFilePhid($file_phid) + ->setName(strlen($title) ? $title : $file->getName()) + ->setDescription($description) + ->setSequence($sequence); + $xactions[] = id(new PholioTransaction()) + ->setTransactionType( + PholioTransactionType::TYPE_IMAGE_REPLACE) + ->setNewValue($replace_image); + } else if (!$existing_image) { // this is an add $add_image = id(new PholioImage()) ->setFilePhid($file_phid) ->setName(strlen($title) ? $title : $file->getName()) @@ -127,22 +156,22 @@ final class PholioMockEditController extends PholioController { ->setNewValue( array('+' => array($add_image))); } else { - // update (maybe) $xactions[] = id(new PholioTransaction()) ->setTransactionType(PholioTransactionType::TYPE_IMAGE_NAME) ->setNewValue( - array($mock_image->getPHID() => $title)); + array($existing_image->getPHID() => $title)); $xactions[] = id(new PholioTransaction()) ->setTransactionType( PholioTransactionType::TYPE_IMAGE_DESCRIPTION) - ->setNewValue(array($mock_image->getPHID() => $description)); - $mock_image->setSequence($sequence); + ->setNewValue( + array($existing_image->getPHID() => $description)); + $existing_image->setSequence($sequence); } $sequence++; } foreach ($mock_images as $file_phid => $mock_image) { - if (!isset($files[$file_phid])) { - // this is a delete + if (!isset($files[$file_phid]) && !isset($replaces[$file_phid])) { + // this is an outright delete $xactions[] = id(new PholioTransaction()) ->setTransactionType(PholioTransactionType::TYPE_IMAGE_FILE) ->setNewValue( diff --git a/src/applications/pholio/controller/PholioMockViewController.php b/src/applications/pholio/controller/PholioMockViewController.php index 96fdd37402..425aab97e1 100644 --- a/src/applications/pholio/controller/PholioMockViewController.php +++ b/src/applications/pholio/controller/PholioMockViewController.php @@ -76,6 +76,8 @@ final class PholioMockViewController extends PholioController { require_celerity_resource('pholio-css'); require_celerity_resource('pholio-inline-comments-css'); + $image_status = $this->getImageStatus($mock, $this->imageID); + $comment_form_id = celerity_generate_unique_node_id(); $output = id(new PholioMockImagesView()) ->setRequestURI($request->getRequestURI()) @@ -100,6 +102,7 @@ final class PholioMockViewController extends PholioController { $content = array( $crumbs, + $image_status, $header, $actions, $properties, @@ -117,6 +120,43 @@ final class PholioMockViewController extends PholioController { )); } + private function getImageStatus(PholioMock $mock, $image_id) { + $status = null; + $images = $mock->getImages(); + foreach ($images as $image) { + if ($image->getID() == $image_id) { + return $status; + } + } + + $images = $mock->getAllImages(); + $images = mpull($images, null, 'getID'); + $image = idx($images, $image_id); + + if ($image) { + $history = $mock->getImageUpdateSet($image_id); + $latest_image = last($history); + $href = $this->getApplicationURI( + 'image/history/'.$latest_image->getID().'/'); + $status = id(new AphrontErrorView()) + ->setSeverity(AphrontErrorView::SEVERITY_NOTICE) + ->setTitle(pht('The requested image is obsolete.')) + ->appendChild(phutil_tag( + 'p', + array(), + array( + pht('You are viewing this mock with the latest image set.'), + ' ', + phutil_tag( + 'a', + array('href' => $href), + pht( + 'Click here to see the history of the now obsolete image.'))))); + } + + return $status; + } + private function buildActionView(PholioMock $mock) { $user = $this->getRequest()->getUser(); diff --git a/src/applications/pholio/editor/PholioMockEditor.php b/src/applications/pholio/editor/PholioMockEditor.php index d5ef09248d..6da81252db 100644 --- a/src/applications/pholio/editor/PholioMockEditor.php +++ b/src/applications/pholio/editor/PholioMockEditor.php @@ -30,6 +30,7 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { $types[] = PholioTransactionType::TYPE_IMAGE_FILE; $types[] = PholioTransactionType::TYPE_IMAGE_NAME; $types[] = PholioTransactionType::TYPE_IMAGE_DESCRIPTION; + $types[] = PholioTransactionType::TYPE_IMAGE_REPLACE; return $types; } @@ -64,6 +65,9 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { $phid = $image->getPHID(); } return array($phid => $description); + case PholioTransactionType::TYPE_IMAGE_REPLACE: + $raw = $xaction->getNewValue(); + return $raw->getReplacesImagePHID(); } } @@ -77,6 +81,9 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { case PholioTransactionType::TYPE_IMAGE_NAME: case PholioTransactionType::TYPE_IMAGE_DESCRIPTION: return $xaction->getNewValue(); + case PholioTransactionType::TYPE_IMAGE_REPLACE: + $raw = $xaction->getNewValue(); + return $raw->getPHID(); case PholioTransactionType::TYPE_IMAGE_FILE: $raw_new_value = $xaction->getNewValue(); $new_value = array(); @@ -107,6 +114,7 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { case PholioTransactionType::TYPE_IMAGE_FILE: + case PholioTransactionType::TYPE_IMAGE_REPLACE: return true; break; } @@ -133,6 +141,11 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { } } break; + case PholioTransactionType::TYPE_IMAGE_REPLACE: + $image = $xaction->getNewValue(); + $image->save(); + $new_images[] = $image; + break; } } $this->setNewImages($new_images); @@ -189,9 +202,21 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { } $object->attachImages($images); break; - case PholioTransactionType::TYPE_IMAGE_NAME: - $image = $this->getImageForXaction($object, $xaction); - $value = (string) head($xaction->getNewValue()); + case PholioTransactionType::TYPE_IMAGE_REPLACE: + $old = $xaction->getOldValue(); + $images = $object->getImages(); + foreach ($images as $seq => $image) { + if ($image->getPHID() == $old) { + $image->setIsObsolete(1); + $image->save(); + unset($images[$seq]); + } + } + $object->attachImages($images); + break; + case PholioTransactionType::TYPE_IMAGE_NAME: + $image = $this->getImageForXaction($object, $xaction); + $value = (string) head($xaction->getNewValue()); $image->setName($value); $image->save(); break; @@ -224,6 +249,10 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { case PholioTransactionType::TYPE_NAME: case PholioTransactionType::TYPE_DESCRIPTION: return $v; + case PholioTransactionType::TYPE_IMAGE_REPLACE: + if ($u->getNewValue() == $v->getOldValue()) { + return $v; + } case PholioTransactionType::TYPE_IMAGE_FILE: return $this->mergePHIDOrEdgeTransactions($u, $v); case PholioTransactionType::TYPE_IMAGE_NAME: diff --git a/src/applications/pholio/lipsum/PhabricatorPholioMockTestDataGenerator.php b/src/applications/pholio/lipsum/PhabricatorPholioMockTestDataGenerator.php index fe36aa67f0..232ae91caf 100644 --- a/src/applications/pholio/lipsum/PhabricatorPholioMockTestDataGenerator.php +++ b/src/applications/pholio/lipsum/PhabricatorPholioMockTestDataGenerator.php @@ -42,6 +42,7 @@ final class PhabricatorPholioMockTestDataGenerator $image = new PholioImage(); $image->setFilePHID($file->getPHID()); $image->setSequence($sequence++); + $image->attachMock($mock); $images[] = $image; } diff --git a/src/applications/pholio/phid/PholioPHIDTypeImage.php b/src/applications/pholio/phid/PholioPHIDTypeImage.php new file mode 100644 index 0000000000..a9696803b7 --- /dev/null +++ b/src/applications/pholio/phid/PholioPHIDTypeImage.php @@ -0,0 +1,47 @@ +setViewer($query->getViewer()) + ->withPHIDs($phids) + ->execute(); + } + + public function loadHandles( + PhabricatorHandleQuery $query, + array $handles, + array $objects) { + + foreach ($handles as $phid => $handle) { + $image = $objects[$phid]; + + $id = $image->getID(); + $mock_id = $image->getMockID(); + $name = $image->getName(); + + $handle->setURI("/M{$mock_id}/{$id}/"); + $handle->setName($name); + $handle->setFullName($name); + } + } + +} diff --git a/src/applications/pholio/query/PholioImageQuery.php b/src/applications/pholio/query/PholioImageQuery.php new file mode 100644 index 0000000000..0f2da89863 --- /dev/null +++ b/src/applications/pholio/query/PholioImageQuery.php @@ -0,0 +1,148 @@ +ids = $ids; + return $this; + } + + public function withPHIDs(array $phids) { + $this->phids = $phids; + return $this; + } + + public function withMockIDs(array $mock_ids) { + $this->mockIDs = $mock_ids; + return $this; + } + + public function withObsolete($obsolete) { + $this->obsolete = $obsolete; + return $this; + } + + public function needInlineComments($need_inline_comments) { + $this->needInlineComments = $need_inline_comments; + return $this; + } + + public function setMockCache($mock_cache) { + $this->mockCache = $mock_cache; + return $this; + } + public function getMockCache() { + return $this->mockCache; + } + + protected function loadPage() { + $table = new PholioImage(); + $conn_r = $table->establishConnection('r'); + + $data = queryfx_all( + $conn_r, + 'SELECT * FROM %T %Q %Q %Q', + $table->getTableName(), + $this->buildWhereClause($conn_r), + $this->buildOrderClause($conn_r), + $this->buildLimitClause($conn_r)); + + $images = $table->loadAllFromArray($data); + + return $images; + } + + private function buildWhereClause(AphrontDatabaseConnection $conn_r) { + $where = array(); + + $where[] = $this->buildPagingClause($conn_r); + + if ($this->ids) { + $where[] = qsprintf( + $conn_r, + 'id IN (%Ld)', + $this->ids); + } + + if ($this->phids) { + $where[] = qsprintf( + $conn_r, + 'phid IN (%Ls)', + $this->phids); + } + + if ($this->mockIDs) { + $where[] = qsprintf( + $conn_r, + 'mockID IN (%Ld)', + $this->mockIDs); + } + + if ($this->obsolete !== null) { + $where[] = qsprintf( + $conn_r, + 'isObsolete = %d', + $this->obsolete); + } + + return $this->formatWhereClause($where); + } + + protected function willFilterPage(array $images) { + assert_instances_of($images, 'PholioImage'); + + $file_phids = mpull($images, 'getFilePHID'); + $all_files = mpull(id(new PhabricatorFile())->loadAllWhere( + 'phid IN (%Ls)', + $file_phids), null, 'getPHID'); + + if ($this->needInlineComments) { + $all_inline_comments = id(new PholioTransactionComment()) + ->loadAllWhere('imageid IN (%Ld)', + mpull($images, 'getID')); + $all_inline_comments = mgroup($all_inline_comments, 'getImageID'); + } + + foreach ($images as $image) { + $file = idx($all_files, $image->getFilePHID()); + if (!$file) { + $file = PhabricatorFile::loadBuiltin($this->getViewer(), 'missing.png'); + } + $image->attachFile($file); + if ($this->needInlineComments) { + $inlines = idx($all_inline_comments, $image->getID(), array()); + $image->attachInlineComments($inlines); + } + } + + if ($this->getMockCache()) { + $mocks = $this->getMockCache(); + } else { + $mock_ids = mpull($images, 'getMockID'); + // DO NOT set needImages to true; recursion results! + $mocks = id(new PholioMockQuery()) + ->setViewer($this->getViewer()) + ->withIDs($mock_ids) + ->execute(); + $mocks = mpull($mocks, null, 'getID'); + } + foreach ($images as $image) { + $image->attachMock($mocks[$image->getMockID()]); + } + + return $images; + } + +} diff --git a/src/applications/pholio/query/PholioMockQuery.php b/src/applications/pholio/query/PholioMockQuery.php index 9ebd849d8d..be350a7a3a 100644 --- a/src/applications/pholio/query/PholioMockQuery.php +++ b/src/applications/pholio/query/PholioMockQuery.php @@ -111,41 +111,20 @@ final class PholioMockQuery private function loadImages(array $mocks) { assert_instances_of($mocks, 'PholioMock'); - $mock_ids = mpull($mocks, 'getID'); - $all_images = id(new PholioImage())->loadAllWhere( - 'mockID IN (%Ld) AND isObsolete = %d', - $mock_ids, - 0); - - $file_phids = mpull($all_images, 'getFilePHID'); - $all_files = mpull(id(new PhabricatorFile())->loadAllWhere( - 'phid IN (%Ls)', - $file_phids), null, 'getPHID'); - - if ($this->needInlineComments) { - $all_inline_comments = id(new PholioTransactionComment()) - ->loadAllWhere('imageid IN (%Ld)', - mpull($all_images, 'getID')); - $all_inline_comments = mgroup($all_inline_comments, 'getImageID'); - } - - foreach ($all_images as $image) { - $file = idx($all_files, $image->getFilePHID()); - if (!$file) { - $file = PhabricatorFile::loadBuiltin($this->getViewer(), 'missing.png'); - } - $image->attachFile($file); - if ($this->needInlineComments) { - $inlines = idx($all_images, $image->getID(), array()); - $image->attachInlineComments($inlines); - } - } + $mock_map = mpull($mocks, null, 'getID'); + $all_images = id(new PholioImageQuery()) + ->setViewer($this->getViewer()) + ->setMockCache($mock_map) + ->withMockIDs(array_keys($mock_map)) + ->needInlineComments($this->needInlineComments) + ->execute(); $image_groups = mgroup($all_images, 'getMockID'); foreach ($mocks as $mock) { $mock_images = $image_groups[$mock->getID()]; - $mock->attachImages($mock_images); + $mock->attachAllImages($mock_images); + $mock->attachImages(mfilter($mock_images, 'getIsObsolete', true)); } } diff --git a/src/applications/pholio/storage/PholioImage.php b/src/applications/pholio/storage/PholioImage.php index 89770a2c6e..74d16ef1b7 100644 --- a/src/applications/pholio/storage/PholioImage.php +++ b/src/applications/pholio/storage/PholioImage.php @@ -4,7 +4,9 @@ * @group pholio */ final class PholioImage extends PholioDAO - implements PhabricatorMarkupInterface { + implements + PhabricatorMarkupInterface, + PhabricatorPolicyInterface { const MARKUP_FIELD_DESCRIPTION = 'markup:description'; @@ -14,9 +16,11 @@ final class PholioImage extends PholioDAO protected $description = ''; protected $sequence; protected $isObsolete = 0; + protected $replacesImagePHID = null; - private $inlineComments; - private $file; + private $inlineComments = self::ATTACHABLE; + private $file = self::ATTACHABLE; + private $mock = self::ATTACHABLE; public function getConfiguration() { return array( @@ -25,9 +29,30 @@ final class PholioImage extends PholioDAO } public function generatePHID() { - return PhabricatorPHID::generateNewPHID('PIMG'); + return PhabricatorPHID::generateNewPHID(PholioPHIDTypeImage::TYPECONST); } + public function attachFile(PhabricatorFile $file) { + $this->file = $file; + return $this; + } + + public function getFile() { + $this->assertAttached($this->file); + return $this->file; + } + + public function attachMock(PholioMock $mock) { + $this->mock = $mock; + return $this; + } + + public function getMock() { + $this->assertAttached($this->mock); + return $this->mock; + } + + public function attachInlineComments(array $inline_comments) { assert_instances_of($inline_comments, 'PholioTransactionComment'); $this->inlineComments = $inline_comments; @@ -35,9 +60,7 @@ final class PholioImage extends PholioDAO } public function getInlineComments() { - if ($this->inlineComments === null) { - throw new Exception("Call attachImages() before getImages()!"); - } + $this->assertAttached($this->inlineComments); return $this->inlineComments; } @@ -66,16 +89,19 @@ final class PholioImage extends PholioDAO return (bool)$this->getID(); } - public function attachFile(PhabricatorFile $file) { - $this->file = $file; - return $this; +/* -( PhabricatorPolicyInterface Implementation )-------------------------- */ + + public function getCapabilities() { + return $this->getMock()->getCapabilities(); } - public function getFile() { - if ($this->file === null) { - throw new Exception("Call attachFile() before getFile()!"); - } - return $this->file; + public function getPolicy($capability) { + return $this->getMock()->getPolicy($capability); + } + + // really the *mock* controls who can see an image + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + return $this->getMock()->hasAutomaticCapability($capability, $viewer); } } diff --git a/src/applications/pholio/storage/PholioMock.php b/src/applications/pholio/storage/PholioMock.php index 5574c310a8..8bec0a8e09 100644 --- a/src/applications/pholio/storage/PholioMock.php +++ b/src/applications/pholio/storage/PholioMock.php @@ -22,9 +22,10 @@ final class PholioMock extends PholioDAO protected $coverPHID; protected $mailKey; - private $images; - private $coverFile; - private $tokenCount; + private $images = self::ATTACHABLE; + private $allImages = self::ATTACHABLE; + private $coverFile = self::ATTACHABLE; + private $tokenCount = self::ATTACHABLE; public function getConfiguration() { return array( @@ -43,6 +44,9 @@ final class PholioMock extends PholioDAO return parent::save(); } + /** + * These should be the images currently associated with the Mock. + */ public function attachImages(array $images) { assert_instances_of($images, 'PholioImage'); $this->images = $images; @@ -50,28 +54,37 @@ final class PholioMock extends PholioDAO } public function getImages() { - if ($this->images === null) { - throw new Exception("Call attachImages() before getImages()!"); - } + $this->assertAttached($this->images); return $this->images; } + /** + * These should be *all* images associated with the Mock. This includes + * images which have been removed and / or replaced from the Mock. + */ + public function attachAllImages(array $images) { + assert_instances_of($images, 'PholioImage'); + $this->allImages = $images; + return $this; + } + + public function getAllImages() { + $this->assertAttached($this->images); + return $this->allImages; + } + public function attachCoverFile(PhabricatorFile $file) { $this->coverFile = $file; return $this; } public function getCoverFile() { - if ($this->coverFile === null) { - throw new Exception("Call attachCoverFile() before getCoverFile()!"); - } + $this->assertAttached($this->coverFile); return $this->coverFile; } public function getTokenCount() { - if ($this->tokenCount === null) { - throw new Exception("Call attachTokenCount() before getTokenCount()!"); - } + $this->assertAttached($this->tokenCount); return $this->tokenCount; } @@ -80,6 +93,30 @@ final class PholioMock extends PholioDAO return $this; } + public function getImageHistorySet($image_id) { + $images = $this->getAllImages(); + $images = mpull($images, null, 'getID'); + $selected_image = $images[$image_id]; + + $replace_map = mpull($images, null, 'getReplacesImagePHID'); + $phid_map = mpull($images, null, 'getPHID'); + + // find the earliest image + $image = $selected_image; + while (isset($phid_map[$image->getReplacesImagePHID()])) { + $image = $phid_map[$image->getReplacesImagePHID()]; + } + + // now build history moving forward + $history = array($image->getID() => $image); + while (isset($replace_map[$image->getPHID()])) { + $image = $replace_map[$image->getPHID()]; + $history[$image->getID()] = $image; + } + + return $history; + } + /* -( PhabricatorSubscribableInterface Implementation )-------------------- */ diff --git a/src/applications/pholio/storage/PholioTransaction.php b/src/applications/pholio/storage/PholioTransaction.php index 93ec4d4335..a36c59e382 100644 --- a/src/applications/pholio/storage/PholioTransaction.php +++ b/src/applications/pholio/storage/PholioTransaction.php @@ -36,6 +36,10 @@ final class PholioTransaction extends PhabricatorApplicationTransaction { case PholioTransactionType::TYPE_IMAGE_FILE: $phids = array_merge($phids, $new, $old); break; + case PholioTransactionType::TYPE_IMAGE_REPLACE: + $phids[] = $new; + $phids[] = $old; + break; case PholioTransactionType::TYPE_IMAGE_DESCRIPTION: case PholioTransactionType::TYPE_IMAGE_NAME: $phids[] = key($new); @@ -69,6 +73,7 @@ final class PholioTransaction extends PhabricatorApplicationTransaction { case PholioTransactionType::TYPE_IMAGE_DESCRIPTION: return 'edit'; case PholioTransactionType::TYPE_IMAGE_FILE: + case PholioTransactionType::TYPE_IMAGE_REPLACE: return 'attach'; } @@ -115,6 +120,13 @@ final class PholioTransaction extends PhabricatorApplicationTransaction { $this->renderHandleLink($author_phid), $count); break; + case PholioTransactionType::TYPE_IMAGE_REPLACE: + return pht( + '%s replaced %s with %s.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink($old), + $this->renderHandleLink($new)); + break; case PholioTransactionType::TYPE_IMAGE_FILE: $add = array_diff($new, $old); $rem = array_diff($old, $new); @@ -197,6 +209,7 @@ final class PholioTransaction extends PhabricatorApplicationTransaction { $this->renderHandleLink($author_phid), $this->renderHandleLink($object_phid)); break; + case PholioTransactionType::TYPE_IMAGE_REPLACE: case PholioTransactionType::TYPE_IMAGE_FILE: return pht( '%s updated images of %s.', @@ -259,6 +272,8 @@ final class PholioTransaction extends PhabricatorApplicationTransaction { case PholioTransactionType::TYPE_IMAGE_NAME: case PholioTransactionType::TYPE_IMAGE_DESCRIPTION: return PhabricatorTransactions::COLOR_BLUE; + case PholioTransactionType::TYPE_IMAGE_REPLACE: + return PhabricatorTransactions::COLOR_YELLOW; case PholioTransactionType::TYPE_IMAGE_FILE: $add = array_diff($new, $old); $rem = array_diff($old, $new); diff --git a/src/applications/pholio/view/PholioMockImagesView.php b/src/applications/pholio/view/PholioMockImagesView.php index 39654709b9..d8d8dd0563 100644 --- a/src/applications/pholio/view/PholioMockImagesView.php +++ b/src/applications/pholio/view/PholioMockImagesView.php @@ -67,13 +67,14 @@ final class PholioMockImagesView extends AphrontView { $y = idx($metadata, PhabricatorFile::METADATA_IMAGE_HEIGHT); $images[] = array( - 'id' => $image->getID(), + 'id' => $image->getID(), 'fullURI' => $image->getFile()->getBestURI(), 'pageURI' => '/M'.$mock->getID().'/'.$image->getID().'/', - 'width' => $x, - 'height' => $y, - 'title' => $image->getName(), - 'desc' => $image->getDescription(), + 'historyURI' => '/pholio/image/history/'.$image->getID().'/', + 'width' => $x, + 'height' => $y, + 'title' => $image->getName(), + 'desc' => $image->getDescription(), ); } diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index 32b6fe24cd..b74267fd5a 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1475,6 +1475,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('20130723.taskstarttime.sql'), ), + '20130722.pholioreplace.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('20130722.pholioreplace.sql'), + ), ); } } diff --git a/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js b/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js index 06b6530a5e..964a86d21b 100644 --- a/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js +++ b/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js @@ -728,6 +728,13 @@ JX.behavior('pholio-mock-view', function(config) { 'View Full Image'); info.push(full_link); + var history_link = JX.$N( + 'a', + { href: image.historyURI }, + 'View Image History'); + info.push(history_link); + + for (var ii = 0; ii < info.length; ii++) { info[ii] = JX.$N('div', {className: 'pholio-image-info-item'}, info[ii]); }