diff --git a/location/js/fog.location.group.edit.js b/location/js/fog.location.group.edit.js deleted file mode 100644 index 24efaf02..00000000 --- a/location/js/fog.location.group.edit.js +++ /dev/null @@ -1,11 +0,0 @@ -(function($) { - // The whole tab -- Update wiring plus the create-and-associate button and - // modal -- is one shared helper (fog.common.js). These nine plugin tabs are - // the same card with a different noun, and each used to carry its own copy - // of the submit wiring. - $.registerSelectTab({ - slug: 'group-location', - send: 'location-send', - node: 'location' - }); -})(jQuery); diff --git a/location/src/Hooks/AddLocationGroup.php b/location/src/Hooks/AddLocationGroup.php deleted file mode 100644 index 62c9e822..00000000 --- a/location/src/Hooks/AddLocationGroup.php +++ /dev/null @@ -1,282 +0,0 @@ - - * @author Lee Rowlett - * @license http://opensource.org/licenses/gpl-3.0 GPLv3 - * @link https://fogproject.org - */ - -namespace FOG\Plugins\Location\Hooks; - -/** - * Adds the location choice to groups. - * - * @category AddLocationGroup - * @package FOGProject - * @author Tom Elliott - * @author Lee Rowlett - * @license http://opensource.org/licenses/gpl-3.0 GPLv3 - * @link https://fogproject.org - */ -class AddLocationGroup extends \FOG\Base\Hook -{ - /** - * The name of this hook. - * - * @var string - */ - public $name = 'AddLocationGroup'; - /** - * The description of this hook. - * - * @var string - */ - public $description = 'Add Location to Groups'; - /** - * The active flag (always true but for posterity) - * - * @var bool - */ - public $active = true; - /** - * The node this hook enacts with. - * - * @var string - */ - public $node = 'location'; - /** - * Initialize object. - * - * @return void - */ - public function __construct() - { - parent::__construct(); - $this->registerInstalled([ - ['PLUGINS_INJECT_TABDATA', 'groupTabData'], - ['GROUP_EDIT_SUCCESS', 'groupAddLocationEdit'], - ['GROUP_ADD_FIELDS', 'groupAddLocationField'], - ]); - } - /** - * The group tab data. - * - * @param mixed $arguments The arguments to change. - * - * @return void - */ - public function groupTabData($arguments) - { - global $node; - if ($node != 'group') { - return; - } - $obj = $arguments['obj']; - - $arguments['pluginsTabData'][] = [ - 'name' => _('Location Association'), - 'id' => 'group-location', - 'generator' => function () use ($obj) { - $this->groupLocation($obj); - } - ]; - } - /** - * The group location display - * - * @param object $obj The group object we're working with. - * - * @return void - */ - public function groupLocation($obj) - { - $locationID = (int)filter_input(INPUT_POST, 'location'); - // Group Locations - $locationSelector = self::getClass('LocationManager') - ->buildSelectBox($locationID, 'location'); - - $fields = [ - \FOG\Base\FOGPage::makeLabel( - 'col-sm-3 col-form-label', - 'location', - _('Group Location') - ) => $locationSelector - ]; - - $buttons = \FOG\Base\FOGPage::makeButton( - 'location-send', - _('Update'), - 'btn btn-primary float-end' - ); - // Create-and-associate, the same button and modal the core association - // tabs get. Added before the *_FIELDS event so a listener can still see - // it, and immediately after Update so Update stays the row's rightmost - // (primary) button with this one to its left. The modal it returns is - // echoed after the form -- see below for why it cannot go inside. - $createModal = \FOG\Base\FOGPage::renderAssocCreate( - 'group-location', - 'location', - $buttons, - $obj->get('id') - ); - - self::$HookManager->processEvent( - 'GROUP_LOCATION_FIELDS', - [ - 'fields' => &$fields, - 'buttons' => &$buttons, - 'Group' => &$obj - ] - ); - $rendered = \FOG\Base\FOGPage::formFields($fields); - unset($fields); - - echo \FOG\Base\FOGPage::makeFormTag( - '', - 'group-location-form', - \FOG\Base\FOGPage::makeTabUpdateURL( - 'group-location', - $obj->get('id') - ), - 'post', - 'application/x-www-form-urlencoded', - true - ); - echo '
'; - echo '
'; - echo '

'; - echo _('Location'); - echo '

'; - echo '
'; - echo '
'; - echo $rendered; - echo '
'; - echo '
'; - echo $buttons; - echo '
'; - echo '
'; - echo ''; - // Outside the form, deliberately. The modal holds the fetched create - // form, and a
inside another is invalid markup: the - // browser drops the inner one and the create would post nothing. - echo $createModal; - } - /** - * The location updater element. - * - * @param object $obj The object we're working with. - * - * @return void - */ - public function groupLocationPost($obj) - { - self::checkAuthAndCSRF(); - $locationID = trim( - (int)filter_input(INPUT_POST, 'location') - ); - $insert_fields = ['hostID', 'locationID']; - $insert_values = []; - $hosts = \FOG\Router\Route::getIds( - 'groupassociation', - ['groupID' => $obj->get('id')], - 'hostID' - ); - if (count($hosts ?: [])) { - \FOG\Router\Route::deletemass( - 'locationassociation', - ['hostID' => $hosts] - ); - if (self::getClass('Location', $locationID)->isValid()) { - foreach ((array)$hosts as $ind => &$hostID) { - $insert_values[] = [$hostID, $locationID]; - unset($hostID); - } - } - } - if (count($insert_values) > 0) { - self::getClass('LocationAssociationManager') - ->insertBatch( - $insert_fields, - $insert_values - ); - } - } - /** - * The group location selector. - * - * @param mixed $arguments The arguments to change. - * - * @return void - */ - public function groupAddLocationEdit($arguments) - { - self::checkAuthAndCSRF(); - global $tab; - global $node; - if ($node != 'group') { - return; - } - $obj = $arguments['Group']; - try { - switch ($tab) { - case 'group-location': - $this->groupLocationPost($obj); - break; - default: - return; - } - $arguments['code'] = \FOG\Router\HTTPResponseCodes::HTTP_ACCEPTED; - $arguments['hook'] = 'GROUP_EDIT_LOCATION_SUCCESS'; - $arguments['msg'] = json_encode( - [ - 'msg' => _('Group Location Updated!'), - 'title' => _('Group Location Update Success') - ] - ); - } catch (\Exception $e) { - $arguments['code'] = ( - $arguments['serverFault'] ? - \FOG\Router\HTTPResponseCodes::HTTP_INTERNAL_SERVER_ERROR : - \FOG\Router\HTTPResponseCodes::HTTP_BAD_REQUEST - ); - $arguments['hook'] = 'GROUP_EDIT_LOCATION_FAIL'; - $arguments['msg'] = json_encode( - [ - 'error' => $e->getMessage(), - 'title' => _('Group Update Location Fail') - ] - ); - } - } - /** - * The group location field for function add. - * - * @param mixed $arguments The arguments to change. - * - * @return void - */ - public function groupAddLocationField($arguments) - { - global $node; - if ($node != 'group') { - return; - } - $locationID = (int)filter_input(INPUT_POST, 'location'); - $locationSelector = self::getClass('LocationManager') - ->buildSelectBox($locationID, 'location'); - - $arguments['fields'][ - \FOG\Base\FOGPage::makeLabel( - 'col-sm-3 col-form-label', - 'location', - _('Group Location') - ) - ] = $locationSelector; - } -} diff --git a/location/src/Hooks/AddLocationHost.php b/location/src/Hooks/AddLocationHost.php index bda2fa8a..d29a2632 100644 --- a/location/src/Hooks/AddLocationHost.php +++ b/location/src/Hooks/AddLocationHost.php @@ -61,6 +61,8 @@ public function __construct() $this->registerInstalled([ ['PLUGINS_INJECT_TABDATA', 'hostTabData'], ['HOST_EDIT_SUCCESS', 'hostAddLocationEdit'], + ['HOST_MASSEDIT_FIELDS', 'massEditFields'], + ['HOST_MASSEDIT_APPLY', 'massEditApply'], ['HOST_ADD_FIELDS', 'hostAddLocationField'], ['HOST_REGISTER', 'hostAddLocationRegister'], ]); @@ -314,4 +316,151 @@ public function hostAddLocationField($arguments) ) ] = $locationSelector; } + /** + * Contributes the location to the host list's Mass Edit form. + * + * ADR 0038 decision 13. This replaces AddLocationGroup, whose only job was + * to set one location across a group's members. That hook was a copy, not a + * grant: it read the membership at the instant the button was pressed and + * wrote a row per member, so a host added to the group afterward got + * nothing and a host removed kept what it had. Worse, it had no way to + * express "leave this host alone" -- every save deleted the association + * for every member first, so setting one host's location through the group + * wiped it from all of them. + * + * A location is single-valued per host: the write path has always been + * delete-then-insert-one. That is why this is a mass edit rather than one + * of ADR 0038's grant tables -- a grant is a SET that several groups can + * union into, and unioning two locations means nothing. + * + * Core draws the three-state action control around this field, which is + * what makes "leave alone" expressible at all. The value control is + * rendered EMPTY on purpose: there is no honest value to pre-fill it with + * when the selection disagrees. What the hosts hold is stated in the hint + * instead, where "(varies)" is sayable. + * + * @param mixed $arguments The arguments to change. + * + * @return void + */ + public function massEditFields($arguments) + { + $hostIDs = (array)$arguments['hostIDs']; + + $arguments['fields']['location'] = [ + 'label' => _('Host Location'), + 'input' => self::getClass('LocationManager')->buildSelectBox( + '', + 'value[location]', + 'name', + '', + false, + 'id', + 'massedit-location' + ), + 'hint' => \FOG\Util\SharedHostValues::hint( + $this->_sharedLocation($hostIDs) + ), + ]; + } + /** + * What the selected hosts hold, as a name rather than an id. + * + * SharedHostValues::forHostRows() answers in the column's own terms, so a + * uniform selection comes back as the location id. Rendering that would put + * a bare number in front of the admin where every other hint shows the + * value they set. The lookup only happens when the answer is uniform and + * non-empty, which is the only case where there is one name to show. + * + * forHostRows() is also the reason this is not a hand-rolled query: a + * host with no location has no row at all, so counting rows would call + * three hosts out of five "in agreement". It compares the row count to + * the selection size for exactly that reason. + * + * @param array $hostIDs the selection + * + * @return array a SharedHostValues info array + */ + private function _sharedLocation(array $hostIDs) + { + $info = \FOG\Util\SharedHostValues::forHostRows( + $hostIDs, + 'locationAssoc', + 'laHostID', + ['location' => 'laLocationID'] + )['location']; + + if (!empty($info['uniform']) && '' !== (string)$info['value']) { + $item = self::getClass('Location', (int)$info['value']); + if ($item->isValid()) { + $info['value'] = $item->get('name'); + } + } + + return $info; + } + /** + * Applies a resolved location action across the selection. + * + * The action arrives already reduced to leave/set/clear, so there is no + * sentinel to parse and no way for an empty control to be mistaken for + * "clear it" -- which is the failure the three-state model exists to stop + * and the one AddLocationGroup could not avoid. + * + * @param mixed $arguments The arguments to change. + * + * @return void + */ + public function massEditApply($arguments) + { + if (!isset($arguments['actions']['location'])) { + return; + } + $instruction = $arguments['actions']['location']; + $action = $instruction['action']; + if ('leave' === $action) { + return; + } + $hostIDs = array_values( + array_filter( + array_map('intval', (array)$arguments['hostIDs']), + function ($id) { + return $id > 0; + } + ) + ); + if (count($hostIDs) < 1) { + return; + } + + $itemID = 0; + if ('set' === $action) { + $itemID = (int)$instruction['value']; + // A set naming a location that does not exist is a bad request, not + // an instruction to clear: silently turning it into one would + // strip the location off every selected host. + if ($itemID < 1 + || !self::getClass('Location', $itemID)->isValid() + ) { + throw new \Exception(_('Invalid Location selected')); + } + } + + // Both branches clear first. The association is single-valued per + // host, so a set is a replace -- and doing it in one deletemass + // rather than per host keeps it to one statement. + \FOG\Router\Route::deletemass( + 'locationassociation', + ['hostID' => $hostIDs] + ); + if ($itemID < 1) { + return; + } + $insert_values = []; + foreach ($hostIDs as $hostID) { + $insert_values[] = [$hostID, $itemID]; + } + self::getClass('LocationAssociationManager') + ->insertBatch(['hostID', 'locationID'], $insert_values); + } } diff --git a/location/src/Hooks/AddLocationJS.php b/location/src/Hooks/AddLocationJS.php index 5be38002..b5022ca3 100644 --- a/location/src/Hooks/AddLocationJS.php +++ b/location/src/Hooks/AddLocationJS.php @@ -75,7 +75,6 @@ public function injectJSFiles($arguments) 'location' => [], 'report' => ['secondary' => true, 'fallback' => true], 'host' => ['secondary' => true, 'fallback' => true], - 'group' => ['secondary' => true, 'fallback' => true], ]); } } diff --git a/ou/js/fog.ou.group.edit.js b/ou/js/fog.ou.group.edit.js deleted file mode 100644 index a03190b0..00000000 --- a/ou/js/fog.ou.group.edit.js +++ /dev/null @@ -1,11 +0,0 @@ -(function($) { - // The whole tab -- Update wiring plus the create-and-associate button and - // modal -- is one shared helper (fog.common.js). These nine plugin tabs are - // the same card with a different noun, and each used to carry its own copy - // of the submit wiring. - $.registerSelectTab({ - slug: 'group-ou', - send: 'ou-send', - node: 'ou' - }); -})(jQuery); diff --git a/ou/src/Hooks/AddOUGroup.php b/ou/src/Hooks/AddOUGroup.php deleted file mode 100644 index 2cf97dfb..00000000 --- a/ou/src/Hooks/AddOUGroup.php +++ /dev/null @@ -1,277 +0,0 @@ - - * @license http://opensource.org/licenses/gpl-3.0 GPLv3 - * @link https://fogproject.org - */ - -namespace FOG\Plugins\OU\Hooks; - -/** - * Adds the OU choice to groups. - * - * @category AddOUGroup - * @package FOGProject - * @author Tom Elliott - * @license http://opensource.org/licenses/gpl-3.0 GPLv3 - * @link https://fogproject.org - */ -class AddOUGroup extends \FOG\Base\Hook -{ - /** - * The name of this hook. - * - * @var string - */ - public $name = 'AddOUGroup'; - /** - * The description of this hook. - * - * @var string - */ - public $description = 'Add OU to Groups'; - /** - * The active flag (always true but for posterity) - * - * @var bool - */ - public $active = true; - /** - * The node this hook enacts with. - * - * @var string - */ - public $node = 'ou'; - /** - * Initialize object. - * - * @return void - */ - public function __construct() - { - parent::__construct(); - $this->registerInstalled([ - ['PLUGINS_INJECT_TABDATA', 'groupTabData'], - ['GROUP_EDIT_SUCCESS', 'groupAddOUEdit'], - ['GROUP_ADD_FIELDS', 'groupAddOUField'], - ]); - } - /** - * The group tab data. - * - * @param mixed $arguments The arguments to change. - * - * @return void - */ - public function groupTabData($arguments) - { - global $node; - if ($node != 'group') { - return; - } - $obj = $arguments['obj']; - - $arguments['pluginsTabData'][] = [ - 'name' => _('OU Association'), - 'id' => 'group-ou', - 'generator' => function () use ($obj) { - $this->groupOU($obj); - } - ]; - } - /** - * The group ou display - * - * @param object $obj The group object we're working with. - * - * @return void - */ - public function groupOU($obj) - { - $ouID = (int)filter_input(INPUT_POST, 'ou'); - // Group OUs - $ouSelector = self::getClass('OUManager') - ->buildSelectBox($ouID, 'ou'); - - $fields = [ - \FOG\Base\FOGPage::makeLabel( - 'col-sm-3 col-form-label', - 'ou', - _('Group OU') - ) => $ouSelector - ]; - - $buttons = \FOG\Base\FOGPage::makeButton( - 'ou-send', - _('Update'), - 'btn btn-primary float-end' - ); - // Create-and-associate, the same button and modal the core association - // tabs get. Added before the *_FIELDS event so a listener can still see - // it, and immediately after Update so Update stays the row's rightmost - // (primary) button with this one to its left. The modal it returns is - // echoed after the form -- see below for why it cannot go inside. - $createModal = \FOG\Base\FOGPage::renderAssocCreate( - 'group-ou', - 'ou', - $buttons, - $obj->get('id'), - _('OU') - ); - - self::$HookManager->processEvent( - 'GROUP_OU_FIELDS', - [ - 'fields' => &$fields, - 'buttons' => &$buttons, - 'Group' => &$obj - ] - ); - $rendered = \FOG\Base\FOGPage::formFields($fields); - unset($fields); - - echo \FOG\Base\FOGPage::makeFormTag( - '', - 'group-ou-form', - \FOG\Base\FOGPage::makeTabUpdateURL( - 'group-ou', - $obj->get('id') - ), - 'post', - 'application/x-www-form-urlencoded', - true - ); - echo '
'; - echo '
'; - echo '

'; - echo _('OU'); - echo '

'; - echo '
'; - echo '
'; - echo $rendered; - echo '
'; - echo '
'; - echo $buttons; - echo '
'; - echo '
'; - echo ''; - // Outside the form, deliberately. The modal holds the fetched create - // form, and a
inside another is invalid markup: the - // browser drops the inner one and the create would post nothing. - echo $createModal; - } - /** - * The OU updater element. - * - * @param object $obj The object we're working with. - * - * @return void - */ - public function groupOUPost($obj) - { - self::checkAuthAndCSRF(); - $ouID = trim( - (int)filter_input(INPUT_POST, 'ou') - ); - $insert_fields = ['hostID', 'ouID']; - $insert_values = []; - $hosts = $obj->get('hosts'); - if (count($hosts ?: [])) { - \FOG\Router\Route::deletemass( - 'ouassociation', - ['hostID' => $hosts] - ); - if ($ouID > 0) { - foreach ((array)$hosts as $ind => &$hostID) { - $insert_values[] = [$hostID, $ouID]; - unset($hostID); - } - } - } - if (count($insert_values) > 0) { - self::getClass('OUAssociationManager') - ->insertBatch( - $insert_fields, - $insert_values - ); - } - } - /** - * The group ou selector. - * - * @param mixed $arguments The arguments to change. - * - * @return void - */ - public function groupAddOUEdit($arguments) - { - self::checkAuthAndCSRF(); - global $tab; - global $node; - if ($node != 'group') { - return; - } - $obj = $arguments['Group']; - try { - switch ($tab) { - case 'group-ou': - $this->groupOUPost($obj); - break; - default: - return; - } - $arguments['code'] = \FOG\Router\HTTPResponseCodes::HTTP_ACCEPTED; - $arguments['hook'] = 'GROUP_EDIT_OU_SUCCESS'; - $arguments['msg'] = json_encode( - [ - 'msg' => _('Group OU Updated!'), - 'title' => _('Group OU Update Success') - ] - ); - } catch (\Exception $e) { - $arguments['code'] = ( - $arguments['serverFault'] ? - \FOG\Router\HTTPResponseCodes::HTTP_INTERNAL_SERVER_ERROR : - \FOG\Router\HTTPResponseCodes::HTTP_BAD_REQUEST - ); - $arguments['hook'] = 'GROUP_EDIT_OU_FAIL'; - $arguments['msg'] = json_encode( - [ - 'error' => $e->getMessage(), - 'title' => _('Group Update OU Fail') - ] - ); - } - } - /** - * The group OU field for function add. - * - * @param mixed $arguments The arguments to change. - * - * @return void - */ - public function groupAddOUField($arguments) - { - global $node; - if ($node != 'group') { - return; - } - $ouID = (int)filter_input(INPUT_POST, 'ou'); - $ouSelector = self::getClass('OUManager') - ->buildSelectBox($ouID, 'ou'); - - $arguments['fields'][ - \FOG\Base\FOGPage::makeLabel( - 'col-sm-3 col-form-label', - 'ou', - _('Group OU') - ) - ] = $ouSelector; - } -} diff --git a/ou/src/Hooks/AddOUHost.php b/ou/src/Hooks/AddOUHost.php index 5d8c9d80..f6e450d2 100644 --- a/ou/src/Hooks/AddOUHost.php +++ b/ou/src/Hooks/AddOUHost.php @@ -61,6 +61,8 @@ public function __construct() $this->registerInstalled([ ['PLUGINS_INJECT_TABDATA', 'hostTabData'], ['HOST_EDIT_SUCCESS', 'hostAddOUEdit'], + ['HOST_MASSEDIT_FIELDS', 'massEditFields'], + ['HOST_MASSEDIT_APPLY', 'massEditApply'], ['HOST_ADD_FIELDS', 'hostAddOUField'], ['HOST_REGISTER', 'hostAddOURegister'], ]); @@ -315,4 +317,151 @@ public function hostAddOUField($arguments) ) ] = $ouSelector; } + /** + * Contributes the ou to the host list's Mass Edit form. + * + * ADR 0038 decision 13. This replaces AddOUGroup, whose only job was + * to set one ou across a group's members. That hook was a copy, not a + * grant: it read the membership at the instant the button was pressed and + * wrote a row per member, so a host added to the group afterward got + * nothing and a host removed kept what it had. Worse, it had no way to + * express "leave this host alone" -- every save deleted the association + * for every member first, so setting one host's ou through the group + * wiped it from all of them. + * + * A ou is single-valued per host: the write path has always been + * delete-then-insert-one. That is why this is a mass edit rather than one + * of ADR 0038's grant tables -- a grant is a SET that several groups can + * union into, and unioning two ous means nothing. + * + * Core draws the three-state action control around this field, which is + * what makes "leave alone" expressible at all. The value control is + * rendered EMPTY on purpose: there is no honest value to pre-fill it with + * when the selection disagrees. What the hosts hold is stated in the hint + * instead, where "(varies)" is sayable. + * + * @param mixed $arguments The arguments to change. + * + * @return void + */ + public function massEditFields($arguments) + { + $hostIDs = (array)$arguments['hostIDs']; + + $arguments['fields']['ou'] = [ + 'label' => _('Host OU'), + 'input' => self::getClass('OUManager')->buildSelectBox( + '', + 'value[ou]', + 'name', + '', + false, + 'id', + 'massedit-ou' + ), + 'hint' => \FOG\Util\SharedHostValues::hint( + $this->_sharedOU($hostIDs) + ), + ]; + } + /** + * What the selected hosts hold, as a name rather than an id. + * + * SharedHostValues::forHostRows() answers in the column's own terms, so a + * uniform selection comes back as the ou id. Rendering that would put + * a bare number in front of the admin where every other hint shows the + * value they set. The lookup only happens when the answer is uniform and + * non-empty, which is the only case where there is one name to show. + * + * forHostRows() is also the reason this is not a hand-rolled query: a + * host with no ou has no row at all, so counting rows would call + * three hosts out of five "in agreement". It compares the row count to + * the selection size for exactly that reason. + * + * @param array $hostIDs the selection + * + * @return array a SharedHostValues info array + */ + private function _sharedOU(array $hostIDs) + { + $info = \FOG\Util\SharedHostValues::forHostRows( + $hostIDs, + 'ouAssoc', + 'oaHostID', + ['ou' => 'oaOUID'] + )['ou']; + + if (!empty($info['uniform']) && '' !== (string)$info['value']) { + $item = self::getClass('OU', (int)$info['value']); + if ($item->isValid()) { + $info['value'] = $item->get('name'); + } + } + + return $info; + } + /** + * Applies a resolved ou action across the selection. + * + * The action arrives already reduced to leave/set/clear, so there is no + * sentinel to parse and no way for an empty control to be mistaken for + * "clear it" -- which is the failure the three-state model exists to stop + * and the one AddOUGroup could not avoid. + * + * @param mixed $arguments The arguments to change. + * + * @return void + */ + public function massEditApply($arguments) + { + if (!isset($arguments['actions']['ou'])) { + return; + } + $instruction = $arguments['actions']['ou']; + $action = $instruction['action']; + if ('leave' === $action) { + return; + } + $hostIDs = array_values( + array_filter( + array_map('intval', (array)$arguments['hostIDs']), + function ($id) { + return $id > 0; + } + ) + ); + if (count($hostIDs) < 1) { + return; + } + + $itemID = 0; + if ('set' === $action) { + $itemID = (int)$instruction['value']; + // A set naming a ou that does not exist is a bad request, not + // an instruction to clear: silently turning it into one would + // strip the ou off every selected host. + if ($itemID < 1 + || !self::getClass('OU', $itemID)->isValid() + ) { + throw new \Exception(_('Invalid OU selected')); + } + } + + // Both branches clear first. The association is single-valued per + // host, so a set is a replace -- and doing it in one deletemass + // rather than per host keeps it to one statement. + \FOG\Router\Route::deletemass( + 'ouassociation', + ['hostID' => $hostIDs] + ); + if ($itemID < 1) { + return; + } + $insert_values = []; + foreach ($hostIDs as $hostID) { + $insert_values[] = [$hostID, $itemID]; + } + self::getClass('OUAssociationManager') + ->insertBatch(['hostID', 'ouID'], $insert_values); + } } diff --git a/ou/src/Hooks/AddOUJS.php b/ou/src/Hooks/AddOUJS.php index b86e7012..0a8a95a3 100644 --- a/ou/src/Hooks/AddOUJS.php +++ b/ou/src/Hooks/AddOUJS.php @@ -75,7 +75,6 @@ public function injectJSFiles($arguments) 'ou' => ['fallback' => true], 'report' => ['secondary' => true, 'fallback' => true], 'host' => ['secondary' => true], - 'group' => ['secondary' => true], ]); } } diff --git a/tests/group-push-became-mass-edit.test.php b/tests/group-push-became-mass-edit.test.php new file mode 100644 index 00000000..1c42f811 --- /dev/null +++ b/tests/group-push-became-mass-edit.test.php @@ -0,0 +1,346 @@ +]`, and no + * plugin draws its own action control -- a two-state field in a mass + * edit is the defect the design exists to prevent. + * + * Usage: php tests/group-push-became-mass-edit.test.php + * Exit status 0 = pass, 1 = fail. + */ + +$root = dirname(__DIR__); +$fails = []; + +if (!function_exists('_')) { + /** + * Stand-in for gettext. + * + * @param string $s the string + * + * @return string + */ + function _($s) + { + return $s; + } +} + +/** + * Records a failure. + * + * @param string $why what went wrong + * + * @return void + */ +function fail($why) +{ + global $fails; + $fails[] = $why; +} + +/** + * Asserts a condition. + * + * @param string $label what is being checked + * @param bool $cond the condition + * + * @return bool + */ +function check($label, $cond) +{ + if (!$cond) { + fail($label); + } + + return (bool)$cond; +} + +require_once __DIR__ . '/stubs/fog-stubs.php'; + +// ------------------------------------------------------- 1. the hooks are gone + +$gone = [ + 'location/src/Hooks/AddLocationGroup.php', + 'ou/src/Hooks/AddOUGroup.php', + 'location/js/fog.location.group.edit.js', + 'ou/js/fog.ou.group.edit.js', +]; +foreach ($gone as $rel) { + check( + "$rel is not shipped any more", + !file_exists($root . '/' . $rel) + ); +} + +// No hook anywhere may write an association over a group's membership again. +// Both old hooks did it the same way: resolve the members, then deletemass. +$hookFiles = []; +foreach (glob($root . '/*/src/Hooks/*.php') as $file) { + $hookFiles[$file] = (string)file_get_contents($file); +} +foreach ($hookFiles as $file => $src) { + $code = preg_replace('#/\*.*?\*/#s', '', $src); + $code = (string)preg_replace('#//[^\n]*#', '', (string)$code); + $readsMembers = ( + false !== strpos($code, "get('hosts')") + || false !== strpos($code, "'groupassociation'") + ); + if (!$readsMembers) { + continue; + } + check( + basename($file) . ' does not deletemass over a group membership', + false === strpos($code, 'deletemass') + ); +} + +// ------------------------------------------- 2. both events, on both plugins + +$cases = [ + [ + 'file' => 'location/src/Hooks/AddLocationHost.php', + 'class' => 'FOG\Plugins\Location\Hooks\AddLocationHost', + 'key' => 'location', + 'assoc' => 'locationassociation', + 'col' => 'locationID', + ], + [ + 'file' => 'ou/src/Hooks/AddOUHost.php', + 'class' => 'FOG\Plugins\OU\Hooks\AddOUHost', + 'key' => 'ou', + 'assoc' => 'ouassociation', + 'col' => 'ouID', + ], +]; + +foreach ($cases as $case) { + require_once $root . '/' . $case['file']; + $hook = new $case['class'](); + $key = $case['key']; + + $events = []; + foreach ($hook->registered as $pair) { + $events[] = $pair[0]; + } + check( + "$key registers HOST_MASSEDIT_FIELDS", + in_array('HOST_MASSEDIT_FIELDS', $events, true) + ); + check( + "$key registers HOST_MASSEDIT_APPLY", + in_array('HOST_MASSEDIT_APPLY', $events, true) + ); + + // ------------------------------------------------- 3. the field it offers + + \FOG\Util\SharedHostValues::$rows = [ + $key => ['uniform' => false, 'value' => ''], + ]; + $fields = []; + $hostIDs = [1, 2, 3]; + $args = ['fields' => &$fields, 'hostIDs' => &$hostIDs]; + $hook->massEditFields($args); + + if (check("$key contributes a field", isset($fields[$key]))) { + $field = $fields[$key]; + check( + "$key names its control value[$key]", + false !== strpos((string)$field['input'], 'name="value[' . $key . ']"') + ); + check( + "$key reports a mixed selection as (varies)", + '(varies)' === $field['hint'] + ); + // Core draws the action control. A plugin drawing its own could ship + // a two-state field, which is the defect this design prevents. + check( + "$key does not draw its own action control", + false === strpos((string)$field['input'], 'action[') + ); + // Rendered empty: there is no honest value when the selection differs. + check( + "$key renders its control with no value preselected", + false === strpos((string)$field['input'], 'selected') + ); + } + + // A uniform selection reports the NAME, not the raw id. + \FOG\Util\SharedHostValues::$rows = [ + $key => ['uniform' => true, 'value' => '7'], + ]; + $fields = []; + $args = ['fields' => &$fields, 'hostIDs' => &$hostIDs]; + $hook->massEditFields($args); + check( + "$key reports a uniform selection by name rather than by id", + isset($fields[$key]) && 'StubName' === $fields[$key]['hint'] + ); + + // ------------------------------------------------------- 4. applying it + + /** + * Runs one apply and returns what it wrote. + * + * @param object $hook the hook + * @param string $key the field key + * @param array $action the resolved instruction + * + * @return array [deletes, batches, threw] + */ + $apply = function ($hook, $key, $action) { + \FOG\Router\Route::$deleted = []; + $manager = new \FOG\Base\StubItem(); + \FOG\Base\Hook::$classes = [ + 'LocationAssociationManager' => $manager, + 'OUAssociationManager' => $manager, + ]; + if (isset($action['invalid'])) { + \FOG\Base\Hook::$classes['Location'] = function ($id) { + $item = new \FOG\Base\StubItem($id); + $item->valid = false; + + return $item; + }; + \FOG\Base\Hook::$classes['OU'] = + \FOG\Base\Hook::$classes['Location']; + } + $hostIDs = [1, 2, 3]; + $actions = [$key => $action]; + $args = ['hostIDs' => &$hostIDs, 'actions' => &$actions]; + $threw = false; + try { + $hook->massEditApply($args); + } catch (\Exception $e) { + $threw = true; + } + \FOG\Base\Hook::$classes = []; + + return [\FOG\Router\Route::$deleted, $manager->batches, $threw]; + }; + + // 'leave' writes NOTHING. The state the old hook could not express. + list($deleted, $batches, $threw) = $apply( + $hook, + $key, + ['action' => 'leave', 'value' => ''] + ); + check("$key leave deletes nothing", 0 === count($deleted)); + check("$key leave inserts nothing", 0 === count($batches)); + check("$key leave does not throw", !$threw); + + // 'set' replaces: one delete over the selection, one row per host. + list($deleted, $batches, $threw) = $apply( + $hook, + $key, + ['action' => 'set', 'value' => '5'] + ); + check("$key set does not throw", !$threw); + if (check("$key set deletes once", 1 === count($deleted))) { + check( + "$key set deletes from " . $case['assoc'], + $case['assoc'] === $deleted[0][0] + ); + check( + "$key set scopes the delete to the selection", + isset($deleted[0][1]['hostID']) + && [1, 2, 3] === $deleted[0][1]['hostID'] + ); + } + if (check("$key set inserts once", 1 === count($batches))) { + check( + "$key set names hostID and " . $case['col'], + ['hostID', $case['col']] === $batches[0][0] + ); + check( + "$key set writes one row per selected host", + 3 === count($batches[0][1]) + && [1, 5] === $batches[0][1][0] + ); + } + + // 'set' naming something that does not exist THROWS. Falling through to + // the delete would clear the value on every selected host. + list($deleted, $batches, $threw) = $apply( + $hook, + $key, + ['action' => 'set', 'value' => '99', 'invalid' => true] + ); + check("$key set of a missing record throws", $threw); + check( + "$key set of a missing record deletes nothing", + 0 === count($deleted) + ); + check( + "$key set of a missing record inserts nothing", + 0 === count($batches) + ); + + // 'clear' deletes and inserts nothing. + list($deleted, $batches, $threw) = $apply( + $hook, + $key, + ['action' => 'clear', 'value' => ''] + ); + check("$key clear does not throw", !$threw); + check("$key clear deletes once", 1 === count($deleted)); + check("$key clear inserts nothing", 0 === count($batches)); + + // An empty selection is not an instruction to delete everything. + \FOG\Router\Route::$deleted = []; + $none = []; + $actions = [$key => ['action' => 'set', 'value' => '5']]; + $args = ['hostIDs' => &$none, 'actions' => &$actions]; + $hook->massEditApply($args); + check( + "$key writes nothing when the selection is empty", + 0 === count(\FOG\Router\Route::$deleted) + ); +} + +if (count($fails)) { + echo "FAIL (" . count($fails) . "):\n"; + foreach ($fails as $why) { + echo " - $why\n"; + } + exit(1); +} + +echo "ok group push became mass edit\n"; +exit(0); diff --git a/tests/stubs/fog-stubs.php b/tests/stubs/fog-stubs.php index eb1abaf0..afa19049 100644 --- a/tests/stubs/fog-stubs.php +++ b/tests/stubs/fog-stubs.php @@ -155,3 +155,221 @@ public static function createTable(...$args) } } } + +namespace FOG\Base { + /** + * Enough of Hook for a hook's class body to exist and be driven. + * + * Hooks were unreachable from these tests until now: every one of them + * extends this class, so none could be loaded at all, and everything a + * hook decided had to be pinned by reading its source. That is the right + * default for a hook whose job is to echo a form -- and the wrong one for + * a hook that WRITES, where the branch taken decides whether rows survive. + * + * registerInstalled() records rather than registers, so a test can assert + * which events a hook actually asked for. + */ + class Hook + { + /** + * Events this hook registered, as [event, method] pairs. + * + * @var array + */ + public $registered = []; + /** + * What getClass() hands back, keyed by class name. + * + * @var array + */ + public static $classes = []; + /** + * Initialize. + */ + public function __construct() + { + } + /** + * Records the registration. + * + * @param array $events the [event, method] pairs + * + * @return void + */ + public function registerInstalled(array $events) + { + $this->registered = $events; + } + /** + * Hands back a fixture, or a permissive default. + * + * @param string $class the class wanted + * @param mixed $id optional id + * + * @return mixed + */ + public static function getClass($class, $id = null) + { + if (isset(self::$classes[$class])) { + $fixture = self::$classes[$class]; + + return is_callable($fixture) ? $fixture($id) : $fixture; + } + + return new StubItem($id); + } + /** + * A no-op in tests; the real one throws on a bad token. + * + * @return void + */ + public static function checkAuthAndCSRF() + { + } + } + /** + * A stand-in model that exists, has a name, and records writes. + */ + class StubItem + { + /** + * The id it was constructed with. + * + * @var mixed + */ + public $id; + /** + * Whether isValid() answers true. + * + * @var bool + */ + public $valid = true; + /** + * Every insertBatch() call made on it. + * + * @var array + */ + public $batches = []; + /** + * Initialize. + * + * @param mixed $id the id + */ + public function __construct($id = null) + { + $this->id = $id; + } + /** + * Whether the record exists. + * + * @return bool + */ + public function isValid() + { + return $this->valid; + } + /** + * Read a value. + * + * @param string $key the key + * + * @return mixed + */ + public function get($key) + { + return 'name' === $key ? 'StubName' : $this->id; + } + /** + * Records the batch. + * + * @param array $fields the columns + * @param array $values the rows + * + * @return void + */ + public function insertBatch($fields, $values) + { + $this->batches[] = [$fields, $values]; + } + /** + * Returns a select box a form can render. + * + * @return string + */ + public function buildSelectBox(...$args) + { + $name = $args[1] ?? ''; + + return ''; + } + } +} + +namespace FOG\Router { + /** + * Enough of Route to record a deletemass rather than perform one. + */ + class Route + { + /** + * Every deletemass() call, as [class, where]. + * + * @var array + */ + public static $deleted = []; + /** + * Records the delete. + * + * @param string $class the class name + * @param array $where the conditions + * + * @return void + */ + public static function deletemass($class, $where) + { + self::$deleted[] = [$class, $where]; + } + } +} + +namespace FOG\Util { + /** + * Enough of SharedHostValues to feed a hint a fixed answer. + */ + class SharedHostValues + { + /** + * What forHostRows() returns, keyed by friendly key. + * + * @var array + */ + public static $rows = []; + /** + * Returns the fixture. + * + * @return array + */ + public static function forHostRows(...$args) + { + return self::$rows; + } + /** + * Renders the info as text a test can assert on. + * + * @param array $info the info + * + * @return string + */ + public static function hint($info) + { + if (empty($info['uniform'])) { + return '(varies)'; + } + + return '' === (string)$info['value'] + ? '(empty on all)' + : (string)$info['value']; + } + } +}