Merge branch 'MDL-83430' of https://github.com/marinaglancy/moodle

customfield/field/number/classes/field_controller.php

@@ -173,6 +173,10 @@ class field_controller  extends \core_customfield\field_controller {
      * @return string|float|null
      */
     public function prepare_field_for_display(mixed $value, ?context $context = null): string|null|float {
+        if ($provider = provider_base::instance($this)) {
+            return $provider->prepare_export_value($value, $context);
+        }
+
         if ($value === null) {
             return null;
         }
@@ -185,16 +189,11 @@ class field_controller  extends \core_customfield\field_controller {
             }
         } else {
             // Let's format the value.
-            $provider = provider_base::instance($this);
+            $value = format_float((float)$value, $decimalplaces);
-            if ($provider) {
-                $value = $provider->prepare_export_value($value, $context);
-            } else {
-                $value = format_float((float)$value, $decimalplaces);
 
-                // Apply the display format.
+            // Apply the display format.
-                $format = $this->get_configdata_property('display') ?? '{value}';
+            $format = $this->get_configdata_property('display') ?? '{value}';
-                $value = str_replace('{value}', $value, $format);
+            $value = str_replace('{value}', $value, $format);
-            }
         }
 
         return format_string($value, true, ['context' => $context ?? system::instance()]);

customfield/field/number/classes/local/numberproviders/nofactivities.php

@@ -67,8 +67,7 @@ class nofactivities extends provider_base {
         // Define the label for the autocomplete element.
         $valuelabel = get_string('activitytypes', 'customfield_number');
         // Add autocomplete element.
-        $mform->addElement('autocomplete', 'configdata[activitytypes]', $valuelabel, $options, ['multiple' => true])
+        $mform->addElement('autocomplete', 'configdata[activitytypes]', $valuelabel, $options, ['multiple' => true]);
-            ->setHiddenLabel(true);
         $mform->hideIf('configdata[activitytypes]', 'configdata[fieldtype]', 'ne', get_class($this));
         $mform->hideIf('configdata[decimalplaces]', 'configdata[fieldtype]', 'eq', get_class($this));
         $mform->hideIf('configdata[display]', 'configdata[fieldtype]', 'eq', get_class($this));
@@ -124,10 +123,10 @@ class nofactivities extends provider_base {
         $records = $DB->get_records_sql($sql, $params + ['siteid' => SITEID]);
         foreach ($records as $record) {
             $value = (int)$record->cnt;
-            if (!isset($displaywhenzero) && !$value) {
+            if ((string)$displaywhenzero === '' && !$value) {
                 // Do not display the field when the number of activities is zero.
                 if ($record->dataid) {
-                    (new data_controller((int)$record->dataid, (object)['id' => $record->dataid]))->delete();
+                    (new data_controller(0, (object)['id' => $record->dataid]))->delete();
                 }
             } else if (empty($record->dataid) || (int)$record->decvalue != $value) {
                 // Stored value is out of date.
@@ -158,13 +157,20 @@ class nofactivities extends provider_base {
     /**
      * Preparation for export for number of activities provider.
      *
-     * @param mixed $value String or float
+     * @param mixed $value String or float or null if the value is not present in the database for this instance
      * @param \context|null $context Context
      * @return ?string
      */
     public function prepare_export_value(mixed $value, ?\context $context = null): ?string {
-        if (trim((string)$value) === '') {
+        if ($value === null) {
             return null;
+        } else if (round((float)$value) == 0) {
+            $whenzero = $this->field->get_configdata_property('displaywhenzero');
+            if ((string) $whenzero === '') {
+                return null;
+            } else {
+                return format_string($whenzero, true, ['context' => $context ?? \core\context\system::instance()]);
+            }
         } else {
             return format_float((float)$value, 0);
         }

customfield/field/number/classes/provider_base.php

@@ -80,14 +80,34 @@ abstract class provider_base {
     }
 
     /**
-     * Provider specific value preparation for export.
+     * How the field should be displayed
+     *
+     * Called from {@see field_controller::prepare_field_for_display()}
+     * The return value may contain safe HTML but all user input must be passed through
+     * format_string/format_text functions
      *
      * @param mixed $value String or float
      * @param context|null $context Context
-     * @return ?string
+     * @return ?string null if the field should not be displayed or string representation of the field
      */
     public function prepare_export_value(mixed $value, ?\context $context = null): ?string {
-        return $value;
+        if ($value === null) {
+            return null;
+        }
+
+        // By default assumes that configuration 'decimalplaces' and 'displaywhenzero' are
+        // present. If they are not used in this provider, override the method.
+        $decimalplaces = (int) $this->field->get_configdata_property('decimalplaces');
+        if (round((float) $value, $decimalplaces) == 0) {
+            $result = $this->field->get_configdata_property('displaywhenzero');
+            if ((string) $result === '') {
+                return null;
+            } else {
+                return format_string($result, true, ['context' => $context ?? \core\context\system::instance()]);
+            }
+        } else {
+            return format_float((float)$value, $decimalplaces);
+        }
     }
 
     /**

customfield/field/number/tests/local/numberproviders/nofactivities/nofactivities_test.php

@@ -113,4 +113,83 @@ final class nofactivities_test extends advanced_testcase {
         $course1customfield = $DB->get_field('customfield_data', 'decvalue', ['instanceid' => $course1->id]);
         $this->assertEquals(3.0000, $course1customfield);
     }
+
+    /**
+     * Test that the data record is updated/deleted when the value is recalculated
+     *
+     * Also test that export_value() is correct
+     *
+     * @return void
+     */
+    public function test_recalculate_change_value(): void {
+        global $DB;
+
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        // Create a course with one activity.
+        $course1 = $this->getDataGenerator()->create_course();
+        $assigngenerator = $this->getDataGenerator()->get_plugin_generator('mod_assign');
+        $assign1 = $assigngenerator->create_instance(['course' => $course1->id, 'visible' => 1]);
+
+        /** @var \core_customfield_generator $generator */
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_customfield');
+
+        // Create a category and two fields, one with displaywhenzero, another one without.
+        $category = $generator->create_category();
+        $field1 = $generator->create_field([
+            'categoryid' => $category->get('id'),
+            'type' => 'number',
+            'configdata' => [
+                'fieldtype' => nofactivities::class,
+                'activitytypes' => ['assign'],
+                'displaywhenzero' => '0',
+            ],
+        ]);
+        $field2 = $generator->create_field([
+            'categoryid' => $category->get('id'),
+            'type' => 'number',
+            'configdata' => [
+                'fieldtype' => nofactivities::class,
+                'activitytypes' => ['assign'],
+                'displaywhenzero' => '',
+            ],
+        ]);
+        $getdata = fn(\customfield_number\field_controller $field): \customfield_number\data_controller =>
+            \core_customfield\api::get_instance_fields_data([$field->get('id') => $field], (int)$course1->id)[$field->get('id')];
+
+        // Recalculate the value of the field and assert it is set to 1 (one activity in the course).
+        (new \customfield_number\task\cron())->execute();
+        $data = $getdata($field1);
+        $this->assertEquals(1, $data->get('decvalue'));
+        $this->assertSame('1', $data->export_value());
+        $data = $getdata($field2);
+        $this->assertEquals(1, $data->get('decvalue'));
+        $this->assertSame('1', $data->export_value());
+
+        // Add another module, recalculate and assert the value of the field is set to 2 (two activities in the course).
+        $assign2 = $assigngenerator->create_instance(['course' => $course1->id, 'visible' => 1]);
+        (new \customfield_number\task\cron())->execute();
+        $data = $getdata($field1);
+        $this->assertEquals(2, $data->get('decvalue'));
+        $this->assertSame('2', $data->export_value());
+        $data = $getdata($field2);
+        $this->assertEquals(2, $data->get('decvalue'));
+        $this->assertSame('2', $data->export_value());
+
+        // Delete both modules, recalculate.
+        course_delete_module($assign1->cmid);
+        course_delete_module($assign2->cmid);
+        (new \customfield_number\task\cron())->execute();
+        // Field1 (displaywhenzero='0') has the value zero.
+        $data = $getdata($field1);
+        $this->assertNotEmpty($data->get('id'));
+        $this->assertEquals(0, $data->get('decvalue'));
+        $this->assertSame('0', $data->export_value());
+        // Field2 (displaywhenzero='') no longer has a data record and it is not displayed.
+        $data = $getdata($field2);
+        $this->assertEmpty($data->get('id'));
+        $this->assertEquals(null, $data->get('decvalue'));
+        $this->assertSame(null, $data->export_value());
+    }
 }