Skip to content

Commit

Permalink
Merge pull request #56 from catalyst/issue55
Browse files Browse the repository at this point in the history 
issue #55: fix broken checkbox field
  • Loading branch information
@dmitriim
dmitriim authored Mar 22, 2024
2 parents 75c8d8d + 5d7d60f commit 70778fd
Show file tree
Hide file tree
Showing 8 changed files with 185 additions and 109 deletions.
24 changes: 6 additions & 18 deletions classes/local/tool_dynamic_cohorts/condition/cohort_field.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,33 +330,21 @@ protected function get_cohort_operator_value() {
*/
public function get_config_description(): string {
$cohortoperator = $this->get_cohort_operators()[$this->get_cohort_operator_value()];
$fieldname = $this->get_field_name();
$configuredfieldname = $this->get_field_name();

if (empty($fieldname)) {
if (empty($configuredfieldname)) {
return '';
}

$fieldinfo = $this->get_fields_info()[$fieldname];
$fieldinfo = $this->get_fields_info()[$configuredfieldname];
$displayedfieldname = $this->get_field_name_text();
$fieldoperator = $this->get_operator_text($fieldinfo->datatype);
$fieldvalue = $this->get_field_value();

if ($fieldname == 'contextid') {
$fieldvalue = $this->get_category_options()[$fieldvalue];
}

if (in_array($this->get_operator_value(), [self::TEXT_IS_EMPTY, self::TEXT_IS_NOT_EMPTY])) {
$fieldvalue = null;
}

if ($fieldinfo->datatype === self::FIELD_DATA_TYPE_SELECT) {
$fieldvalue = $fieldinfo->param1[$fieldvalue];
}

$fieldname = $fieldinfo->name;
$fieldvalue = $this->get_field_value_text();

$description = get_string('condition:cohort_field_description', 'tool_dynamic_cohorts', (object)[
'operator' => $cohortoperator,
'field' => $fieldname,
'field' => $displayedfieldname,
'fieldoperator' => $fieldoperator,
'fieldvalue' => $fieldvalue,
]);
Expand Down
40 changes: 34 additions & 6 deletions classes/local/tool_dynamic_cohorts/condition/fields_trait.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,43 @@ protected function get_field_value(): ?string {
$fieldvalue = $configdata[$field . '_value'];
}

// A special case for checkbox field.
return $fieldvalue;
}

/**
* Return the field name as a text.
*
* @return string
*/
protected function get_field_name_text(): string {
return $this->get_fields_info()[$this->get_field_name()]->name ?? '-';
}

/**
* Return a field value as a human-readable text.
*
* @return string|null
*/
protected function get_field_value_text(): ?string {
$fieldname = $this->get_field_name();
if (!empty($fieldname) && !empty($this->get_fields_info()[$fieldname])) {
$datatype = $this->get_fields_info()[$fieldname]->datatype;
if ($datatype == self::FIELD_DATA_TYPE_CHECKBOX) {
$fieldvalue = empty($fieldvalue) ? get_string('no') : get_string('yes');
$fieldvalue = $this->get_field_value();
$fieldinfo = $this->get_fields_info();

if (!empty($fieldname) && !empty($fieldinfo[$fieldname])) {
switch ($fieldinfo[$fieldname]->datatype) {
case self::FIELD_DATA_TYPE_SELECT:
case self::FIELD_DATA_TYPE_MENU:
case self::FIELD_DATA_TYPE_CHECKBOX:
$fieldvalue = $fieldinfo[$fieldname]->param1[$fieldvalue];
break;

}
}

if (in_array($this->get_operator_value(), [self::TEXT_IS_EMPTY, self::TEXT_IS_NOT_EMPTY])) {
$fieldvalue = null;
}

return $fieldvalue;
}

Expand All @@ -102,7 +130,7 @@ protected function get_operator_value(): int {
}

/**
* Returns a text for the configured operator based on a field data type.
* Returns a human-readable text for the configured operator based on a field data type.
*
* @param string $fielddatatype Field data type.
* @return string
Expand Down
34 changes: 12 additions & 22 deletions classes/local/tool_dynamic_cohorts/condition/user_profile.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,40 +174,30 @@ protected function get_fields_info(): array {
return $fields;
}

/**
* Return the field name as a text.
*
* @return string
*/
protected function get_field_text(): string {
return $this->get_fields_info()[$this->get_field_name()]->name ?? '-';
}

/**
* Human-readable description of the configured condition.
*
* @return string
*/
public function get_config_description(): string {
$fieldname = $this->get_field_name();

if (empty($fieldname)) {
$configuredfieldname = $this->get_field_name();

if (empty($configuredfieldname)) {
return '';
}

$datatype = $this->get_fields_info()[$fieldname]->datatype;
$fieldinfo = $this->get_fields_info()[$configuredfieldname];
$displayedfieldname = $this->get_field_name_text();
$fieldoperator = $this->get_operator_text($fieldinfo->datatype);

if (in_array($this->get_operator_value(), [self::TEXT_IS_EMPTY, self::TEXT_IS_NOT_EMPTY])) {
return $this->get_field_text() . ' ' . $this->get_operator_text($datatype);
} else {
$fieldvalue = $this->get_field_value();
if ($fieldname == 'auth') {
$authplugins = core_plugin_manager::instance()->get_plugins_of_type('auth');
$fieldvalue = $authplugins[$fieldvalue]->displayname;
}
$fieldvalue = $this->get_field_value_text();

return $this->get_field_text() . ' '. $this->get_operator_text($datatype) . ' ' . $fieldvalue;
}
return get_string('condition:profile_field_description', 'tool_dynamic_cohorts', (object)[
'field' => $displayedfieldname,
'fieldoperator' => $fieldoperator,
'fieldvalue' => $fieldvalue,
]);
}

/**
Expand Down
1 change: 1 addition & 0 deletions lang/en/tool_dynamic_cohorts.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
$string['condition:cohort_membership_broken_description'] = 'Condition is broken. Using the same cohort that the given rule is configured to manage to.';
$string['condition:cohort_field'] = 'Cohort field';
$string['condition:cohort_field_description'] = 'A user {$a->operator} cohorts with field \'{$a->field}\' {$a->fieldoperator} {$a->fieldvalue}';
$string['condition:profile_field_description'] = '{$a->field} {$a->fieldoperator} {$a->fieldvalue}';
$string['condition:user_profile'] = 'User standard profile field';
$string['condition:user_custom_profile'] = 'User custom profile field';
$string['cf_include_missing_data'] = 'Include cohorts with missing data.';
Expand Down
90 changes: 72 additions & 18 deletions tests/local/tool_dynamic_cohorts/condition/cohort_field_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,17 +272,27 @@ public function test_get_sql_data_standard_fields() {
*
* @return \core_customfield\field_controller
*/
protected function create_cohort_custom_field(): \core_customfield\field_controller {

/**
* Create cohort custom field for testing.
*
* @param string $shortname Field shortname
* @param string $datatype $field data type.
*
* @return \core_customfield\field_controller
*/
protected function create_cohort_custom_field(string $shortname = 'testfield1',
string $datatype = 'text'): \core_customfield\field_controller {
$fieldcategory = self::getDataGenerator()->create_custom_field_category([
'component' => 'core_cohort',
'area' => 'cohort',
'name' => 'Other fields',
]);

return self::getDataGenerator()->create_custom_field([
'shortname' => 'testfield1',
'shortname' => $shortname,
'name' => 'Custom field',
'type' => 'text',
'type' => $datatype,
'categoryid' => $fieldcategory->get('id'),
]);
}
Expand All @@ -309,6 +319,32 @@ public function test_config_description_custom_field() {
'A user is member of cohorts with field \'Custom field\' is equal to Test value',
$condition->get_config_description()
);

$checkboxfield = $this->create_cohort_custom_field('checkboxfield', 'checkbox');
$checkboxfieldname = cohort_field::CUSTOM_FIELD_PREFIX . $checkboxfield->get('shortname');
$condition = $this->get_condition([
'cohort_field_operator' => cohort_field::OPERATOR_IS_MEMBER_OF,
'cohort_field_field' => $checkboxfieldname,
$checkboxfieldname . '_operator' => condition_base::TEXT_IS_EQUAL_TO,
$checkboxfieldname . '_value' => 1,
]);

$this->assertSame(
'A user is member of cohorts with field \'Custom field\' is equal to Yes',
$condition->get_config_description()
);

$condition = $this->get_condition([
'cohort_field_operator' => cohort_field::OPERATOR_IS_MEMBER_OF,
'cohort_field_field' => $checkboxfieldname,
$checkboxfieldname . '_operator' => condition_base::TEXT_IS_EQUAL_TO,
$checkboxfieldname . '_value' => 0,
]);

$this->assertSame(
'A user is member of cohorts with field \'Custom field\' is equal to No',
$condition->get_config_description()
);
}

/**
Expand All @@ -326,9 +362,13 @@ public function test_get_sql_data_custom_fields() {
// We need admin to be able to add custom fields data for cohorts.
$this->setAdminUser();

$this->create_cohort_custom_field();
$textfield = $this->create_cohort_custom_field();
$checkboxfield = $this->create_cohort_custom_field('checkboxfield', 'checkbox');

$cohort1 = $this->getDataGenerator()->create_cohort(['customfield_testfield1' => 'Test value 1']);
$cohort1 = $this->getDataGenerator()->create_cohort([
'customfield_' . $textfield->get('shortname') => 'Test value 1',
'customfield_' . $checkboxfield->get('shortname') => '1',
]);
$cohort2 = $this->getDataGenerator()->create_cohort();

$user1 = $this->getDataGenerator()->create_user();
Expand All @@ -340,13 +380,15 @@ public function test_get_sql_data_custom_fields() {
cohort_add_member($cohort2->id, $user3->id);

$totalusers = $DB->count_records('user');
$textfieldname = cohort_field::CUSTOM_FIELD_PREFIX . $textfield->get('shortname');
$checkboxfieldname = cohort_field::CUSTOM_FIELD_PREFIX . $checkboxfield->get('shortname');

// User 1 and user 2 as they are members of cohort 1.
$condition = $this->get_condition([
'cohort_field_operator' => cohort_field::OPERATOR_IS_MEMBER_OF,
'cohort_field_field' => cohort_field::CUSTOM_FIELD_PREFIX . 'testfield1',
cohort_field::CUSTOM_FIELD_PREFIX . 'testfield1_operator' => condition_base::TEXT_IS_EQUAL_TO,
cohort_field::CUSTOM_FIELD_PREFIX . 'testfield1_value' => 'Test value 1',
'cohort_field_field' => $textfieldname,
$textfieldname . '_operator' => condition_base::TEXT_IS_EQUAL_TO,
$textfieldname . '_value' => 'Test value 1',
]);

$result = $condition->get_sql();
Expand All @@ -356,9 +398,9 @@ public function test_get_sql_data_custom_fields() {
// Everyone except user 1 and user 2 as they are member of cohort 1.
$condition = $this->get_condition([
'cohort_field_operator' => cohort_field::OPERATOR_IS_NOT_MEMBER_OF,
'cohort_field_field' => cohort_field::CUSTOM_FIELD_PREFIX . 'testfield1',
cohort_field::CUSTOM_FIELD_PREFIX . 'testfield1_operator' => condition_base::TEXT_IS_EQUAL_TO,
cohort_field::CUSTOM_FIELD_PREFIX . 'testfield1_value' => 'Test value 1',
'cohort_field_field' => $textfieldname,
$textfieldname . '_operator' => condition_base::TEXT_IS_EQUAL_TO,
$textfieldname . '_value' => 'Test value 1',
]);

$result = $condition->get_sql();
Expand All @@ -368,9 +410,9 @@ public function test_get_sql_data_custom_fields() {
// Everyone as cohort is empty.
$condition = $this->get_condition([
'cohort_field_operator' => cohort_field::OPERATOR_IS_NOT_MEMBER_OF,
'cohort_field_field' => cohort_field::CUSTOM_FIELD_PREFIX . 'testfield1',
cohort_field::CUSTOM_FIELD_PREFIX . 'testfield1_operator' => condition_base::TEXT_IS_EQUAL_TO,
cohort_field::CUSTOM_FIELD_PREFIX . 'testfield1_value' => 'Test value 2',
'cohort_field_field' => $textfieldname,
$textfieldname . '_operator' => condition_base::TEXT_IS_EQUAL_TO,
$textfieldname . '_value' => 'Test value 2',
]);

$result = $condition->get_sql();
Expand All @@ -380,15 +422,27 @@ public function test_get_sql_data_custom_fields() {
// User 1, user 2 and user 3 as they are members of cohort 1 and cohort 2 (this one is with missing custom field data).
$condition = $this->get_condition([
'cohort_field_operator' => cohort_field::OPERATOR_IS_MEMBER_OF,
'cohort_field_field' => cohort_field::CUSTOM_FIELD_PREFIX . 'testfield1',
cohort_field::CUSTOM_FIELD_PREFIX . 'testfield1_operator' => condition_base::TEXT_IS_EQUAL_TO,
cohort_field::CUSTOM_FIELD_PREFIX . 'testfield1_value' => 'Test value 1',
cohort_field::CUSTOM_FIELD_PREFIX . 'testfield1_include_missing_data' => '1',
'cohort_field_field' => $textfieldname,
$textfieldname . '_operator' => condition_base::TEXT_IS_EQUAL_TO,
$textfieldname . '_value' => 'Test value 1',
$textfieldname . '_include_missing_data' => '1',
]);

$result = $condition->get_sql();
$sql = "SELECT u.id FROM {user} u {$result->get_join()} WHERE {$result->get_where()}";
$this->assertCount(3, $DB->get_records_sql($sql, $result->get_params()));

// User 1 and user 2 as they are members of cohort 1.
$condition = $this->get_condition([
'cohort_field_operator' => cohort_field::OPERATOR_IS_MEMBER_OF,
'cohort_field_field' => $checkboxfieldname,
$checkboxfieldname . '_operator' => condition_base::TEXT_IS_EQUAL_TO,
$checkboxfieldname . '_value' => 1,
]);

$result = $condition->get_sql();
$sql = "SELECT u.id FROM {user} u {$result->get_join()} WHERE {$result->get_where()}";
$this->assertCount(2, $DB->get_records_sql($sql, $result->get_params()));
}

/**
Expand Down
Loading

0 comments on commit 70778fd

Please sign in to comment.