From 6ccf35f9a256bcefcb8be032ec29b35a2b4dfbb1 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 10 Apr 2018 06:09:51 -0700 Subject: [PATCH] Edit Interfaces in Almanac with EditEngine Summary: Depends on D19323. Ref T13120. Ref T12414. Move editing to modern stuff and fix some implementation errors from D19323 (mostly copy/paste stuff). Test Plan: - Created and edited interfaces. - Tried to create/edit an interface with a bogus/empty address/port, got errors. - Tried to create an interface on a bogus device, got an error. - Tried to create an interface on a device I could not edit, got an error. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13120, T12414 Differential Revision: https://secure.phabricator.com/D19324 --- src/__phutil_library_map__.php | 10 +- .../AlmanacInterfaceEditController.php | 151 ++---------------- .../editor/AlmanacInterfaceEditEngine.php | 139 ++++++++++++++++ .../AlmanacInterfaceAddressTransaction.php | 2 +- .../AlmanacInterfaceDeviceTransaction.php | 18 ++- .../AlmanacInterfaceNetworkTransaction.php | 2 +- .../AlmanacInterfacePortTransaction.php | 4 +- 7 files changed, 175 insertions(+), 151 deletions(-) create mode 100644 src/applications/almanac/editor/AlmanacInterfaceEditEngine.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 08806ab1b9..b3def5acde 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -62,6 +62,7 @@ phutil_register_library_map(array( 'AlmanacInterfaceDeleteController' => 'applications/almanac/controller/AlmanacInterfaceDeleteController.php', 'AlmanacInterfaceDeviceTransaction' => 'applications/almanac/xaction/AlmanacInterfaceDeviceTransaction.php', 'AlmanacInterfaceEditController' => 'applications/almanac/controller/AlmanacInterfaceEditController.php', + 'AlmanacInterfaceEditEngine' => 'applications/almanac/editor/AlmanacInterfaceEditEngine.php', 'AlmanacInterfaceEditor' => 'applications/almanac/editor/AlmanacInterfaceEditor.php', 'AlmanacInterfaceNetworkTransaction' => 'applications/almanac/xaction/AlmanacInterfaceNetworkTransaction.php', 'AlmanacInterfacePHIDType' => 'applications/almanac/phid/AlmanacInterfacePHIDType.php', @@ -5252,15 +5253,16 @@ phutil_register_library_map(array( 'PhabricatorExtendedPolicyInterface', 'PhabricatorApplicationTransactionInterface', ), - 'AlmanacInterfaceAddressTransaction' => 'AlmanacNetworkTransactionType', + 'AlmanacInterfaceAddressTransaction' => 'AlmanacInterfaceTransactionType', 'AlmanacInterfaceDatasource' => 'PhabricatorTypeaheadDatasource', 'AlmanacInterfaceDeleteController' => 'AlmanacDeviceController', - 'AlmanacInterfaceDeviceTransaction' => 'AlmanacNetworkTransactionType', + 'AlmanacInterfaceDeviceTransaction' => 'AlmanacInterfaceTransactionType', 'AlmanacInterfaceEditController' => 'AlmanacDeviceController', + 'AlmanacInterfaceEditEngine' => 'PhabricatorEditEngine', 'AlmanacInterfaceEditor' => 'PhabricatorApplicationTransactionEditor', - 'AlmanacInterfaceNetworkTransaction' => 'AlmanacNetworkTransactionType', + 'AlmanacInterfaceNetworkTransaction' => 'AlmanacInterfaceTransactionType', 'AlmanacInterfacePHIDType' => 'PhabricatorPHIDType', - 'AlmanacInterfacePortTransaction' => 'AlmanacNetworkTransactionType', + 'AlmanacInterfacePortTransaction' => 'AlmanacInterfaceTransactionType', 'AlmanacInterfaceQuery' => 'AlmanacQuery', 'AlmanacInterfaceTableView' => 'AphrontView', 'AlmanacInterfaceTransaction' => 'PhabricatorModularTransaction', diff --git a/src/applications/almanac/controller/AlmanacInterfaceEditController.php b/src/applications/almanac/controller/AlmanacInterfaceEditController.php index d223360003..017dad00ce 100644 --- a/src/applications/almanac/controller/AlmanacInterfaceEditController.php +++ b/src/applications/almanac/controller/AlmanacInterfaceEditController.php @@ -4,32 +4,16 @@ final class AlmanacInterfaceEditController extends AlmanacDeviceController { public function handleRequest(AphrontRequest $request) { - $viewer = $request->getViewer(); + $viewer = $this->getViewer(); + + $engine = id(new AlmanacInterfaceEditEngine()) + ->setController($this); $id = $request->getURIData('id'); - if ($id) { - $interface = id(new AlmanacInterfaceQuery()) - ->setViewer($viewer) - ->withIDs(array($id)) - ->requireCapabilities( - array( - PhabricatorPolicyCapability::CAN_VIEW, - PhabricatorPolicyCapability::CAN_EDIT, - )) - ->executeOne(); - if (!$interface) { - return new Aphront404Response(); - } - - $device = $interface->getDevice(); - - $is_new = false; - $title = pht('Edit Interface'); - $save_button = pht('Save Changes'); - } else { + if (!$id) { $device = id(new AlmanacDeviceQuery()) ->setViewer($viewer) - ->withIDs(array($request->getStr('deviceID'))) + ->withIDs(array($request->getInt('deviceID'))) ->requireCapabilities( array( PhabricatorPolicyCapability::CAN_VIEW, @@ -40,127 +24,12 @@ final class AlmanacInterfaceEditController return new Aphront404Response(); } - $interface = AlmanacInterface::initializeNewInterface(); - $is_new = true; - - $title = pht('Create Interface'); - $save_button = pht('Create Interface'); + $engine + ->addContextParameter('deviceID', $device->getID()) + ->setDevice($device); } - $device_uri = $device->getURI(); - $cancel_uri = $device_uri; - - $v_network = $interface->getNetworkPHID(); - - $v_address = $interface->getAddress(); - $e_address = true; - - $v_port = $interface->getPort(); - - $validation_exception = null; - - if ($request->isFormPost()) { - $v_network = $request->getStr('networkPHID'); - $v_address = $request->getStr('address'); - $v_port = $request->getStr('port'); - - $type_interface = AlmanacDeviceTransaction::TYPE_INTERFACE; - - $address = AlmanacAddress::newFromParts($v_network, $v_address, $v_port); - - $xaction = id(new AlmanacDeviceTransaction()) - ->setTransactionType($type_interface) - ->setNewValue($address->toDictionary()); - - if ($interface->getID()) { - $xaction->setOldValue(array( - 'id' => $interface->getID(), - ) + $interface->toAddress()->toDictionary()); - } else { - $xaction->setOldValue(array()); - } - - $xactions = array(); - $xactions[] = $xaction; - - $editor = id(new AlmanacDeviceEditor()) - ->setActor($viewer) - ->setContentSourceFromRequest($request) - ->setContinueOnNoEffect(true) - ->setContinueOnMissingFields(true); - - try { - $editor->applyTransactions($device, $xactions); - - $device_uri = $device->getURI(); - return id(new AphrontRedirectResponse())->setURI($device_uri); - } catch (PhabricatorApplicationTransactionValidationException $ex) { - $validation_exception = $ex; - $e_address = $ex->getShortMessage($type_interface); - } - } - - $networks = id(new AlmanacNetworkQuery()) - ->setViewer($viewer) - ->execute(); - - $form = id(new AphrontFormView()) - ->setUser($viewer) - ->appendChild( - id(new AphrontFormSelectControl()) - ->setLabel(pht('Network')) - ->setName('networkPHID') - ->setValue($v_network) - ->setOptions(mpull($networks, 'getName', 'getPHID'))) - ->appendChild( - id(new AphrontFormTextControl()) - ->setLabel(pht('Address')) - ->setName('address') - ->setValue($v_address) - ->setError($e_address)) - ->appendChild( - id(new AphrontFormTextControl()) - ->setLabel(pht('Port')) - ->setName('port') - ->setValue($v_port) - ->setError($e_address)) - ->appendChild( - id(new AphrontFormSubmitControl()) - ->addCancelButton($cancel_uri) - ->setValue($save_button)); - - $box = id(new PHUIObjectBoxView()) - ->setValidationException($validation_exception) - ->setHeaderText(pht('Interface')) - ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) - ->setForm($form); - - $crumbs = $this->buildApplicationCrumbs(); - $crumbs->addTextCrumb($device->getName(), $device_uri); - if ($is_new) { - $crumbs->addTextCrumb(pht('Create Interface')); - $header = id(new PHUIHeaderView()) - ->setHeader(pht('Create Interface')) - ->setHeaderIcon('fa-plus-square'); - } else { - $crumbs->addTextCrumb(pht('Edit Interface')); - $header = id(new PHUIHeaderView()) - ->setHeader(pht('Edit Interface')) - ->setHeaderIcon('fa-pencil'); - } - $crumbs->setBorder(true); - - $view = id(new PHUITwoColumnView()) - ->setHeader($header) - ->setFooter(array( - $box, - )); - - return $this->newPage() - ->setTitle($title) - ->setCrumbs($crumbs) - ->appendChild($view); - + return $engine->buildResponse(); } } diff --git a/src/applications/almanac/editor/AlmanacInterfaceEditEngine.php b/src/applications/almanac/editor/AlmanacInterfaceEditEngine.php new file mode 100644 index 0000000000..c7e7992743 --- /dev/null +++ b/src/applications/almanac/editor/AlmanacInterfaceEditEngine.php @@ -0,0 +1,139 @@ +device = $device; + return $this; + } + + public function getDevice() { + if (!$this->device) { + throw new PhutilInvalidStateException('setDevice'); + } + return $this->device; + } + + public function isEngineConfigurable() { + return false; + } + + public function getEngineName() { + return pht('Almanac Interfaces'); + } + + public function getSummaryHeader() { + return pht('Edit Almanac Interface Configurations'); + } + + public function getSummaryText() { + return pht('This engine is used to edit Almanac interfaces.'); + } + + public function getEngineApplicationClass() { + return 'PhabricatorAlmanacApplication'; + } + + protected function newEditableObject() { + $interface = AlmanacInterface::initializeNewInterface(); + + $device = $this->getDevice(); + $interface + ->setDevicePHID($device->getPHID()) + ->attachDevice($device); + + return $interface; + } + + protected function newObjectQuery() { + return new AlmanacInterfaceQuery(); + } + + protected function getObjectCreateTitleText($object) { + return pht('Create Interface'); + } + + protected function getObjectCreateButtonText($object) { + return pht('Create Interface'); + } + + protected function getObjectEditTitleText($object) { + return pht('Edit Interface'); + } + + protected function getObjectEditShortText($object) { + return pht('Edit Interface'); + } + + protected function getObjectCreateShortText() { + return pht('Create Interface'); + } + + protected function getObjectName() { + return pht('Interface'); + } + + protected function getEditorURI() { + return '/almanac/interface/edit/'; + } + + protected function getObjectCreateCancelURI($object) { + return '/almanac/interface/'; + } + + protected function getObjectViewURI($object) { + return $object->getDevice()->getURI(); + } + + protected function buildCustomEditFields($object) { + $viewer = $this->getViewer(); + + // TODO: Some day, this should be a datasource. + $networks = id(new AlmanacNetworkQuery()) + ->setViewer($viewer) + ->execute(); + $network_map = mpull($networks, 'getName', 'getPHID'); + + return array( + id(new PhabricatorTextEditField()) + ->setKey('device') + ->setLabel(pht('Device')) + ->setIsConduitOnly(true) + ->setTransactionType( + AlmanacInterfaceDeviceTransaction::TRANSACTIONTYPE) + ->setDescription(pht('When creating an interface, set the device.')) + ->setConduitDescription(pht('Set the device.')) + ->setConduitTypeDescription(pht('Device PHID.')) + ->setValue($object->getDevicePHID()), + id(new PhabricatorSelectEditField()) + ->setKey('network') + ->setLabel(pht('Network')) + ->setDescription(pht('Network for the interface.')) + ->setTransactionType( + AlmanacInterfaceNetworkTransaction::TRANSACTIONTYPE) + ->setValue($object->getNetworkPHID()) + ->setOptions($network_map), + id(new PhabricatorTextEditField()) + ->setKey('address') + ->setLabel(pht('Address')) + ->setDescription(pht('Address of the service.')) + ->setTransactionType( + AlmanacInterfaceAddressTransaction::TRANSACTIONTYPE) + ->setIsRequired(true) + ->setValue($object->getAddress()), + id(new PhabricatorTextEditField()) + ->setKey('port') + ->setLabel(pht('Port')) + ->setDescription(pht('Port of the service.')) + ->setTransactionType(AlmanacInterfacePortTransaction::TRANSACTIONTYPE) + ->setIsRequired(true) + ->setValue($object->getPort()), + ); + } + +} diff --git a/src/applications/almanac/xaction/AlmanacInterfaceAddressTransaction.php b/src/applications/almanac/xaction/AlmanacInterfaceAddressTransaction.php index 47eab92c16..7eb8e0a567 100644 --- a/src/applications/almanac/xaction/AlmanacInterfaceAddressTransaction.php +++ b/src/applications/almanac/xaction/AlmanacInterfaceAddressTransaction.php @@ -1,7 +1,7 @@ isEmptyTextTransaction($object->getAddress(), $xactions)) { + $device_phid = $object->getDevicePHID(); + if ($this->isEmptyTextTransaction($device_phid, $xactions)) { $errors[] = $this->newRequiredError( pht('Interfaces must have a device.')); } @@ -52,6 +53,19 @@ final class AlmanacInterfaceDeviceTransaction $xaction); continue; } + + $device = head($devices); + $can_edit = PhabricatorPolicyFilter::hasCapability( + $this->getActor(), + $device, + PhabricatorPolicyCapability::CAN_EDIT); + if (!$can_edit) { + $errors[] = $this->newInvalidError( + pht( + 'You can not attach an interface to a device which you do not '. + 'have permission to edit.')); + continue; + } } return $errors; diff --git a/src/applications/almanac/xaction/AlmanacInterfaceNetworkTransaction.php b/src/applications/almanac/xaction/AlmanacInterfaceNetworkTransaction.php index 50353d904d..def85c08e2 100644 --- a/src/applications/almanac/xaction/AlmanacInterfaceNetworkTransaction.php +++ b/src/applications/almanac/xaction/AlmanacInterfaceNetworkTransaction.php @@ -1,7 +1,7 @@ isEmptyTextTransaction($object->getName(), $xactions)) { + if ($this->isEmptyTextTransaction($object->getPort(), $xactions)) { $errors[] = $this->newRequiredError( pht('Interfaces must have a port number.')); }