From 246f847a2c65cfd9e067e54e648d7ec64f45e202 Mon Sep 17 00:00:00 2001
From: Stephan Robotta <stephan.robotta@bfh.ch>
Date: Wed, 20 Nov 2024 17:16:48 +0100
Subject: [PATCH] MDL-83772 mod_data: Fixed autosave bug in text areas with
 same name

---
 .../mod/data/field/textarea/field.class.php   | 66 ++++++++++++++---
 .../mod/data/tests/behat/edit_entries.feature | 70 +++++++++++++++++++
 2 files changed, 127 insertions(+), 9 deletions(-)
 create mode 100644 public/mod/data/tests/behat/edit_entries.feature

diff --git a/public/mod/data/field/textarea/field.class.php b/public/mod/data/field/textarea/field.class.php
index 2b69334884c..fd7238ffd08 100644
--- a/public/mod/data/field/textarea/field.class.php
+++ b/public/mod/data/field/textarea/field.class.php
@@ -31,6 +31,13 @@ require_once($CFG->dirroot.'/repository/lib.php');
 class data_field_textarea extends data_field_base {
 
     var $type = 'textarea';
+
+    /**
+     * Prefix for the field name to split field id and the random part.
+     * @var string
+     */
+    protected const RND_PREFIX = 'xZx';
+
     /**
      * priority for globalsearch indexing
      *
@@ -38,6 +45,40 @@ class data_field_textarea extends data_field_base {
      */
     protected static $priority = self::LOW_PRIORITY;
 
+    /**
+     * The field name for the content field.
+     *
+     * @var string|null
+     */
+    protected $fieldname = null;
+
+    /**
+     * Returns a random field name for the content field.
+     *
+     * @return string
+     */
+    protected function get_content_field_name(): string {
+        if ($this->fieldname === null) {
+            $this->fieldname = sprintf(
+                'field_%s_%s%s',
+                $this->field->id,
+                self::RND_PREFIX,
+                bin2hex(random_bytes(5))
+            );
+        }
+        return $this->fieldname;
+    }
+
+    /**
+     * Check if the given field name is the content field that contains the actual data.
+     *
+     * @param string $name
+     * @return bool
+     */
+    protected function is_content_field(string $name): bool {
+        return (strlen($name) === 13 && substr($name, 0, 3) === self::RND_PREFIX);
+    }
+
     public function supports_preview(): bool {
         return true;
     }
@@ -82,7 +123,7 @@ class data_field_textarea extends data_field_base {
         $text   = '';
         $format = 0;
         $str = '<div title="' . s($this->field->description) . '" class="d-inline-flex">';
-        $str .= '<label for="field_' . $this->field->id . '">';
+        $str .= '<label for="' . $this->get_content_field_name() . '">';
         $str .= html_writer::span($this->field->name, 'accesshide');
         if ($this->field->required) {
             $image = $OUTPUT->pix_icon('req', get_string('requiredelement', 'form'));
@@ -109,9 +150,12 @@ class data_field_textarea extends data_field_base {
             } else {
                 $draftitemid = file_get_unused_draft_itemid();
             }
-            $fieldname = 'field_' . $this->field->id;
-            if (isset($formdata->$fieldname)) {
-                $text = $formdata->$fieldname;
+            $fieldname = 'field_' . $this->field->id . '_' . self::RND_PREFIX;
+            foreach (array_keys(get_object_vars($formdata)) as $prop) {
+                if (strpos($prop, $fieldname) === 0) {
+                    $text = $formdata->$prop;
+                    break;
+                }
             }
         } else if ($recordid &&
                    $content = $DB->get_record('data_content', array('fieldid' => $this->field->id, 'recordid' => $recordid))) {
@@ -181,11 +225,11 @@ class data_field_textarea extends data_field_base {
             $formats[$fid] = $strformats[$fid];
         }
         $editor->set_text($text);
-        $editor->use_editor($field, $options, $fpoptions);
+        $editor->use_editor($this->get_content_field_name(), $options, $fpoptions);
         $str .= '<input type="hidden" name="'.$field.'_itemid" value="'.s($draftitemid).'" />';
         $str .= '<div class="mod-data-input">';
-        $str .= '<div><textarea id="'.$field.'" name="'.$field.'" rows="'.$this->field->param3.'" class="form-control" ' .
-            'data-fieldtype="editor" ' .
+        $str .= '<div><textarea id="' . $this->get_content_field_name() . '" name="' . $this->get_content_field_name() . '" ' .
+            'rows="'.$this->field->param3 . '" class="form-control" data-fieldtype="editor" ' .
             'cols="'.$this->field->param2.'" spellcheck="true">'.s($text).'</textarea></div>';
         $str .= '<div><label class="accesshide" for="' . $field . '_content1">' . get_string('format') . '</label>';
         $str .= '<select id="' . $field . '_content1" name="'.$field.'_content1" class="form-select mt-2">';
@@ -201,7 +245,6 @@ class data_field_textarea extends data_field_base {
         return $str;
     }
 
-
     function display_search_field($value = '') {
         return '<label class="accesshide" for="f_' . $this->field->id . '">' . s($this->field->name) . '</label>' .
                '<input type="text" size="16" id="f_' . $this->field->id . '" name="f_' . $this->field->id . '" ' .
@@ -241,11 +284,13 @@ class data_field_textarea extends data_field_base {
             if ($names[2] == 'itemid') {
                 // the value will be retrieved by file_get_submitted_draft_itemid, do not need to save in DB
                 return true;
+            } else if ($this->is_content_field($names[2])) {
+                $content->content = clean_param($value, PARAM_RAW_TRIMMED);
             } else {
                 $content->{$names[2]} = clean_param($value, PARAM_NOTAGS);  // content[1-4]
             }
         } else {
-            $content->content = clean_param($value, PARAM_CLEAN);
+            $content->content = clean_param($value, PARAM_RAW_TRIMMED);
         }
 
         if ($oldcontent = $DB->get_record('data_content', array('fieldid'=>$this->field->id, 'recordid'=>$recordid))) {
@@ -319,6 +364,9 @@ class data_field_textarea extends data_field_base {
             // Don't assume that this is coming from a text editor with tags.
             return strval($value) !== '';
         }
+        if ($this->is_content_field($names[2])) {
+            return strval($value) !== '';
+        }
         return false;
     }
 
diff --git a/public/mod/data/tests/behat/edit_entries.feature b/public/mod/data/tests/behat/edit_entries.feature
new file mode 100644
index 00000000000..67848b5e75b
--- /dev/null
+++ b/public/mod/data/tests/behat/edit_entries.feature
@@ -0,0 +1,70 @@
+@mod @mod_data @javascript @editor_tiny
+Feature: Edit existing database entries
+  In order to modify a database entry
+  As a teacher
+  I need to successfully have changes recorded for each entry
+
+  Background:
+    Given the following "users" exist:
+      | username | firstname | lastname | email                |
+      | teacher1 | Teacher   | 1        | teacher1@example.com |
+    And the following "courses" exist:
+      | fullname | shortname | category |
+      | Course 1 | C1        | 0        |
+    And the following "course enrolments" exist:
+      | user     | course | role           |
+      | teacher1 | C1     | editingteacher |
+    And the following "activities" exist:
+      | activity | name               | intro      | course | idnumber |
+      | data     | Test database name | Intro text | C1     | data1    |
+    And the following "mod_data > fields" exist:
+      | database | type     | name        | required | description |
+      | data1    | text     | headline    | 1        | Headline    |
+      | data1    | textarea | description | 0        | Description |
+    And the following "mod_data > entries" exist:
+      | database | user     | headline   | description   |
+      | data1    | teacher1 | Headline 1 | Original text |
+      | data1    | teacher1 | Headline 2 |               |
+
+  Scenario: Text areas are filled correctly when editing datasets
+    Given I am on the "Test database name" "data activity" page logged in as "teacher1"
+    And I select "Single view" from the "jump" singleselect
+    And I should see "Headline 1" in the "region-main" "region"
+    And I should see "Original text" in the "region-main" "region"
+    And I click on ".defaulttemplate-single-body .action-menu" "css_element"
+    # Edit fields and observe changes.
+    And I click on "Edit" "link" in the ".defaulttemplate-single-body .dropdown-menu" "css_element"
+    And I wait until the page is ready
+    And I set the field "headline" to "New headline"
+    And I set the field "description" to "New text"
+    When I click on "Save" "button" in the "sticky-footer" "region"
+    Then I should not see "Headline 1" in the "region-main" "region"
+    And I should not see "Original text" in the "region-main" "region"
+    And I should see "New headline" in the "region-main" "region"
+    And I should see "New text" in the "region-main" "region"
+
+  Scenario: Text areas are filled correctly when triggering autosave without making changes
+    Given I am on the "Test database name" "data activity" page logged in as "teacher1"
+    And I select "Single view" from the "jump" singleselect
+    # Edit the the first DB entry textfield, but don't save the changes.
+    And I click on ".defaulttemplate-single-body .action-menu" "css_element"
+    And I click on "Edit" "link" in the ".defaulttemplate-single-body .dropdown-menu" "css_element"
+    And I wait until the page is ready
+    And I set the field "description" to "Unsaved changes"
+    # Trigger autosave.
+    And I press tab
+    And I wait "1" seconds
+    And I click on "Cancel" "button" in the "sticky-footer" "region"
+    # Edit the second DB entry headline only.
+    And I select "Single view" from the "jump" singleselect
+    And I click on "2" "link" in the "sticky-footer" "region"
+    And I click on ".defaulttemplate-single-body .action-menu" "css_element"
+    And I click on "Edit" "link" in the ".defaulttemplate-single-body .dropdown-menu" "css_element"
+    And I wait until the page is ready
+    And I set the field "headline" to "New headline"
+    And I click on "Save" "button" in the "sticky-footer" "region"
+    And I select "Single view" from the "jump" singleselect
+    And I click on "2" "link" in the "sticky-footer" "region"
+    # Only the new heading is updated and the description field autosave did not carry over from the first entry.
+    And I should see "New headline" in the "region-main" "region"
+    And I should not see "Unsaved changes" in the "region-main" "region"