diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3621,6 +3621,7 @@ 'PhabricatorProjectHovercardEngineExtension' => 'applications/project/engineextension/PhabricatorProjectHovercardEngineExtension.php', 'PhabricatorProjectIconSet' => 'applications/project/icon/PhabricatorProjectIconSet.php', 'PhabricatorProjectIconsConfigOptionType' => 'applications/project/config/PhabricatorProjectIconsConfigOptionType.php', + 'PhabricatorProjectImageTransaction' => 'applications/project/xaction/PhabricatorProjectImageTransaction.php', 'PhabricatorProjectInterface' => 'applications/project/interface/PhabricatorProjectInterface.php', 'PhabricatorProjectListController' => 'applications/project/controller/PhabricatorProjectListController.php', 'PhabricatorProjectListView' => 'applications/project/view/PhabricatorProjectListView.php', @@ -9035,6 +9036,7 @@ 'PhabricatorProjectHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension', 'PhabricatorProjectIconSet' => 'PhabricatorIconSet', 'PhabricatorProjectIconsConfigOptionType' => 'PhabricatorConfigJSONOptionType', + 'PhabricatorProjectImageTransaction' => 'PhabricatorProjectTransactionType', 'PhabricatorProjectListController' => 'PhabricatorProjectController', 'PhabricatorProjectListView' => 'AphrontView', 'PhabricatorProjectLockController' => 'PhabricatorProjectController', diff --git a/src/applications/files/controller/PhabricatorFileComposeController.php b/src/applications/files/controller/PhabricatorFileComposeController.php --- a/src/applications/files/controller/PhabricatorFileComposeController.php +++ b/src/applications/files/controller/PhabricatorFileComposeController.php @@ -48,7 +48,8 @@ $xactions = array(); $xactions[] = id(new PhabricatorProjectTransaction()) - ->setTransactionType(PhabricatorProjectTransaction::TYPE_IMAGE) + ->setTransactionType( + PhabricatorProjectImageTransaction::TRANSACTIONTYPE) ->setNewValue($file->getPHID()); $editor = id(new PhabricatorProjectTransactionEditor()) diff --git a/src/applications/project/controller/PhabricatorProjectEditPictureController.php b/src/applications/project/controller/PhabricatorProjectEditPictureController.php --- a/src/applications/project/controller/PhabricatorProjectEditPictureController.php +++ b/src/applications/project/controller/PhabricatorProjectEditPictureController.php @@ -78,7 +78,8 @@ $xactions = array(); $xactions[] = id(new PhabricatorProjectTransaction()) - ->setTransactionType(PhabricatorProjectTransaction::TYPE_IMAGE) + ->setTransactionType( + PhabricatorProjectImageTransaction::TRANSACTIONTYPE) ->setNewValue($new_value); $editor = id(new PhabricatorProjectTransactionEditor()) diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php --- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php +++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php @@ -30,7 +30,6 @@ $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; $types[] = PhabricatorTransactions::TYPE_JOIN_POLICY; - $types[] = PhabricatorProjectTransaction::TYPE_IMAGE; $types[] = PhabricatorProjectTransaction::TYPE_ICON; $types[] = PhabricatorProjectTransaction::TYPE_COLOR; $types[] = PhabricatorProjectTransaction::TYPE_LOCKED; @@ -49,8 +48,6 @@ PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { - case PhabricatorProjectTransaction::TYPE_IMAGE: - return $object->getProfileImagePHID(); case PhabricatorProjectTransaction::TYPE_ICON: return $object->getIcon(); case PhabricatorProjectTransaction::TYPE_COLOR: @@ -78,7 +75,6 @@ PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { - case PhabricatorProjectTransaction::TYPE_IMAGE: case PhabricatorProjectTransaction::TYPE_ICON: case PhabricatorProjectTransaction::TYPE_COLOR: case PhabricatorProjectTransaction::TYPE_LOCKED: @@ -105,9 +101,6 @@ PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { - case PhabricatorProjectTransaction::TYPE_IMAGE: - $object->setProfileImagePHID($xaction->getNewValue()); - return; case PhabricatorProjectTransaction::TYPE_ICON: $object->setIcon($xaction->getNewValue()); return; @@ -150,7 +143,6 @@ $new = $xaction->getNewValue(); switch ($xaction->getTransactionType()) { - case PhabricatorProjectTransaction::TYPE_IMAGE: case PhabricatorProjectTransaction::TYPE_ICON: case PhabricatorProjectTransaction::TYPE_COLOR: case PhabricatorProjectTransaction::TYPE_LOCKED: @@ -336,7 +328,7 @@ switch ($xaction->getTransactionType()) { case PhabricatorProjectNameTransaction::TRANSACTIONTYPE: case PhabricatorProjectStatusTransaction::TRANSACTIONTYPE: - case PhabricatorProjectTransaction::TYPE_IMAGE: + case PhabricatorProjectImageTransaction::TRANSACTIONTYPE: case PhabricatorProjectTransaction::TYPE_ICON: case PhabricatorProjectTransaction::TYPE_COLOR: PhabricatorPolicyFilter::requireCapability( @@ -475,22 +467,6 @@ return true; } - protected function extractFilePHIDsFromCustomTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorProjectTransaction::TYPE_IMAGE: - $new = $xaction->getNewValue(); - if ($new) { - return array($new); - } - break; - } - - return parent::extractFilePHIDsFromCustomTransaction($object, $xaction); - } - protected function applyFinalEffects( PhabricatorLiskDAO $object, array $xactions) { diff --git a/src/applications/project/storage/PhabricatorProjectTransaction.php b/src/applications/project/storage/PhabricatorProjectTransaction.php --- a/src/applications/project/storage/PhabricatorProjectTransaction.php +++ b/src/applications/project/storage/PhabricatorProjectTransaction.php @@ -3,7 +3,6 @@ final class PhabricatorProjectTransaction extends PhabricatorModularTransaction { - const TYPE_IMAGE = 'project:image'; const TYPE_ICON = 'project:icon'; const TYPE_COLOR = 'project:color'; const TYPE_LOCKED = 'project:locked'; @@ -45,10 +44,6 @@ $rem = array_diff($old, $new); $req_phids = array_merge($add, $rem); break; - case self::TYPE_IMAGE: - $req_phids[] = $old; - $req_phids[] = $new; - break; } return array_merge($req_phids, parent::getRequiredHandlePHIDs()); @@ -106,8 +101,6 @@ } case self::TYPE_ICON: return PhabricatorProjectIconSet::getIconIcon($new); - case self::TYPE_IMAGE: - return 'fa-photo'; case self::TYPE_MEMBERS: return 'fa-user'; } @@ -126,26 +119,6 @@ '%s created this project.', $this->renderHandleLink($author_phid)); - case self::TYPE_IMAGE: - // TODO: Some day, it would be nice to show the images. - if (!$old) { - return pht( - "%s set this project's image to %s.", - $author_handle, - $this->renderHandleLink($new)); - } else if (!$new) { - return pht( - "%s removed this project's image.", - $author_handle); - } else { - return pht( - "%s updated this project's image from %s to %s.", - $author_handle, - $this->renderHandleLink($old), - $this->renderHandleLink($new)); - } - break; - case self::TYPE_ICON: $set = new PhabricatorProjectIconSet(); @@ -253,27 +226,6 @@ $new = $this->getNewValue(); switch ($this->getTransactionType()) { - case self::TYPE_IMAGE: - // TODO: Some day, it would be nice to show the images. - if (!$old) { - return pht( - '%s set the image for %s to %s.', - $author_handle, - $object_handle, - $this->renderHandleLink($new)); - } else if (!$new) { - return pht( - '%s removed the image for %s.', - $author_handle, - $object_handle); - } else { - return pht( - '%s updated the image for %s from %s to %s.', - $author_handle, - $object_handle, - $this->renderHandleLink($old), - $this->renderHandleLink($new)); - } case self::TYPE_ICON: $set = new PhabricatorProjectIconSet(); @@ -313,7 +265,7 @@ switch ($this->getTransactionType()) { case PhabricatorProjectNameTransaction::TRANSACTIONTYPE: case PhabricatorProjectSlugsTransaction::TRANSACTIONTYPE: - case self::TYPE_IMAGE: + case PhabricatorProjectImageTransaction::TRANSACTIONTYPE: case self::TYPE_ICON: case self::TYPE_COLOR: $tags[] = self::MAILTAG_METADATA; diff --git a/src/applications/project/xaction/PhabricatorProjectImageTransaction.php b/src/applications/project/xaction/PhabricatorProjectImageTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/project/xaction/PhabricatorProjectImageTransaction.php @@ -0,0 +1,136 @@ +getProfileImagePHID(); + } + + public function applyInternalEffects($object, $value) { + $object->setProfileImagePHID($value); + } + + public function applyExternalEffects($object, $value) { + $old = $this->getOldValue(); + $new = $value; + $all = array(); + if ($old) { + $all[] = $old; + } + if ($new) { + $all[] = $new; + } + + $files = id(new PhabricatorFileQuery()) + ->setViewer($this->getActor()) + ->withPHIDs($all) + ->execute(); + $files = mpull($files, null, 'getPHID'); + + $old_file = idx($files, $old); + if ($old_file) { + $old_file->detachFromObject($object->getPHID()); + } + + $new_file = idx($files, $new); + if ($new_file) { + $new_file->attachToObject($object->getPHID()); + } + } + + public function getTitle() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + // TODO: Some day, it would be nice to show the images. + if (!$old) { + return pht( + "%s set this project's image to %s.", + $this->renderAuthor(), + $this->renderNewHandle()); + } else if (!$new) { + return pht( + "%s removed this project's image.", + $this->renderAuthor()); + } else { + return pht( + "%s updated this project's image from %s to %s.", + $this->renderAuthor(), + $this->renderOldHandle(), + $this->renderNewHandle()); + } + } + + public function getTitleForFeed() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + // TODO: Some day, it would be nice to show the images. + if (!$old) { + return pht( + '%s set the image for %s to %s.', + $this->renderAuthor(), + $this->renderObject(), + $this->renderNewHandle()); + } else if (!$new) { + return pht( + '%s removed the image for %s.', + $this->renderAuthor(), + $this->renderObject()); + } else { + return pht( + '%s updated the image for %s from %s to %s.', + $this->renderAuthor(), + $this->renderObject(), + $this->renderOldHandle(), + $this->renderNewHandle()); + } + } + + public function getIcon() { + return 'fa-photo'; + } + + public function extractFilePHIDs($object, $value) { + if ($value) { + return array($value); + } + return array(); + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + $viewer = $this->getActor(); + + foreach ($xactions as $xaction) { + $file_phid = $xaction->getNewValue(); + + // Only validate if file was uploaded + if ($file_phid) { + $file = id(new PhabricatorFileQuery()) + ->setViewer($viewer) + ->withPHIDs(array($file_phid)) + ->executeOne(); + + if (!$file) { + $errors[] = $this->newInvalidError( + pht('"%s" is not a valid file PHID.', + $file_phid)); + } else { + if (!$file->isViewableImage()) { + $mime_type = $file->getMimeType(); + $errors[] = $this->newInvalidError( + pht('File mime type of "%s" is not a valid viewable image.', + $mime_type)); + } + } + } + } + + return $errors; + } + +} diff --git a/src/applications/project/xaction/PhabricatorProjectSlugsTransaction.php b/src/applications/project/xaction/PhabricatorProjectSlugsTransaction.php --- a/src/applications/project/xaction/PhabricatorProjectSlugsTransaction.php +++ b/src/applications/project/xaction/PhabricatorProjectSlugsTransaction.php @@ -61,7 +61,6 @@ count($rem), $this->renderSlugList($rem)); } - break; } public function getTitleForFeed() {