Skip to content

Commit

Permalink
Convert the type on the UserJob entity to be a string
Browse files Browse the repository at this point in the history
Currently we are storing a numeric ID - however if we
permit non-core classes to register types we find numeric ids
quickly become hard to manage as uninstalling an extension could
change the id. This switches to using a string type
  • Loading branch information
eileenmcnaughton committed Jun 28, 2022
1 parent b03e69b commit d2b81f5
Show file tree
Hide file tree
Showing 15 changed files with 4,291 additions and 4,243 deletions.
12 changes: 6 additions & 6 deletions CRM/Core/BAO/UserJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,37 +142,37 @@ public static function getStatuses(): array {
public static function getTypes(): array {
return [
[
'id' => 1,
'id' => 'contact_import',
'name' => 'contact_import',
'label' => ts('Contact Import'),
'class' => 'CRM_Contact_Import_Parser_Contact',
],
[
'id' => 2,
'id' => 'contribution_import',
'name' => 'contribution_import',
'label' => ts('Contribution Import'),
'class' => 'CRM_Contribute_Import_Parser_Contribution',
],
[
'id' => 3,
'id' => 'membership_import',
'name' => 'membership_import',
'label' => ts('Membership Import'),
'class' => 'CRM_Member_Import_Parser_Membership',
],
[
'id' => 4,
'id' => 'activity_import',
'name' => 'activity_import',
'label' => ts('Activity Import'),
'class' => 'CRM_Activity_Import_Parser_Activity',
],
[
'id' => 5,
'id' => 'participant_import',
'name' => 'participant_import',
'label' => ts('Participant Import'),
'class' => 'CRM_Event_Import_Parser_Participant',
],
[
'id' => 6,
'id' => 'custom_field_import',
'name' => 'custom_field_import',
'label' => ts('Multiple Value Custom Field Import'),
'class' => 'CRM_Custom_Import_Parser_Api',
Expand Down
23 changes: 14 additions & 9 deletions CRM/Core/DAO/UserJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*
* Generated from xml/schema/CRM/Core/UserJob.xml
* DO NOT EDIT. Generated by CRM_Core_CodeGen
* (GenCodeChecksum:dbe7ab7b35b9ebe7f7248cc6385e19b6)
* (GenCodeChecksum:4976da6332cddd37dbae0071e2319611)
*/

/**
Expand Down Expand Up @@ -110,11 +110,13 @@ class CRM_Core_DAO_UserJob extends CRM_Core_DAO {
public $status_id;

/**
* @var int|string
* (SQL type: int unsigned)
* Name of the job type, which will allow finding the correct class
*
* @var string
* (SQL type: varchar(64))
* Note that values will be retrieved from the database as a string.
*/
public $type_id;
public $job_type;

/**
* FK to Queue
Expand Down Expand Up @@ -321,12 +323,15 @@ public static function &fields() {
],
'add' => '5.50',
],
'type_id' => [
'name' => 'type_id',
'type' => CRM_Utils_Type::T_INT,
'title' => ts('User Job Type ID'),
'job_type' => [
'name' => 'job_type',
'type' => CRM_Utils_Type::T_STRING,
'title' => ts('User Job Type'),
'description' => ts('Name of the job type, which will allow finding the correct class'),
'required' => TRUE,
'where' => 'civicrm_user_job.type_id',
'maxlength' => 64,
'size' => CRM_Utils_Type::BIG,
'where' => 'civicrm_user_job.job_type',
'table_name' => 'civicrm_user_job',
'entity' => 'UserJob',
'bao' => 'CRM_Core_BAO_UserJob',
Expand Down
2 changes: 1 addition & 1 deletion CRM/Import/DataSource.php
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ private function getAdditionalTrackingFields(): string {
private function getParser() {
$parserClass = '';
foreach (CRM_Core_BAO_UserJob::getTypes() as $type) {
if ($this->getUserJob()['type_id'] === $type['id']) {
if ($this->getUserJob()['job_type'] === $type['id']) {
$parserClass = $type['class'];
}
}
Expand Down
4 changes: 2 additions & 2 deletions CRM/Import/Forms.php
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ protected function createUserJob(): int {
$id = UserJob::create(FALSE)
->setValues([
'created_id' => CRM_Core_Session::getLoggedInContactID(),
'type_id:name' => $this->getUserJobType(),
'job_type' => $this->getUserJobType(),
'status_id:name' => 'draft',
// This suggests the data could be cleaned up after this.
'expires_date' => '+ 1 week',
Expand Down Expand Up @@ -589,7 +589,7 @@ protected function getAvailableFields(): array {
*/
protected function getParser() {
foreach (CRM_Core_BAO_UserJob::getTypes() as $jobType) {
if ($jobType['id'] === $this->getUserJob()['type_id']) {
if ($jobType['id'] === $this->getUserJob()['job_type']) {
$className = $jobType['class'];
$classObject = new $className();
$classObject->setUserJobID($this->getUserJobID());
Expand Down
4 changes: 2 additions & 2 deletions CRM/Import/Parser.php
Original file line number Diff line number Diff line change
Expand Up @@ -1890,10 +1890,10 @@ protected function getFieldMappings(): array {
* @throws \CRM_Core_Exception
*/
public static function runImport($taskContext, $userJobID, $limit) {
$userJob = UserJob::get()->addWhere('id', '=', $userJobID)->addSelect('type_id')->execute()->first();
$userJob = UserJob::get()->addWhere('id', '=', $userJobID)->addSelect('job_type')->execute()->first();
$parserClass = NULL;
foreach (CRM_Core_BAO_UserJob::getTypes() as $userJobType) {
if ($userJob['type_id'] === $userJobType['id']) {
if ($userJob['job_type'] === $userJobType['id']) {
$parserClass = $userJobType['class'];
}
}
Expand Down
39 changes: 39 additions & 0 deletions CRM/Upgrade/Incremental/php/FiveFiftyOne.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,16 @@ public function upgrade_5_51_alpha1($rev): void {
$this->addTask('Backfill "civicrm_queue.status" and "civicrm_queue.error")', 'fillQueueColumns');
}

/**
* Upgrade step; adds tasks including 'runSql'.
*
* @param string $rev
* The version number matching this function name
*/
public function upgrade_5_51_beta1($rev): void {
$this->addTask('Convert UserJob table type_id to job_type', 'updateUserJobTable');
}

public static function fillQueueColumns($ctx): bool {
// Generally, anything we do here is nonsensical because there shouldn't be much real world data,
// and the goal is to require something specific going forward (for anything that has an automatic runner).
Expand Down Expand Up @@ -233,4 +243,33 @@ public static function convertMappingFieldLabelsToNames(): bool {
return TRUE;
}

/**
* Update user job table to use a text job_type not an integer type_id.
*
* This makes it easier for non-core classes to register types as
* sequential is not required.
*
* @param $context
*
* @return bool
*/
public static function updateUserJobTable($context): bool {
self::addColumn($context, 'civicrm_user_job', 'job_type', 'varchar(64) NOT NULL');
// This is really only for rc-upgraders. There has been no stable with type_id.
CRM_Core_DAO::executeQuery(
"UPDATE civicrm_user_job SET job_type =
CASE
WHEN type_id = 1 THEN 'contact_import'
WHEN type_id = 2 THEN 'contribution_import'
WHEN type_id = 3 THEN 'membership_import'
WHEN type_id = 4 THEN 'activity_import'
WHEN type_id = 5 THEN 'participant_import'
WHEN type_id = 6 THEN 'custom_field_import'
END
"
);
self::dropColumn($context, 'civicrm_user_job', 'type_id');
return TRUE;
}

}
1 change: 1 addition & 0 deletions Civi/UserJob/UserJobInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<?php
Loading

0 comments on commit d2b81f5

Please sign in to comment.