diff --git a/app/plugins/CoreJob/config/plugin.json b/app/plugins/CoreJob/config/plugin.json index 307e6a147..120075d24 100644 --- a/app/plugins/CoreJob/config/plugin.json +++ b/app/plugins/CoreJob/config/plugin.json @@ -3,6 +3,7 @@ "job": [ "AdopterJob", "AssignerJob", + "DeletionJob", "ProvisionerJob", "SyncJob" ] diff --git a/app/plugins/CoreJob/resources/locales/en_US/core_job.po b/app/plugins/CoreJob/resources/locales/en_US/core_job.po index 2386045ac..49beab675 100644 --- a/app/plugins/CoreJob/resources/locales/en_US/core_job.po +++ b/app/plugins/CoreJob/resources/locales/en_US/core_job.po @@ -34,6 +34,12 @@ msgstr "Source Keys to process, comma separated (requires external_identity_sour msgid "opt.assigner.context" msgstr "Identifier Assignment context" +msgid "opt.deletion.target_id" +msgstr "Record ID of Model to delete" + +msgid "opt.deletion.target_model" +msgstr "Model to delete" + msgid "opt.entities" msgstr "Comma separated list of entity IDs to process" @@ -94,6 +100,21 @@ msgstr "Assigned {0}: {1}" msgid "Assigner.start_summary" msgstr "Assigning all Identifiers in context {0} for CO {1} ({2} entities)" +msgid "Deletion.error.co" +msgstr "DeletionJob can only be scheduled from the COmanage CO" + +msgid "Deletion.error.model" +msgstr "Unsupported target model" + +msgid "Deletion.register_summary" +msgstr "Requested hard delete of {0} {1}" + +msgid "Deletion.start_summary" +msgstr "Beginning hard delete of {0} {1}" + +msgid "Deletion.finish_summary" +msgstr "Deletion completed successfully" + msgid "Provisioner.cancel_summary" msgstr "Job canceled after provisioning {0} entities ({1} errors)" diff --git a/app/plugins/CoreJob/src/Lib/Jobs/DeletionJob.php b/app/plugins/CoreJob/src/Lib/Jobs/DeletionJob.php new file mode 100644 index 000000000..0345e958b --- /dev/null +++ b/app/plugins/CoreJob/src/Lib/Jobs/DeletionJob.php @@ -0,0 +1,107 @@ + [ + 'help' => __d('core_job', 'opt.deletion.target_model'), + 'type' => 'select', + 'choices' => ['Cos'], + 'required' => true + ], + 'target_id' => [ + 'help' => __d('core_job', 'opt.deletion.target_id'), + 'type' => 'integer', + 'required' => true + ] + ]; + } + + /** + * Run the requested Job. + * + * @since COmanage Registry v5.2.0 + * @param JobsTable $JobsTable Jobs table, for updating the Job status + * @param JobHistoryRecordsTable $JobHistoryRecordsTable Job History Records table, for recording additional history + * @param Job $job Job entity + * @param array $parameters Parameters for this Job + * @throws InvalidArgumentException + */ + + public function run( + \App\Model\Table\JobsTable $JobsTable, + \App\Model\Table\JobHistoryRecordsTable $JobHistoryRecordsTable, + \App\Model\Entity\Job $job, + array $parameters + ) { + // Check that the requesting CO is the COmanage CO. + + $requestingCO = $JobsTable->Cos->get($job->co_id); + + if(!$requestingCO->isCOmanageCO()) { + throw new \InvalidArgumentException(__d('core_job', 'Deletion.error.co')); + } + + // We currently only support deleting COs. + + if($parameters['target_model'] != 'Cos') { + throw new \InvalidArgumentException(__d('core_job', 'Deletion.error.model')); + } + + // We need to pull the CO to call delete on it. + + // get() will throw an exception on an invalid CO ID + $targetCO = $JobsTable->Cos->get($parameters['target_id']); + + $JobsTable->start(job: $job, summary: __d('core_job', 'Deletion.start_summary', [$parameters['target_model'], $parameters['target_id']])); + + $JobsTable->Cos->deleteOrFail($targetCO); + + $JobsTable->finish(job: $job, summary: __d('core_job', 'Deletion.finish_summary')); + } +} \ No newline at end of file diff --git a/app/resources/locales/en_US/information.po b/app/resources/locales/en_US/information.po index 229813397..1c587f265 100644 --- a/app/resources/locales/en_US/information.po +++ b/app/resources/locales/en_US/information.po @@ -45,6 +45,9 @@ msgstr "Changelog, expanded, button, click to collapse" msgid "changelog.deleted" msgstr "This record has been deleted" +msgid "cos.delete.sched" +msgstr "This CO is scheduled for deletion" + msgid "cos.none" msgstr "You are not an active member in any collaboration. If your request for enrollment is still being processed, you will not be able to login until it is approved. Please contact an administrator for assistance." diff --git a/app/resources/locales/en_US/operation.po b/app/resources/locales/en_US/operation.po index f1c2a522e..098a9b1d4 100644 --- a/app/resources/locales/en_US/operation.po +++ b/app/resources/locales/en_US/operation.po @@ -184,7 +184,13 @@ msgid "delete" msgstr "Delete" msgid "delete.confirm" -msgstr "Are you sure you wish to delete this record ({0})?" +msgstr "Are you sure you want to delete this record ({0})?" + +msgid "delete.queue" +msgstr "Queue for Deletion" + +msgid "delete.queue.confirm" +msgstr "Are you sure you want to schedule this record ({0}) for deletion?" msgid "duplicate" msgstr "Duplicate" diff --git a/app/src/Controller/CosController.php b/app/src/Controller/CosController.php index 67cd7a86f..9478863d9 100644 --- a/app/src/Controller/CosController.php +++ b/app/src/Controller/CosController.php @@ -31,8 +31,6 @@ // XXX not doing anything with Log yet use Cake\Log\Log; -//use \App\Lib\Enum\PermissionEnum; - use Cake\ORM\TableRegistry; class CosController extends StandardController { @@ -70,6 +68,51 @@ public function beforeRender(\Cake\Event\EventInterface $event) { return parent::beforeRender($event); } + /** + * Handle a delete action for a Standard object. + * + * @since COmanage Registry v5.2.0 + * @param Integer $id Object ID + */ + + public function delete($id) { + // XXX this could ultimately merge into StandardController + if(!empty($this->request->getQuery('queue')) + && $this->request->getQuery('queue') == 'yes') { + // Register a Job to delete the requested entity + + $JobTable = TableRegistry::getTableLocator()->get("Jobs"); + + try { + $comanageco = $this->Cos->find('COmanageCO')->firstOrFail(); + + $JobTable->register( + coId: $comanageco->id, + plugin: 'CoreJob.DeletionJob', + parameters: ['target_model' => 'Cos', 'target_id' => $id], + registerSummary: __d('core_job', 'Deletion.register_summary', ['Cos', $id]) + ); + + // Because (unlike v4 Garbage Collection) Deletion Job doesn't use + // a special status, we update the entity description to provide a + // simple indicator to administrators. See also CFM-94. + + $co = $this->Cos->get($id); + $co->description = __d('information', 'cos.delete.sched'); + $this->Cos->save($co); + + $this->Flash->success(__d('core_job', 'Deletion.register_summary', ['Cos', $id])); + } + catch(\Exception $e) { + $this->Flash->error($e->getMessage()); + } + + return $this->generateRedirect(null); + } else { + return parent::delete($id); + } + } + /* * XXX implement, also REST API * diff --git a/app/src/Lib/Traits/RuleTrait.php b/app/src/Lib/Traits/RuleTrait.php new file mode 100644 index 000000000..5b01905e4 --- /dev/null +++ b/app/src/Lib/Traits/RuleTrait.php @@ -0,0 +1,69 @@ +id) { + // Exclude the current record from the uniqueness check + $whereClause['id <>'] = $entity->id; + } + + foreach($options['fields'] as $f) { + $whereClause['LOWER('.$f.')'] = strtolower($entity->$f); + } + + $count = $this->find() + ->where($whereClause) + ->count(); + + if($count > 0) { + // XXX note this error lookup won't work for Plugins + return __d('error', 'exists', [__d('controller', StringUtilities::entityToClassName($entity), [1])]); + } + + return true; + } +} diff --git a/app/src/Model/Behavior/ChangelogBehavior.php b/app/src/Model/Behavior/ChangelogBehavior.php index bb5c019d2..7b80451f8 100644 --- a/app/src/Model/Behavior/ChangelogBehavior.php +++ b/app/src/Model/Behavior/ChangelogBehavior.php @@ -33,6 +33,7 @@ use Cake\Event\Event; use Cake\ORM\Behavior; use Cake\ORM\Query; +use Cake\ORM\TableRegistry; use Cake\Utility\Inflector; class ChangelogBehavior extends Behavior @@ -48,16 +49,24 @@ class ChangelogBehavior extends Behavior */ public function beforeDelete(Event $event, $entity, \ArrayObject $options) { + $subject = $event->getSubject(); + $tableName = $subject->getTable(); + $alias = $subject->getAlias(); + $parentfk = Inflector::singularize($tableName) . "_id"; + if(isset($options['useHardDelete']) && $options['useHardDelete']) { - // Hard delete requested, so just return + // Hard delete requested. In order for a hard delete to succeed, any archive records + // must also (and first) be hard deleted, so we don't run into any (Changelog related) + // foreign key issues (GMR-7). We can just deleteAll since we neither want nor need + // callbacks. + + $Table = TableRegistry::getTableLocator()->get($alias); + + $Table->deleteAll([$parentfk => $entity->id]); + $event->setResult(true); return; } - - $subject = $event->getSubject(); - $table = $subject->getTable(); - $alias = $subject->getAlias(); - $parentfk = Inflector::singularize($table) . "_id"; // Before we do anything else, make sure we're not trying to update an archive record if($entity->deleted || !empty($entity->$parentfk)) { @@ -110,9 +119,9 @@ public function beforeFind(Event $event, Query $query, \ArrayObject $options, bo } $subject = $event->getSubject(); - $table = $subject->getTable(); + $tableName = $subject->getTable(); $alias = $subject->getAlias(); - $parentfk = Inflector::singularize($table) . "_id"; + $parentfk = Inflector::singularize($tableName) . "_id"; LogBehavior::strace($alias, 'Changelog altering find conditions'); @@ -132,8 +141,6 @@ public function beforeFind(Event $event, Query $query, \ArrayObject $options, bo // that will be not-changelog but might become changelog? $query->where([$alias . '.deleted IS NOT true']) ->where([$alias . '.' . $parentfk . ' IS NULL']); - - // XXX need to also check parent key IS NULL } /** @@ -153,9 +160,9 @@ public function beforeSave(Event $event, EntityInterface $entity, \ArrayObject $ } $subject = $event->getSubject(); - $table = $subject->getTable(); + $tableName = $subject->getTable(); $alias = $subject->getAlias(); - $parentfk = Inflector::singularize($table) . "_id"; + $parentfk = Inflector::singularize($tableName) . "_id"; // Before we do anything else, make sure we're not trying to update an archive record if($entity->deleted || !empty($entity->$parentfk)) { diff --git a/app/src/Model/Table/ApiUsersTable.php b/app/src/Model/Table/ApiUsersTable.php index 1276cee3a..96ad31ec4 100644 --- a/app/src/Model/Table/ApiUsersTable.php +++ b/app/src/Model/Table/ApiUsersTable.php @@ -136,6 +136,8 @@ public function beforeMarshal(EventInterface $event, ArrayObject $data, ArrayObj public function buildRules(RulesChecker $rules): RulesChecker { // AR-ApiUser-3 API usernames must be unique across the entire platform. + // Note we don't enforce case insensitive tests here, so we could have two + // different API Users called "co_2.apiuser" and "co_2.ApiUser". $rules->add( $rules->isUnique(['username']), 'usernameUnique', diff --git a/app/src/Model/Table/CosTable.php b/app/src/Model/Table/CosTable.php index 8aec0857b..f8ae36667 100644 --- a/app/src/Model/Table/CosTable.php +++ b/app/src/Model/Table/CosTable.php @@ -34,7 +34,6 @@ use Cake\ORM\Query; use Cake\ORM\RulesChecker; use Cake\ORM\Table; -use Cake\ORM\TableRegistry; use Cake\Utility\Hash; use Cake\Validation\Validator; @@ -47,8 +46,10 @@ class CosTable extends Table { use \App\Lib\Traits\AutoViewVarsTrait; use \App\Lib\Traits\ChangelogBehaviorTrait; use \App\Lib\Traits\CoLinkTrait; + use \App\Lib\Traits\LabeledLogTrait; use \App\Lib\Traits\PermissionsTrait; use \App\Lib\Traits\TableMetaTrait; + use \App\Lib\Traits\RuleTrait; use \App\Lib\Traits\ValidationTrait; /** @@ -159,12 +160,11 @@ public function buildRules(RulesChecker $rules): RulesChecker { $rules->addDelete([$this, 'ruleIsCOmanageCO'], 'isCOmanageCO', ['errorField' => 'name']); - + // AR-CO-3 Two COs cannot share the same name -// XXX CO-1736 In general, these checks should be case insensitive -// (ie: I shouldn't be able to create a CO called "comanage", similarly COUs etc) -// Also, with CO-1845 maybe unique ignores non-alphanumeric - $rules->add($rules->isUnique(['name'], __d('error', 'exists', [__d('controller', 'Cos', [1])]))); + $rules->add([$this, 'ruleIsCaseInsensitiveUnique'], + 'isUnique', + ['errorField' => 'name', 'fields' => ['name']]); // AR-CO-5 A CO cannot be deleted if it is in Active status // This basically requires two steps to delete a CO (set to Suspended), @@ -191,6 +191,25 @@ public function deleteOrFail(EntityInterface $entity, $options = []): bool { // dependency paths, we can't simply rely on Cake's dependency propagation // on delete. + // Before we start, we manually check AR-CO-2 and AR-CO-5 here. Cake's normal + // rule checking won't be applied until we call the parent delete() below, + // at which point we'll already have started hard deleting records, which + // seems not ideal. + + // AR-CO-2 The COmanage CO cannot be deleted + $err = $this->ruleIsCOmanageCO($entity, []); + + if($err !== true) { + throw new \InvalidArgumentException($err); + } + + // AR-CO-5 An Active CO cannot be deleted + $err = $this->ruleIsActive($entity, []); + + if($err !== true) { + throw new \InvalidArgumentException($err); + } + // We ignore $options['useHardDelete'] because COs can _only_ be hard deleted. // We'll start by obtaining the set of models directly associated with the CO model. @@ -216,7 +235,7 @@ public function deleteOrFail(EntityInterface $entity, $options = []): bool { if(method_exists($targetTable, "getPluggableModelType")) { $pluggable[ $a->getClassName() ] = $targetTable; - } elseif($targetTable->getIsConfigurationTable()) { + } elseif($targetTable->isConfigurationTable()) { // eg: CoSettings $targetAssociations = $a->associations(); @@ -245,6 +264,8 @@ public function deleteOrFail(EntityInterface $entity, $options = []): bool { } } + $this->llog('trace', "Beginning deletion of CO " . $entity->id); + // First, delete plugin related models // XXX unclear that we need to do anything here... PluggableModelTrait will // automatically bind instantiated Entry Point Models when a Pluggable Table object @@ -265,13 +286,19 @@ public function deleteOrFail(EntityInterface $entity, $options = []): bool { $this->paginatedDelete($entity->id, $configLast); + $this->llog('trace', "Deleting Changelog archives for CO " . $entity->id); + // Delete any Changelog records for this CO. We can use deleteAll because we // don't need any callbacks to fire. $this->deleteAll(['Cos.co_id' => $entity->id]); + $this->llog('trace', "Deleting CO " . $entity->id); + // Finally, delete the CO itself parent::deleteOrFail($entity, ['useHardDelete' => true, 'checkRules' => false]); + $this->llog('trace', "Finished deletion of CO " . $entity->id); + return true; } @@ -376,10 +403,25 @@ public function localAfterSave(\Cake\Event\EventInterface $event, \Cake\Datasour protected function paginatedDelete(int $coId, array $tableSet) { foreach($tableSet as $tableName => $table) { + // Because of how keyset pagination works, we are pretty much guaranteed + // to retrieve an active record before of any its related changelog + // archives, since the archived copy will generally get a higher record + // key since we maintain the current record key for the active record. + // As such, when we try to delete the active record we'll get a SQL error. + // + // The workaround is to have ChangelogBehavior::beforeDelete remove + // archive copies when a hard delete of an active record is requested + // (this is probably the correct behavior anyway), however we'll then + // run into a different problem, which is the PaginatedSqlIteratpr will + // probably have pulled one or more archived records, which will then + // no longer exist when we try to delete them. + $iterator = new PaginatedSqlIterator(table: $table, conditions: ['co_id' => $coId], options: ['archived' => true]); + $this->llog('trace', "Performing paginated delete from " . $table->getAlias() . " for CO $coId (record count: " . $iterator->count() . ")"); + foreach($iterator as $k => $tentity) { // We call delete on each entity individually so that callbacks fire, // in particular the unsetting of foreign keys that might be set. @@ -391,38 +433,36 @@ protected function paginatedDelete(int $coId, array $tableSet) { } /** - * Application Rule to determine if the current entity is the COmanage CO. - * - * @param Entity $entity Entity to be validated - * @param array $options Application rule options + * Application Rule to determine if the current entity is not Active. * - * @return string|bool true if the Rule check passes, false otherwise * @since COmanage Registry v5.0.0 + * @param Entity $entity Entity to be validated + * @param array $options Application rule options + * @return bool|string true if the Rule check passes, false otherwise */ - public function ruleIsCOmanageCO($entity, array $options): string|bool { - // We want negative logic since we want to fail if we're editing the COmanage CO - if($entity->isCOmanageCO()) { - return __d('error', 'edit.comanage'); - } - + public function ruleIsActive($entity, array $options): bool|string { + // We want negative logic since we want to fail if the record is Active + if($entity->status === TemplateableStatusEnum::Active) { + return __d('error', 'delete.active'); + } + return true; } /** - * Application Rule to determine if the current entity is not Active. - * - * @param Entity $entity Entity to be validated - * @param array $options Application rule options + * Application Rule to determine if the current entity is the COmanage CO. * - * @return bool|string true if the Rule check passes, false otherwise * @since COmanage Registry v5.0.0 + * @param Entity $entity Entity to be validated + * @param array $options Application rule options + * @return string|bool true if the Rule check passes, false otherwise */ - public function ruleIsActive($entity, array $options): bool|string { - // We want negative logic since we want to fail if the record is Active - if($entity->status === TemplateableStatusEnum::Active) { - return __d('error', 'delete.active'); + public function ruleIsCOmanageCO($entity, array $options): string|bool { + // We want negative logic since we want to fail if we're editing the COmanage CO + if($entity->isCOmanageCO()) { + return __d('error', 'edit.comanage'); } return true; diff --git a/app/src/Model/Table/CousTable.php b/app/src/Model/Table/CousTable.php index 3d9185e60..cdd228e45 100644 --- a/app/src/Model/Table/CousTable.php +++ b/app/src/Model/Table/CousTable.php @@ -55,6 +55,7 @@ class CousTable extends Table { use \App\Lib\Traits\ProvisionableTrait{ requestProvisioning as traitRequestProvisioning; } + use \App\Lib\Traits\RuleTrait; use \App\Lib\Traits\SearchFilterTrait; use \App\Lib\Traits\TableMetaTrait; use \App\Lib\Traits\TreeTrait; @@ -163,7 +164,9 @@ public function buildRules(RulesChecker $rules): RulesChecker { ['errorField' => 'parent_id']); // AR-COU-3 Two COUs within the same CO cannot share the same name - $rules->add($rules->isUnique(['name', 'co_id'], __d('error', 'exists', [__d('controller', 'Cous', [1])]))); + $rules->add([$this, 'ruleIsCaseInsensitiveUnique'], + 'isUnique', + ['errorField' => 'name', 'fields' => ['name', 'co_id']]); // This is not an Application Rule per se, but the parent_id must be a valid // potential parent diff --git a/app/templates/Cos/columns.inc b/app/templates/Cos/columns.inc index 0f09ca87b..f540bfe40 100644 --- a/app/templates/Cos/columns.inc +++ b/app/templates/Cos/columns.inc @@ -51,9 +51,9 @@ $rowActions = [ 'label' => __d('operation', 'Cos.switch'), 'if' => 'isActive' ], - [ +/* CFM-127 [ 'action' => 'duplicate', 'class' => '', 'icon' => 'content_copy' - ] + ]*/ ]; diff --git a/app/templates/Cos/fields.inc b/app/templates/Cos/fields.inc index e5e4882fe..b2bc26b9b 100644 --- a/app/templates/Cos/fields.inc +++ b/app/templates/Cos/fields.inc @@ -30,3 +30,26 @@ $fields = [ 'description', 'status' ]; + +// Top Links +if($vv_action == 'edit') { + $topLinks[] = [ + 'icon' => 'add_to_queue', + 'order' => 'Default', + 'label' => __d('operation', 'delete.queue'), + // 'url' => $vv_base_url . $vv_obj->name, + 'link' => [ + 'action' => 'delete', + $vv_obj->id, + '?' => [ + 'queue' => 'yes' + ] + ], + 'confirm' => [ + 'dg_body_txt' => __d('operation', 'delete.queue.confirm', [$vv_obj->id]), + 'dg_confirm_btn' => __d('operation', 'confirm.yes'), + 'dg_cancel_btn' => __d('operation', 'confirm.no') + ], + 'class' => '' + ]; +} \ No newline at end of file