diff --git a/app/config/schema/schema.json b/app/config/schema/schema.json index 7e9227acb..05186b12a 100644 --- a/app/config/schema/schema.json +++ b/app/config/schema/schema.json @@ -270,14 +270,16 @@ "open": { "type": "boolean" }, "status": {}, "group_type": { "type": "string", "size": 2 }, - "nesting_mode_all": { "type": "boolean" } + "nesting_mode_all": { "type": "boolean" }, + "owners_group_id": { "type": "integer", "foreignkey": { "table": "groups", "column": "id" } } }, "indexes": { "groups_i1": { "columns": [ "co_id" ] }, "groups_i2": { "columns": [ "co_id", "name" ] }, "groups_i3": { "columns": [ "co_id", "group_type" ] }, "groups_i4": { "columns": [ "cou_id", "group_type" ] }, - "groups_i5": { "needed": false, "columns": [ "cou_id" ]} + "groups_i5": { "needed": false, "columns": [ "cou_id" ]}, + "groups_i6": { "needed": false, "columns": [ "owners_group_id" ]} } }, @@ -311,19 +313,6 @@ } }, - "group_owners": { - "columns": { - "id": {}, - "group_id": {}, - "person_id": {} - }, - "indexes": { - "group_owners_i1": { "columns": [ "group_id" ]}, - "group_owners_i2": { "columns": [ "person_id" ]}, - "group_owners_i3": { "columns": [ "group_id", "person_id" ]} - } - }, - "names": { "columns": { "id": {}, diff --git a/app/resources/locales/en_US/enumeration.po b/app/resources/locales/en_US/enumeration.po index 676a12464..45d21a71f 100644 --- a/app/resources/locales/en_US/enumeration.po +++ b/app/resources/locales/en_US/enumeration.po @@ -69,6 +69,9 @@ msgstr "Admins" msgid "GroupTypeEnum.M" msgstr "All Members" +msgid "GroupTypeEnum.O" +msgstr "Owners" + msgid "GroupTypeEnum.S" msgstr "Standard" diff --git a/app/resources/locales/en_US/error.po b/app/resources/locales/en_US/error.po index 7129e16e9..50c8cfe47 100644 --- a/app/resources/locales/en_US/error.po +++ b/app/resources/locales/en_US/error.po @@ -93,6 +93,9 @@ msgstr "Please recheck these fields: {0}" msgid "fields.primary_link" msgstr "The Primary Link {0} is frozen and cannot be changed" +msgid "fields.read_only" +msgstr "The field {0} is not modifiable" + msgid "file" msgstr "Cannot read file {0}" @@ -118,9 +121,15 @@ msgstr "Target group is already nested into this Group" msgid "GroupNestings.same" msgstr "Group cannot be nested into itself" +msgid "Groups.name.prefix" +msgstr "Standard Groups may not have names starting with \"CO:\"" + msgid "Groups.nested" msgstr "Group is nested or has nestings, and cannot be suspended or deleted" +msgid "Groups.owners.update" +msgstr "Update called on Owners Group" + msgid "IdentifierAssignments.exists" msgstr "The identifier \"{0}\" is already in use" diff --git a/app/resources/locales/en_US/field.po b/app/resources/locales/en_US/field.po index e5fbe143d..cea0ec87c 100644 --- a/app/resources/locales/en_US/field.po +++ b/app/resources/locales/en_US/field.po @@ -354,6 +354,9 @@ msgstr "{0} Members" msgid "Groups.desc.members.active" msgstr "{0} Active Members" +msgid "Groups.group_type" +msgstr "Group Type" + msgid "Groups.nesting_mode_all" msgstr "Require All for Nested Memberships" @@ -366,6 +369,12 @@ msgstr "Open" msgid "Groups.open.desc" msgstr "Open groups may be self-joined by any Person in the CO" +msgid "Groups.owners.desc.affix" +msgstr "{0} Owners" + +msgid "Groups.owners_group_id" +msgstr "Owners Group" + msgid "IdentifierAssignments.email_address_type_id" msgstr "Email Address Type" diff --git a/app/src/Command/SetupCommand.php b/app/src/Command/SetupCommand.php index c6845e434..a57f85155 100644 --- a/app/src/Command/SetupCommand.php +++ b/app/src/Command/SetupCommand.php @@ -167,16 +167,19 @@ public function execute(Arguments $args, ConsoleIo $io) ], ['validate' => false])]; - $person->group_members = [$coTable->People->GroupMembers->newEntity([ - 'group_id' => $coTable->Groups->getAdminGroupId(coId: $co_id) - ], - ['validate' => false])]; + $g = $$coTable->Groups->find('adminGroup', ['co_id' => $co_id])->firstOrFail(); + + $person->group_members = [ + $coTable->People->GroupMembers->newEntity( + ['group_id' => $g->id], + ['validate' => false] + ), + $coTable->People->GroupMembers->newEntity( + ['group_id' => $g->owners_group_id], + ['validate' => false] + ) + ]; - $person->group_owners = [$coTable->People->GroupOwners->newEntity([ - 'group_id' => $coTable->Groups->getAdminGroupId(coId: $co_id) - ], - ['validate' => false])]; - $coTable->People->save($person); // Write the salt file if not set in environment and file does not exist. diff --git a/app/src/Command/TransmogrifyCommand.php b/app/src/Command/TransmogrifyCommand.php index f2efead3f..43651d127 100644 --- a/app/src/Command/TransmogrifyCommand.php +++ b/app/src/Command/TransmogrifyCommand.php @@ -38,6 +38,7 @@ use Cake\I18n\FrozenTime; use Cake\ORM\TableRegistry; use Cake\Utility\Inflector; +use \App\Lib\Util\PaginatedSqlIterator; use Doctrine\DBAL\DriverManager; use Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException; @@ -77,7 +78,7 @@ class TransmogrifyCommand extends Command { 'fieldMap' => [ 'global_search_limit' => 'search_global_limit', 'required_fields_addr' => 'required_fields_address', - 'telephone_number_permitted_fields' => '&populate_co_settings_phone', + 'permitted_fields_telephone_number' => '&populate_co_settings_phone', // XXX CFM-80 these fields are not yet migrated // be sure to add appropriate fields to 'booleans' 'enable_nsf_demo' => null, // CFM-123 @@ -98,7 +99,9 @@ class TransmogrifyCommand extends Command { 'co_theme_id' => null, 'person_picker_email_type' => null, 'person_picker_identifier_type' => null, - 'person_picker_display_types' => null + 'person_picker_display_types' => null, + // No longer supported in PE, see CFM-316 + 'group_create_admin_only' => null ] ], 'authentication_events' => [ @@ -171,14 +174,17 @@ class TransmogrifyCommand extends Command { 'groups' => [ 'source' => 'cm_co_groups', 'displayField' => 'name', - 'cache' => [ 'co_id' ], + 'cache' => [ 'co_id', 'owners_group_id' ], 'booleans' => [ 'nesting_mode_all', 'open' ], 'fieldMap' => [ // auto is implied by group_type 'auto' => null, // Rename the changelog key - 'co_group_id' => 'group_id' - ] + 'co_group_id' => 'group_id', + // Make sure group_type is populated if not already set + 'group_type' => '?S' + ], + 'postTable' => 'createOwnersGroups' ], 'group_nestings' => [ 'source' => 'cm_co_group_nestings', @@ -206,7 +212,7 @@ class TransmogrifyCommand extends Command { // Temporary until implemented 'source_org_identity_id' => null ], - 'preRow' => 'check_group_memberships' + 'preRow' => 'check_group_membership' ], 'names' => [ 'source' => 'cm_names', @@ -326,7 +332,10 @@ class TransmogrifyCommand extends Command { 'complete_time' => 'finish_time', 'job_type_fk' => null, 'job_params' => 'parameters', - 'requeued_from_co_job_id' => 'requeued_from_job_id' + 'requeued_from_co_job_id' => 'requeued_from_job_id', + // XXX CFM-246 not yet supported + 'max_retry' => null, + 'max_retry_count' => null ], 'preRow' => 'filterJobs' ], @@ -413,7 +422,7 @@ protected function cacheResults(string $table, array $row) { } /** - * Check if a group membership is actually asserted. + * Check if a group membership is actually asserted, and reassign ownerships. * * @since COmanage Registry v5.0.0 * @param array $origRow Row of table data (original data) @@ -421,29 +430,80 @@ protected function cacheResults(string $table, array $row) { * @throws InvalidArgumentException */ - protected function check_group_memberships(array $origRow, array $row) { - if($row['owner'] && !$row['deleted'] && !$row['co_group_member_id']) { - // Insert a GroupOwner row for this record. Note we ignore valid from and - // through for this. We also ignore non-current changelog records. - - $ownerRow = [ - 'group_id' => $origRow['co_group_id'], - 'person_id' => $origRow['co_person_id'], - 'created' => $origRow['created'], - 'modified' => $origRow['modified'], - 'group_owner_id' => null, - 'revision' => 0, - 'deleted' => 'f', - 'actor_identifier' => $origRow['actor_identifier'] - ]; - - $this->outconn->insert('group_owners', $ownerRow); + protected function check_group_membership(array $origRow, array $row) { + // We need to handle the various member+owner scenarios, but basically + // (1) If 'owner' is set, manually create a Group Membership in the appropriate + // Owners Group (we need to be called via preRow to do this) + // (2) If 'member' is NOT set, throw an exception so we don't create + // in invalid membership + // (3) Otherwise just return so the Membership gets created + + if($origRow['owner'] && !$origRow['deleted'] && !$origRow['co_group_member_id']) { + // Create a membership in the appropriate owners group, but not + // on changelog entries + + if(!empty($this->cache['groups']['id'][ $origRow['co_group_id'] ]['owners_group_id'])) { + $ownerRow = [ + 'group_id' => $this->cache['groups']['id'][ $origRow['co_group_id'] ]['owners_group_id'], + 'person_id' => $origRow['co_person_id'], + 'created' => $origRow['created'], + 'modified' => $origRow['modified'], + 'group_member_id' => null, + 'revision' => 0, + 'deleted' => 'f', + 'actor_identifier' => $origRow['actor_identifier'] + ]; + + $this->outconn->insert('group_members', $ownerRow); + } else { + $this->io->error("Could not find owners group for CoGroupMember " . $origRow['id']); + } } - - if(!$row['member']) { + + if(!$row['member'] && !$row['owner']) { throw new \InvalidArgumentException('member not set on GroupMember'); } } + + /** + * Create an Owners Group for an existing Group. + * + * @since COmanage Registry v5.0.0 + * @param array $origRow Row of table data (original data) + * @param array $row Row of table data (post fixes) + */ + + protected function createOwnersGroups() { + // Pull all Groups and create Owners Group for them. Deployments generally + // don't have so many Groups that we need PaginatedSqlIterator, but we'll + // use it here anyway just in case. + + // By doing this once for the table we avoid having to sort through + // changelog metadata to figure out which rows to actually create owners + // groups for. + + $Groups = TableRegistry::getTableLocator()->get('Groups'); + + $iterator = new PaginatedSqlIterator($Groups, []); + + foreach($iterator as $k => $group) { + try { + // Because PaginatedSqlIterator will pick up new Groups as we create them, + // we need to check for any Owners groups (that we just created) and skip them. + if(!$group->isOwners()) { + $ownersGid = $Groups->createOwnersGroup($group); + + // We need to manually populate the cache + $this->cache['groups']['id'][$group->id]['owners_group_id'] = $ownersGid; + } + } + catch(\Exception $e) { + $this->io->error("Failed to create owners group for " + . $group->name . " (" . $group->id . "): " + . $e->getMessage()); + } + } + } /** * Execute the Transmogrify Command. @@ -888,6 +948,13 @@ protected function mapFields(string $table, array &$row) { if(!$row[$oldname]) { throw new \InvalidArgumentException("Could not find value for $table $oldname"); } + } elseif($newname[0] == '?') { + // This is a default value to populate if the current value is null + $v = substr($newname, 1); + + if($row[$oldname] === null) { + $row[$oldname] = $v; + } } else { // Copy the value to the new name, then unset the old name $row[$newname] = $row[$oldname]; @@ -917,7 +984,7 @@ protected function map_address_type(array $row) { */ protected function map_affiliation_type(array $row) { - return $this->map_type($row, 'PersonRoles.affiliation_type', $this->findCoId($row), 'affiliation_type'); + return $this->map_type($row, 'PersonRoles.affiliation_type', $this->findCoId($row), 'affiliation'); } /** @@ -1134,7 +1201,7 @@ protected function map_telephone_type(array $row) { * Map a type string to a foreign key. * * @since COmanage Registry v5.0.0 - * @param array $row Row of table data (ignored) + * @param array $row Row of table data * @param string $type Type to map (types:attribute) * @param int $coId CO ID * @param string $attr Row column to use for type value diff --git a/app/src/Controller/GroupOwnersController.php b/app/src/Controller/GroupOwnersController.php deleted file mode 100644 index 0cdbb5236..000000000 --- a/app/src/Controller/GroupOwnersController.php +++ /dev/null @@ -1,61 +0,0 @@ - [ - 'People.primary_name.name' => 'asc' - ] - ]; - - /** - * Callback run prior to the request render. - * - * @since COmanage Registry v5.0.0 - * @param EventInterface $event Cake Event - */ - - public function beforeRender(\Cake\Event\EventInterface $event) { - // Pull the Group name for breadcrumb rendering - - $link = $this->getPrimaryLink(true); - - if(!empty($link->value)) { - $this->set('vv_bc_parent_obj', $this->GroupOwners->Groups->get($link->value)); - $this->set('vv_bc_parent_displayfield', $this->GroupOwners->Groups->getDisplayField()); - } - - return parent::beforeRender($event); - } -} \ No newline at end of file diff --git a/app/src/Lib/Enum/GroupTypeEnum.php b/app/src/Lib/Enum/GroupTypeEnum.php index fe12fdcc5..ae4144a47 100644 --- a/app/src/Lib/Enum/GroupTypeEnum.php +++ b/app/src/Lib/Enum/GroupTypeEnum.php @@ -33,5 +33,6 @@ class GroupTypeEnum extends StandardEnum { const ActiveMembers = 'MA'; const Admins = 'A'; const AllMembers = 'M'; + const Owners = 'O'; const Standard = 'S'; } \ No newline at end of file diff --git a/app/src/Lib/Util/PaginatedSqlIterator.php b/app/src/Lib/Util/PaginatedSqlIterator.php index 2b2a4e3a8..f39b3ad8c 100644 --- a/app/src/Lib/Util/PaginatedSqlIterator.php +++ b/app/src/Lib/Util/PaginatedSqlIterator.php @@ -76,7 +76,7 @@ public function __construct($table, $conditions=null, $options=[]) { * @return mixed Element at the current position */ - public function current() { + public function current(): mixed { return $this->results[$this->position]; } @@ -113,7 +113,7 @@ public function key(): int { * @since COmanage Registry v3.3.0 */ - protected function loadCount() { + protected function loadCount(): void { $query = $this->table->find(); if($this->conditions) { @@ -129,7 +129,7 @@ protected function loadCount() { * @since COmanage Registry v3.3.0 */ - protected function loadPage() { + protected function loadPage(): void { unset($this->results); $this->results = null; @@ -172,7 +172,7 @@ protected function loadPage() { * @since COmanage Registry v3.3.0 */ - public function next() { + public function next(): void { $this->position++; if($this->position >= count($this->results)) { @@ -187,7 +187,7 @@ public function next() { * @since COmanage Registry v3.3.0 */ - public function rewind() { + public function rewind(): void { $this->maxid = 0; $this->loadPage(); @@ -200,7 +200,7 @@ public function rewind() { * @return boolean True if the current position is valid, false otherwise */ - public function valid() { + public function valid(): bool { return !empty($this->results[$this->position]); } } diff --git a/app/src/Model/Entity/Group.php b/app/src/Model/Entity/Group.php index e47027272..b960892c3 100644 --- a/app/src/Model/Entity/Group.php +++ b/app/src/Model/Entity/Group.php @@ -39,6 +39,17 @@ class Group extends Entity { 'slug' => false, ]; + /** + * Determine if this entity record can be deleted. + * + * @since COmanage Registry v5.0.0 + * @return bool True if the record can be deleted, false otherwise + */ + + public function canDelete(): bool { + return !$this->isSystem(); + } + /** * Determine if this is the All Members group. * @@ -60,16 +71,16 @@ public function isAllMembers(): bool { public function isAutomatic(): bool { return in_array($this->group_type, [GroupTypeEnum::ActiveMembers, GroupTypeEnum::AllMembers]); } - + /** - * Determine if this entity record can be deleted. - * + * Determine if this is an owners group. + * * @since COmanage Registry v5.0.0 - * @return bool True if the record can be deleted, false otherwise + * @return bool true if this is an owners group, false otherwise. */ - public function canDelete(): bool { - return !$this->isSystem(); + public function isOwners(): bool { + return $this->group_type == GroupTypeEnum::Owners; } /** @@ -80,7 +91,13 @@ public function canDelete(): bool { */ public function isSystem(): bool { - return in_array($this->group_type, [GroupTypeEnum::ActiveMembers, GroupTypeEnum::Admins, GroupTypeEnum::AllMembers]); + return in_array($this->group_type, + [ + GroupTypeEnum::ActiveMembers, + GroupTypeEnum::Admins, + GroupTypeEnum::AllMembers, + GroupTypeEnum::Owners + ]); } /** diff --git a/app/src/Model/Entity/GroupOwner.php b/app/src/Model/Entity/GroupOwner.php deleted file mode 100644 index 9aae5bea7..000000000 --- a/app/src/Model/Entity/GroupOwner.php +++ /dev/null @@ -1,40 +0,0 @@ - true, - 'id' => false, - 'slug' => false, - ]; -} \ No newline at end of file diff --git a/app/src/Model/Table/GroupOwnersTable.php b/app/src/Model/Table/GroupOwnersTable.php deleted file mode 100644 index 4f258206f..000000000 --- a/app/src/Model/Table/GroupOwnersTable.php +++ /dev/null @@ -1,166 +0,0 @@ -addBehavior('Changelog'); - $this->addBehavior('Log'); - $this->addBehavior('Timestamp'); - - $this->setTableType(\App\Lib\Enum\TableTypeEnum::Secondary); - - // Define associations - $this->belongsTo('Groups'); - $this->belongsTo('People'); - - $this->setDisplayField('id'); - - $this->setPrimaryLink('group_id'); - $this->setRequiresCO(true); - - $this->setEditContains(['Groups', 'People.PrimaryName']); - - $this->setIndexContains(['Groups', 'People.PrimaryName']); - - $this->setPermissions([ -// XXX update for couAdmins, group owners, etc - // Actions that operate over an entity (ie: require an $id) - 'entity' => [ - 'delete' => ['platformAdmin', 'coAdmin'], - 'edit' => false, - 'view' => ['platformAdmin', 'coAdmin'] - ], - // Actions that operate over a table (ie: do not require an $id) - 'table' => [ - 'add' => ['platformAdmin', 'coAdmin'], - 'index' => ['platformAdmin', 'coAdmin'] - ] - ]); - } - - /** - * Define business rules. - * - * @since COmanage Registry v5.0.0 - * @param RulesChecker $rules RulesChecker object - * @return RulesChecker - */ - - public function buildRules(RulesChecker $rules): RulesChecker { -// XXX This isn't explicitly an Application Rule, should it be? -// XXX This isn't a great error message, GroupMember has a better one but needs additional -// context to construct it - $rules->add($rules->isUnique(['person_id', 'group_id'], __d('error', 'exists', [__d('controller', 'GroupOwners', [1])]))); - - return $rules; - } - - /** - * Callback after model save. - * - * @since COmanage Registry v5.0.0 - * @param EventInterface $event Event - * @param EntityInterface $entity Entity (ie: Co) - * @param ArrayObject $options Save options - * @return bool True on success - */ - - public function localAfterSave(\Cake\Event\EventInterface $event, \Cake\Datasource\EntityInterface $entity, \ArrayObject $options): bool { - // Pull the related entities for HistoryRecord comment creation. - $person = $this->People->get($entity->person_id, ['contain' => ['PrimaryName']]); - $group = $this->Groups->get($entity->group_id); - - if($entity->isNew()) { - $action = ActionEnum::GroupOwnerAdded; - $comment = __d('result', 'GroupOwners.added', [$person->primary_name->full_name, $group->name]); - } elseif($entity->get('deleted')) { - $action = ActionEnum::GroupOwnerDeleted; - $comment = __d('result', 'GroupOwners.deleted', [$person->primary_name->full_name, $group->name]); - } else { - // GroupOwners can't currently be edited -// $action = ActionEnum::GroupOwnerEdited; -// $comment = __d('result', 'GroupOwners.edited', [$person->primary_name->full_name, $group->name, $this->changesToString($entity)]); - } - - $this->recordHistory($entity, $action, $comment); - - return true; - } - - /** - * Set validation rules. - * - * @since COmanage Registry v5.0.0 - * @param Validator $validator Validator - * @return Validator Validator - */ - - public function validationDefault(Validator $validator): Validator { - $schema = $this->getSchema(); - - $validator->add('group_id', [ - 'content' => ['rule' => 'isInteger'] - ]); - $validator->notEmptyString('group_id'); - - $validator->add('person_id', [ - 'content' => ['rule' => 'isInteger'] - ]); - $validator->notEmptyString('person_id'); - - return $validator; - } -} \ No newline at end of file diff --git a/app/src/Model/Table/GroupsTable.php b/app/src/Model/Table/GroupsTable.php index bc1005ced..bb8193630 100644 --- a/app/src/Model/Table/GroupsTable.php +++ b/app/src/Model/Table/GroupsTable.php @@ -29,6 +29,7 @@ namespace App\Model\Table; +use Cake\Event\EventInterface; use Cake\ORM\Query; use Cake\ORM\RulesChecker; use Cake\ORM\Table; @@ -73,16 +74,32 @@ public function initialize(array $config): void { // Define associations $this->belongsTo('Cos'); $this->belongsTo('Cous'); - + + // Most Groups (except other Owners groups) have an Owner Group, + // which we should define with a hasOne relation (and a foreign key + // like owners_for_group_id). However, this doesn't intuitively define + // the relationship (having owners_group_id point to the Owners group + // is more obvious than having the Owners group fk back to the original), + // and also makes it more expensive to query the database, so we use + // a belongsTo relation instead. (This also aligns with Cou::parent_id.) + // The downside of this is we have to manually cascade deletes to the Owners + // group, since cascades don't travers belongsTo. + $this->belongsTo('OwnersGroup') + ->setClassName('Groups') + ->setForeignKey('owners_group_id') + ->setProperty('owners_group'); + + // And the inverse relation, for the Owners Group + $this->hasOne('OwnersForGroup') + ->setClassName('Groups') + ->setForeignKey('owners_group_id'); + $this->hasMany('GroupMembers') ->setDependent(true) ->setCascadeCallbacks(true); $this->hasMany('GroupNestings') ->setDependent(true) ->setCascadeCallbacks(true); - $this->hasMany('GroupOwners') - ->setDependent(true) - ->setCascadeCallbacks(true); $this->hasMany('HistoryRecords') ->setDependent(true) ->setCascadeCallbacks(true); @@ -100,7 +117,11 @@ public function initialize(array $config): void { $this->setRequiresCO(true); $this->setEditContains([ - 'Identifiers' + 'Identifiers', + // For an Owners Group, the group it manages owners for + 'OwnersForGroup', + // For a regular group, the Owners Group + 'OwnersGroup' ]); $this->setAutoViewVars([ @@ -108,7 +129,7 @@ public function initialize(array $config): void { 'type' => 'enum', 'class' => 'SuspendableStatusEnum' ], - 'group_types' => [ + 'groupTypes' => [ 'type' => 'enum', 'class' => 'GroupTypeEnum' ] @@ -129,6 +150,8 @@ public function initialize(array $config): void { // Actions that operate over a table (ie: do not require an $id) 'table' => [ 'add' => ['platformAdmin', 'coAdmin'], + // Note that self service Group creation will be implemented via + // a Dashboard widget (CFM-316) and NOT via this index page 'index' => ['platformAdmin', 'coAdmin'] ], // Related models whose permissions we'll need, typically for table views @@ -139,7 +162,6 @@ public function initialize(array $config): void { // groups. Maybe it's OK for now, since all groups are visible to all members of the CO. 'GroupMembers', 'GroupNestings', - 'GroupOwners', 'HistoryRecords', 'IdentifierAssignments', 'Identifiers', @@ -255,6 +277,45 @@ public function addDefaults(int $coId, int $couId=null, bool $rename=false): boo return true; } + /** + * Callback before model delete. + * + * @since COmanage Registry v5.0.0 + * @param CakeEventEvent $event The beforeDelete event + * @param $entity Entity + * @param ArrayObject $options Options + * @return boolean True on success + */ + + public function beforeDelete(EventInterface $event, $entity, \ArrayObject $options) { + // AR-Group-8 When a Group is deleted, its corresponding Owners Group is also deleted. + if(!empty($entity->owners_group_id)) { + $ownersGroup = $this->get($entity->owners_group_id); + $this->delete($ownersGroup); + + // We leave the foreign key in place on $entity in case someone decides + // to look at the archived data. + } + + return true; + } + + /** + * Callback before data is marshaled into an entity. + * + * @since COmanage Registry v5.0.0 + * @param EventInterface $event beforeMarshal event + * @param ArrayObject $data Entity data + * @param ArrayObject $options Callback options + */ + + public function beforeMarshal(EventInterface $event, \ArrayObject $data, \ArrayObject $options) { + // If no group_type was set, this is a Standard Group, so fill in the field. + if(empty($data['group_type'])) { + $data['group_type'] = GroupTypeEnum::Standard; + } + } + /** * Define business rules. * @@ -272,17 +333,77 @@ public function buildRules(RulesChecker $rules): RulesChecker { // are to avoid unexpected consequences from implicitly undoing a nesting... // the administrator must do that first. $rules->addUpdate([$this, 'ruleIsNested'], - 'isNestedUpdate', - ['errorField' => 'status']); + 'isNestedUpdate', + ['errorField' => 'status']); // AR-Group-3 A Group cannot be deleted if it is nested into a Target Group // or is a Target Group for a nesting $rules->addDelete([$this, 'ruleIsNested'], - 'isNestedDelete', - ['errorField' => 'status']); - + 'isNestedDelete', + ['errorField' => 'status']); + + // AR-Group-4 The name, description, and status of a Group of type Owners + // cannot be manually changed. + $rules->addUpdate([$this, 'ruleOwnerIsModified'], + 'ownerDescriptionModified', + ['errorField' => 'description']); + $rules->addUpdate([$this, 'ruleOwnerIsModified'], + 'ownerNameModified', + ['errorField' => 'name']); + $rules->addUpdate([$this, 'ruleOwnerIsModified'], + 'ownerStatusModified', + ['errorField' => 'status']); + + // Similarly, the group_type cannot be changed for any Group + $rules->addUpdate([$this, 'ruleTypeIsModified'], + 'typeModified', + ['errorField' => 'group_type']); + + // AR-Group-9 Standard Groups may not be named starting with the prefix CO:, + // which is reserved for System Groups. + $rules->add([$this, 'ruleCheckNamePrefix'], + 'checkNamePrefix', + ['errorField' => 'name']); + return $rules; } + + /** + * Create an Owners Group for the requested Group. + * + * @since COmanage Registry v5.0.0 + * @param Group $group Group Entity to create an Owners Group for + * @return int Owners Group ID + * @throws PersistenceFailedException + */ + + public function createOwnersGroup($group): int { + if($group->isOwners()) { + throw new \InvalidArgumentException("Group is already an Owners Group"); + } + + $ownerGroup = $this->newEntity([ + 'co_id' => $group->co_id, + 'cou_id' => $group->cou_id, + // For now we just prefix everything with the same string, but maybe + // we want to be smarter for System Groups? + 'name' => 'CO:owners:' . $group->name, + 'description' => __d('field', 'Groups.owners.desc.affix', [$group->name]), + 'open' => false, + 'status' => SuspendableStatusEnum::Active, + 'group_type' => GroupTypeEnum::Owners + ]); + + // AR-Group-6 Groups of type Owners cannot be provisioned. + $this->saveOrFail($ownerGroup); + + // Update the original Group with a pointer to this one + $group->owners_group_id = $ownerGroup->id; + + $this->saveOrFail($group); + + return $ownerGroup->id; + } /** * Find a CO's Administrators group. @@ -354,6 +475,24 @@ public function getMembersViaNesting(int $id, int $groupNestingId): PaginatedSql return $this->getMembers($id, $groupNestingId); } + /** + * Define the table's implemented events. + * + * @since COmanage Registry v5.0.0 + */ + + public function implementedEvents(): array { + $events = parent::implementedEvents(); + + // We need to adjust our beforeDelete priority to run before ChangelogBehavior's. + $events['Model.beforeDelete'] = [ + 'callable' => 'beforeDelete', + 'priority' => 1 + ]; + + return $events; + } + /** * Callback after model save. * @@ -378,6 +517,20 @@ public function localAfterSave(\Cake\Event\EventInterface $event, \Cake\Datasour $this->recordHistory($entity, $action, $comment); + if(!$entity->isOwners()) { + if($entity->isNew()) { + // When a new Group is created, create the owners Group for it. + // This includes automatic Groups. + + $this->createOwnersGroup($entity); + } elseif(!$entity->get('deleted')) { + // If a Group is updated, we may need to update the same attributes + // in the Owners Group. + + $this->updateOwnersGroup($entity); + } + } + return true; } @@ -627,6 +780,24 @@ protected function reconcileNestedMemberships(\Cake\Datasource\EntityInterface $ return true; } + + /** + * Application Rule to determine if the Group name has an invalid prefix. + * + * @since COmanage Registry v5.0.0 + * @param Entity $entity Entity to be validated + * @param array $options Application rule options + * @return boolean true if the Rule check passes, false otherwise + */ + + public function ruleCheckNamePrefix($entity, $options) { + if($entity->group_type == GroupTypeEnum::Standard + && strncmp($entity->name, "CO:", 3)==0) { + return __d('error', 'Groups.name.prefix'); + } + + return true; + } /** * Application Rule to determine if the group is nested. @@ -659,6 +830,49 @@ public function ruleIsNested($entity, $options) { return true; } + /** + * Application Rule to determine if a non-modifiable Owners Group field was modified. + * + * @since COmanage Registry v5.0.0 + * @param Entity $entity Entity to be validated + * @param array $options Application rule options + * @return boolean true if the Rule check passes, false otherwise + */ + + public function ruleOwnerIsModified($entity, $options) { + if(!$entity->isOwners()) { + return true; + } + + // We'll check the field specified in $options['errorField'] + if($entity->isDirty($options['errorField'])) { + return __d('error', 'fields.read_only', $options['errorField']); + } + + return true; + } + + /** + * Application Rule to determine if the Group Type was changed. + * + * @since COmanage Registry v5.0.0 + * @param Entity $entity Entity to be validated + * @param array $options Application rule options + * @return boolean true if the Rule check passes, false otherwise + */ + + public function ruleTypeIsModified($entity, $options) { + if($entity->isDirty('group_type') + // For some reason the field is flagged as dirty on update + // (presumably when we update owners_group_id in localAfterSave) + // so we need to compare the original value + && $entity->get('group_type') != $entity->getOriginal('group_type')) { + return __d('error', 'fields.read_only', 'group_type'); + } + + return true; + } + /** * Perform a keyword search. * @@ -695,6 +909,34 @@ public function search(int $coId, string $q, int $limit) { ->all(); } + /** + * Update the attributes of an Owners Group, based on the attributes of the + * related primary Group. + * + * @since COmanage Registry v5.0.0 + * @param Group $group Owners Group entity + * @return int Owners Group ID + * @throws PersistenceFailedException + */ + + public function updateOwnersGroup($group): int { + if($group->isOwners()) { + throw new \InvalidArgumentException(__d('error', 'Groups.owners.desc.affix')); + } + + $ownerGroup = $this->get($group->owners_group_id); + + // We synchronize name, description, and status + $ownerGroup->name = 'CO:owners:' . $group->name; + $ownerGroup->description = $group->name . " Owners"; + $ownerGroup->status = $group->status; + + // We need to disable rule checking since these fields are not normally modifiable + $this->saveOrFail($ownerGroup, ['checkRules' => false]); + + return $ownerGroup->id; + } + /** * Set validation rules. * @@ -739,6 +981,13 @@ public function validationDefault(Validator $validator): Validator { 'content' => ['rule' => ['boolean']] ]); $validator->allowEmptyString('nesting_mode_all'); + + // This will be null for Owner Groups, which don't have further Owner Groups + $validator->add('owner_group_id', [ + 'content' => ['rule' => 'isInteger'] + ]); + $validator->allowEmptyString('owner_group_id'); + return $validator; } diff --git a/app/src/Model/Table/PeopleTable.php b/app/src/Model/Table/PeopleTable.php index 910982195..066393201 100644 --- a/app/src/Model/Table/PeopleTable.php +++ b/app/src/Model/Table/PeopleTable.php @@ -197,6 +197,10 @@ public function initialize(array $config): void { */ public function beforeDelete(\Cake\Event\Event $event, $entity, \ArrayObject $options) { + // Note this callback successfully fires because ChangelogBehavior ignores + // hard deletes. See GroupsTable for an example of using implementedEvents() + // to change priorities. + if(isset($options['useHardDelete']) && $options['useHardDelete'] && $entity->id > 0) { diff --git a/app/templates/GroupOwners/columns.inc b/app/templates/GroupOwners/columns.inc deleted file mode 100644 index 379061d11..000000000 --- a/app/templates/GroupOwners/columns.inc +++ /dev/null @@ -1,50 +0,0 @@ - [ - 'type' => 'relatedLink', - 'model' => 'person', - 'submodel' => 'primary_name', - 'field' => 'full_name', -// XXX not clear how to sort on submodel, but maybe just leave it for UX revisions? -// 'sortable' => 'PrimaryName.family' - ] -]; - -/* -// When the $bulkActions variable exists in a columns.inc config, the "Bulk edit" switch will appear in the index. -$bulkActions = [ - // TODO: develop bulk actions. For now, use a placeholder. - 'delete' => true -]; -*/ - -$subnav = [ - 'name' => 'group', - 'active' => 'owners' -]; \ No newline at end of file diff --git a/app/templates/GroupOwners/fields-nav.inc b/app/templates/GroupOwners/fields-nav.inc deleted file mode 100644 index d0dbed97b..000000000 --- a/app/templates/GroupOwners/fields-nav.inc +++ /dev/null @@ -1,34 +0,0 @@ - 'group', - 'active' => 'owners' -]; \ No newline at end of file diff --git a/app/templates/GroupOwners/fields.inc b/app/templates/GroupOwners/fields.inc deleted file mode 100644 index 302546218..000000000 --- a/app/templates/GroupOwners/fields.inc +++ /dev/null @@ -1,32 +0,0 @@ -Field->control('person_id', ['type' => 'text']); -} \ No newline at end of file diff --git a/app/templates/Groups/columns.inc b/app/templates/Groups/columns.inc index ba15093f0..3f36adfb0 100644 --- a/app/templates/Groups/columns.inc +++ b/app/templates/Groups/columns.inc @@ -31,7 +31,7 @@ $indexColumns = [ ], 'status' => [ 'type' => 'enum', - 'class' => 'StatusEnum', + 'class' => 'SuspendableStatusEnum', 'sortable' => true ], 'description' => [ diff --git a/app/templates/Groups/fields-nav.inc b/app/templates/Groups/fields-nav.inc index 355c896fe..6078c539f 100644 --- a/app/templates/Groups/fields-nav.inc +++ b/app/templates/Groups/fields-nav.inc @@ -48,54 +48,63 @@ $topLinks = [ ] ], 'class' => '' - ], - [ - 'icon' => 'cloud_sync', - 'order' => 'Default', - 'label' => __d('operation', 'provisioning.status'), - 'link' => [ - 'controller' => 'provisioning_targets', - 'action' => 'status', - '?' => [ - 'group_id' => $vv_obj->id - ] - ], - 'class' => '' - ], - [ - 'icon' => 'badge', - 'order' => 'Default', - 'label' => __d('operation', 'identifiers.assign'), - 'link' => [ - 'controller' => 'identifier_assignments', - 'action' => 'assign', - '?' => [ - 'group_id' => $vv_obj->id - ] - ], - 'class' => '' - ], - [ - 'icon' => 'sync', - 'order' => 'Default', - 'label' => __d('operation', 'reconcile'), - 'link' => [ - 'action' => 'reconcile', - $vv_obj->id - ], - 'class' => '' ] ]; +if(!$vv_obj->isOwners()) { + // Most actions aren't available for Owners Groups + $topLinks += [ + [ + 'icon' => 'cloud_sync', + 'order' => 'Default', + 'label' => __d('operation', 'provisioning.status'), + 'link' => [ + 'controller' => 'provisioning_targets', + 'action' => 'status', + '?' => [ + 'group_id' => $vv_obj->id + ] + ], + 'class' => '' + ], + [ + 'icon' => 'badge', + 'order' => 'Default', + 'label' => __d('operation', 'identifiers.assign'), + 'link' => [ + 'controller' => 'identifier_assignments', + 'action' => 'assign', + '?' => [ + 'group_id' => $vv_obj->id + ] + ], + 'class' => '' + ], + [ + 'icon' => 'sync', + 'order' => 'Default', + 'label' => __d('operation', 'reconcile'), + 'link' => [ + 'action' => 'reconcile', + $vv_obj->id + ], + 'class' => '' + ] + ]; +} + // $addMenuLinks is also given slightly different treatment from the typical $topLinks found in most views: // it is a page-global menu used for adding MVEAs and is given special treatment in element/mveaCanvas.php. -$addMenuLinks = [ - [ - 'controller' => 'identifiers', - 'action' => 'add', - 'icon' => 'fingerprint' - ] -]; + +if(!$vv_obj->isOwners()) { + $addMenuLinks = [ + [ + 'controller' => 'identifiers', + 'action' => 'add', + 'icon' => 'fingerprint' + ] + ]; +} $subnav = [ 'name' => 'group', diff --git a/app/templates/Groups/fields.inc b/app/templates/Groups/fields.inc index ce4e5833e..9f621f714 100644 --- a/app/templates/Groups/fields.inc +++ b/app/templates/Groups/fields.inc @@ -25,12 +25,57 @@ * @license Apache License, Version 2.0 (http://www.apache.org/licenses/LICENSE-2.0) */ -print $this->Field->control('name'); +// Most fields can't be edited for Owners Groups +$options = []; -print $this->Field->control('description'); +if($vv_obj->isOwners()) { + $options['readonly'] = true; +} -print $this->Field->control('status'); +print $this->Field->control( + fieldName: 'name', + options: $options +); -print $this->Field->control('open'); +print $this->Field->control( + fieldName: 'description', + options: $options +); + +print $this->Field->control( + fieldName: 'status', + options: $options +); + +print $this->Field->control( + fieldName: 'open', + options: $options +); print $this->Field->control('nesting_mode_all'); + +if($vv_action != 'add') { + print $this->Field->control( + fieldName: 'group_type', + options: ['readonly' => true] + ); + + if($vv_obj->isOwners()) { + // Link to the main group + if(!empty($vv_obj->owners_for_group)) { + print $this->Field->statusControl( + fieldName: 'owners_group_id', + status: $vv_obj->owners_for_group->name, + link: ['url' => ['controller' => 'groups', 'action' => 'edit', $vv_obj->owners_for_group->id]] + ); + } + } else { + // Link to the owners group + + print $this->Field->statusControl( + fieldName: 'owners_group_id', + status: $vv_obj->owners_group->name, + link: ['url' => ['controller' => 'groups', 'action' => 'edit', $vv_obj->owners_group_id]] + ); + } +} diff --git a/app/templates/IdentifierAssignments/columns.inc b/app/templates/IdentifierAssignments/columns.inc index ed843c2b5..8a168457e 100644 --- a/app/templates/IdentifierAssignments/columns.inc +++ b/app/templates/IdentifierAssignments/columns.inc @@ -44,7 +44,7 @@ $indexColumns = [ ], 'status' => [ 'type' => 'enum', - 'class' => 'StatusEnum', + 'class' => 'SuspendableStatusEnum', 'sortable' => true ] ]; diff --git a/app/templates/Servers/columns.inc b/app/templates/Servers/columns.inc index 8ecb6f587..19e4ae6fa 100644 --- a/app/templates/Servers/columns.inc +++ b/app/templates/Servers/columns.inc @@ -36,7 +36,7 @@ $indexColumns = [ ], 'status' => [ 'type' => 'enum', - 'class' => 'StatusEnum', + 'class' => 'SuspendableStatusEnum', 'sortable' => true ] ]; diff --git a/app/templates/Types/columns.inc b/app/templates/Types/columns.inc index 2449df40f..fbbc8a4f3 100644 --- a/app/templates/Types/columns.inc +++ b/app/templates/Types/columns.inc @@ -36,7 +36,7 @@ $indexColumns = [ ], 'status' => [ 'type' => 'enum', - 'class' => 'StatusEnum', + 'class' => 'SuspendableStatusEnum', 'sortable' => true ] ]; diff --git a/app/templates/element/subnavigation.php b/app/templates/element/subnavigation.php index bb624ad0c..b4e2a4afa 100644 --- a/app/templates/element/subnavigation.php +++ b/app/templates/element/subnavigation.php @@ -289,19 +289,6 @@ ); ?> -
  • - Html->link( - __d('controller', 'Owners', [99]), - [ 'controller' => 'group_owners', - 'action' => 'index', - '?' => $linkFilter - ], - ['class' => $linkClass] - ); - ?> -