diff --git a/src/applications/macro/editor/PhabricatorMacroEditEngine.php b/src/applications/macro/editor/PhabricatorMacroEditEngine.php --- a/src/applications/macro/editor/PhabricatorMacroEditEngine.php +++ b/src/applications/macro/editor/PhabricatorMacroEditEngine.php @@ -21,6 +21,10 @@ return 'PhabricatorMacroApplication'; } + public function isEngineConfigurable() { + return false; + } + protected function newEditableObject() { $viewer = $this->getViewer(); return PhabricatorFileImageMacro::initializeNewFileImageMacro($viewer); @@ -35,7 +39,7 @@ } protected function getObjectEditTitleText($object) { - return pht('Edit %s', $object->getName()); + return pht('Edit Macro %s', $object->getName()); } protected function getObjectEditShortText($object) { @@ -63,6 +67,19 @@ PhabricatorMacroManageCapability::CAPABILITY); } + protected function willConfigureFields($object, array $fields) { + if ($this->getIsCreate()) { + $subscribers_field = idx($fields, + PhabricatorSubscriptionsEditEngineExtension::FIELDKEY); + if ($subscribers_field) { + // By default, hide the subscribers field when creating a macro + // because it makes the workflow SO HARD and wastes SO MUCH TIME. + $subscribers_field->setIsHidden(true); + } + } + return $fields; + } + protected function buildCustomEditFields($object) { return array( @@ -73,6 +90,7 @@ ->setConduitDescription(pht('Rename the macro.')) ->setConduitTypeDescription(pht('New macro name.')) ->setTransactionType(PhabricatorMacroNameTransaction::TRANSACTIONTYPE) + ->setIsRequired(true) ->setValue($object->getName()), id(new PhabricatorFileEditField()) ->setKey('filePHID') @@ -80,7 +98,8 @@ ->setDescription(pht('Image file to import.')) ->setTransactionType(PhabricatorMacroFileTransaction::TRANSACTIONTYPE) ->setConduitDescription(pht('File PHID to import.')) - ->setConduitTypeDescription(pht('File PHID.')), + ->setConduitTypeDescription(pht('File PHID.')) + ->setValue($object->getFilePHID()), ); } diff --git a/src/applications/macro/xaction/PhabricatorMacroFileTransaction.php b/src/applications/macro/xaction/PhabricatorMacroFileTransaction.php --- a/src/applications/macro/xaction/PhabricatorMacroFileTransaction.php +++ b/src/applications/macro/xaction/PhabricatorMacroFileTransaction.php @@ -49,7 +49,7 @@ public function getTitleForFeed() { return pht( - '%s changed the image for macro %s.', + '%s changed the image for %s.', $this->renderAuthor(), $this->renderObject()); } @@ -58,29 +58,37 @@ $errors = array(); $viewer = $this->getActor(); + $old_phid = $object->getFilePHID(); + foreach ($xactions as $xaction) { $file_phid = $xaction->getNewValue(); - if ($this->isEmptyTextTransaction($file_phid, $xactions)) { - $errors[] = $this->newRequiredError( - pht('Image macros must have a file.')); + if (!$old_phid) { + if ($this->isEmptyTextTransaction($file_phid, $xactions)) { + $errors[] = $this->newRequiredError( + pht('Image macros must have a file.')); + return $errors; + } } - $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->isViewableInBrowser()) { - $mime_type = $file->getMimeType(); + // 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('File mime type of "%s" is not a valid viewable image.', - $mime_type)); + 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)); + } } } diff --git a/src/applications/macro/xaction/PhabricatorMacroNameTransaction.php b/src/applications/macro/xaction/PhabricatorMacroNameTransaction.php --- a/src/applications/macro/xaction/PhabricatorMacroNameTransaction.php +++ b/src/applications/macro/xaction/PhabricatorMacroNameTransaction.php @@ -23,7 +23,7 @@ public function getTitleForFeed() { return pht( - '%s renamed %s macro %s to %s.', + '%s renamed %s from %s to %s.', $this->renderAuthor(), $this->renderObject(), $this->renderOldValue(), @@ -37,6 +37,7 @@ if ($this->isEmptyTextTransaction($object->getName(), $xactions)) { $errors[] = $this->newRequiredError( pht('Macros must have a name.')); + return $errors; } $max_length = $object->getColumnMaximumByteLength('name'); diff --git a/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsEditEngineExtension.php b/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsEditEngineExtension.php --- a/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsEditEngineExtension.php +++ b/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsEditEngineExtension.php @@ -4,6 +4,7 @@ extends PhabricatorEditEngineExtension { const EXTENSIONKEY = 'subscriptions.subscribers'; + const FIELDKEY = 'subscriberPHIDs'; const EDITKEY_ADD = 'subscribers.add'; const EDITKEY_SET = 'subscribers.set'; @@ -42,7 +43,7 @@ } $subscribers_field = id(new PhabricatorSubscribersEditField()) - ->setKey('subscriberPHIDs') + ->setKey(self::FIELDKEY) ->setLabel(pht('Subscribers')) ->setEditTypeKey('subscribers') ->setAliases(array('subscriber', 'subscribers'))