diff --git a/src/applications/pholio/controller/PholioMockEditController.php b/src/applications/pholio/controller/PholioMockEditController.php --- a/src/applications/pholio/controller/PholioMockEditController.php +++ b/src/applications/pholio/controller/PholioMockEditController.php @@ -143,20 +143,22 @@ $replace_image = PholioImage::initializeNewImage() ->setAuthorPHID($viewer->getPHID()) ->setReplacesImagePHID($replaces_image_phid) - ->setFilePhid($file_phid) + ->setFilePHID($file_phid) ->attachFile($file) ->setName(strlen($title) ? $title : $file->getName()) ->setDescription($description) - ->setSequence($sequence); + ->setSequence($sequence) + ->save(); + $xactions[] = id(new PholioTransaction()) - ->setTransactionType( - PholioImageReplaceTransaction::TRANSACTIONTYPE) - ->setNewValue($replace_image); + ->setTransactionType(PholioImageReplaceTransaction::TRANSACTIONTYPE) + ->setNewValue($replace_image->getPHID()); + $posted_mock_images[] = $replace_image; } else if (!$existing_image) { // this is an add $add_image = PholioImage::initializeNewImage() ->setAuthorPHID($viewer->getPHID()) - ->setFilePhid($file_phid) + ->setFilePHID($file_phid) ->attachFile($file) ->setName(strlen($title) ? $title : $file->getName()) ->setDescription($description) @@ -202,7 +204,6 @@ ->setMetadataValue('edge:type', $proj_edge_type) ->setNewValue(array('=' => array_fuse($v_projects))); - $mock->openTransaction(); $editor = id(new PholioMockEditor()) ->setContentSourceFromRequest($request) ->setContinueOnNoEffect(true) @@ -210,8 +211,6 @@ $xactions = $editor->applyTransactions($mock, $xactions); - $mock->saveTransaction(); - return id(new AphrontRedirectResponse()) ->setURI('/M'.$mock->getID()); } diff --git a/src/applications/pholio/editor/PholioMockEditor.php b/src/applications/pholio/editor/PholioMockEditor.php --- a/src/applications/pholio/editor/PholioMockEditor.php +++ b/src/applications/pholio/editor/PholioMockEditor.php @@ -4,6 +4,8 @@ private $newImages = array(); + private $images = array(); + public function getEditorApplicationClass() { return 'PhabricatorPholioApplication'; } @@ -48,9 +50,7 @@ foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { case PholioImageFileTransaction::TRANSACTIONTYPE: - case PholioImageReplaceTransaction::TRANSACTIONTYPE: return true; - break; } } return false; @@ -75,11 +75,6 @@ } } break; - case PholioImageReplaceTransaction::TRANSACTIONTYPE: - $image = $xaction->getNewValue(); - $image->save(); - $new_images[] = $image; - break; } } $this->setNewImages($new_images); @@ -275,4 +270,37 @@ return parent::shouldImplyCC($object, $xaction); } + public function loadPholioImage($object, $phid) { + if (!isset($this->images[$phid])) { + + $image = id(new PholioImageQuery()) + ->setViewer($this->getActor()) + ->withPHIDs(array($phid)) + ->executeOne(); + + if (!$image) { + throw new Exception( + pht( + 'No image exists with PHID "%s".', + $phid)); + } + + $mock_phid = $image->getMockPHID(); + if ($mock_phid) { + if ($mock_phid !== $object->getPHID()) { + throw new Exception( + pht( + 'Image ("%s") belongs to the wrong object ("%s", expected "%s").', + $phid, + $mock_phid, + $object->getPHID())); + } + } + + $this->images[$phid] = $image; + } + + return $this->images[$phid]; + } + } diff --git a/src/applications/pholio/xaction/PholioImageReplaceTransaction.php b/src/applications/pholio/xaction/PholioImageReplaceTransaction.php --- a/src/applications/pholio/xaction/PholioImageReplaceTransaction.php +++ b/src/applications/pholio/xaction/PholioImageReplaceTransaction.php @@ -6,25 +6,25 @@ const TRANSACTIONTYPE = 'image-replace'; public function generateOldValue($object) { - $new_image = $this->getNewValue(); - return $new_image->getReplacesImagePHID(); - } + $editor = $this->getEditor(); + $new_phid = $this->getNewValue(); - public function generateNewValue($object, $value) { - return $value->getPHID(); + return $editor->loadPholioImage($object, $new_phid) + ->getReplacesImagePHID(); } - public function applyInternalEffects($object, $value) { - $old = $this->getOldValue(); - $images = $object->getImages(); - foreach ($images as $seq => $image) { - if ($image->getPHID() == $old) { - $image->setIsObsolete(1); - $image->save(); - unset($images[$seq]); - } - } - $object->attachImages($images); + public function applyExternalEffects($object, $value) { + $editor = $this->getEditor(); + $old_phid = $this->getOldValue(); + + $old_image = $editor->loadPholioImage($object, $old_phid) + ->setIsObsolete(1) + ->save(); + + $editor->loadPholioImage($object, $value) + ->setMockPHID($object->getPHID()) + ->setSequence($old_image->getSequence()) + ->save(); } public function getTitle() { @@ -54,27 +54,87 @@ $object, PhabricatorApplicationTransaction $u, PhabricatorApplicationTransaction $v) { - $u_img = $u->getNewValue(); - $v_img = $v->getNewValue(); - if ($u_img->getReplacesImagePHID() == $v_img->getReplacesImagePHID()) { + + $u_phid = $u->getOldValue(); + $v_phid = $v->getOldValue(); + + if ($u_phid === $v_phid) { return $v; } + + return null; } public function extractFilePHIDs($object, $value) { - $file_phids = array(); + $editor = $this->getEditor(); + + $file_phid = $editor->loadPholioImage($object, $value) + ->getFilePHID(); + + return array($file_phid); + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + $mock_phid = $object->getPHID(); $editor = $this->getEditor(); - $images = $editor->getNewImages(); - foreach ($images as $image) { - if ($image->getPHID() !== $value) { + foreach ($xactions as $xaction) { + $new_phid = $xaction->getNewValue(); + + try { + $new_image = $editor->loadPholioImage($object, $new_phid); + } catch (Exception $ex) { + $errors[] = $this->newInvalidError( + pht( + 'Unable to load replacement image ("%s"): %s', + $new_phid, + $ex->getMessage()), + $xaction); continue; } - $file_phids[] = $image->getFilePHID(); + $old_phid = $new_image->getReplacesImagePHID(); + if (!$old_phid) { + $errors[] = $this->newInvalidError( + pht( + 'Image ("%s") does not specify which image it replaces.', + $new_phid), + $xaction); + continue; + } + + try { + $old_image = $editor->loadPholioImage($object, $old_phid); + } catch (Exception $ex) { + $errors[] = $this->newInvalidError( + pht( + 'Unable to load replaced image ("%s"): %s', + $old_phid, + $ex->getMessage()), + $xaction); + continue; + } + + if ($old_image->getMockPHID() !== $mock_phid) { + $errors[] = $this->newInvalidError( + pht( + 'Replaced image ("%s") belongs to the wrong mock ("%s", expected '. + '"%s").', + $old_phid, + $old_image->getMockPHID(), + $mock_phid), + $xaction); + continue; + } + + // TODO: You shouldn't be able to replace an image if it has already + // been replaced. + } - return $file_phids; + return $errors; } }