From 5f6d35e471b21f31cc833d35b6976fe243ae0ac2 Mon Sep 17 00:00:00 2001
From: Amaia Anabitarte <amaia@moodle.com>
Date: Wed, 16 Nov 2022 13:08:08 +0100
Subject: [PATCH] MDL-76083 mod_data: Fix preset importer to remove existing
 fields.

* We should delete fields that have not been marked as to preserve.

Co-authored by: Laurent David <laurent.david@moodle.com>
---
 .../classes/local/importer/preset_importer.php | 17 ++++++-----------
 mod/data/lib.php                               | 18 ++++++++----------
 mod/data/tests/behat/import_presets.feature    | 17 +++++++++++++++++
 mod/data/tests/behat/use_presets.feature       | 14 ++++++++++++++
 4 files changed, 45 insertions(+), 21 deletions(-)

diff --git a/mod/data/classes/local/importer/preset_importer.php b/mod/data/classes/local/importer/preset_importer.php
index 0394330d9fb..6383b0bc811 100644
--- a/mod/data/classes/local/importer/preset_importer.php
+++ b/mod/data/classes/local/importer/preset_importer.php
@@ -283,17 +283,12 @@ abstract class preset_importer {
         }
 
         // Get rid of all old unused data.
-        if (!empty($preservedfields)) {
-            foreach ($currentfields as $cid => $currentfield) {
-                if (!array_key_exists($cid, $preservedfields)) {
-                    // Data not used anymore so wipe!
-                    print "Deleting field $currentfield->name<br />";
-
-                    $id = $currentfield->id;
-                    // Why delete existing data records and related comments/ratings??
-                    $DB->delete_records('data_content', ['fieldid' => $id]);
-                    $DB->delete_records('data_fields', ['id' => $id]);
-                }
+        foreach ($currentfields as $cid => $currentfield) {
+            if (!array_key_exists($cid, $this->fieldstoupdate)) {
+                $id = $currentfield->id;
+                // Why delete existing data records and related comments/ratings??
+                $DB->delete_records('data_content', ['fieldid' => $id]);
+                $DB->delete_records('data_fields', ['id' => $id]);
             }
         }
 
diff --git a/mod/data/lib.php b/mod/data/lib.php
index a80dc425439..ff9a9809387 100644
--- a/mod/data/lib.php
+++ b/mod/data/lib.php
@@ -2544,17 +2544,15 @@ abstract class data_preset_importer {
         }
 
         /* Get rid of all old unused data */
-        if (!empty($preservedfields)) {
-            foreach ($currentfields as $cid => $currentfield) {
-                if (!array_key_exists($cid, $preservedfields)) {
-                    /* Data not used anymore so wipe! */
-                    print "Deleting field $currentfield->name<br />";
+        foreach ($currentfields as $cid => $currentfield) {
+            if (!array_key_exists($cid, $preservedfields)) {
+                /* Data not used anymore so wipe! */
+                echo "Deleting field $currentfield->name<br />";
 
-                    $id = $currentfield->id;
-                    //Why delete existing data records and related comments/ratings??
-                    $DB->delete_records('data_content', array('fieldid'=>$id));
-                    $DB->delete_records('data_fields', array('id'=>$id));
-                }
+                $id = $currentfield->id;
+                // Why delete existing data records and related comments/ratings??
+                $DB->delete_records('data_content', ['fieldid' => $id]);
+                $DB->delete_records('data_fields', ['id' => $id]);
             }
         }
 
diff --git a/mod/data/tests/behat/import_presets.feature b/mod/data/tests/behat/import_presets.feature
index 238d3c31541..4d36e925235 100644
--- a/mod/data/tests/behat/import_presets.feature
+++ b/mod/data/tests/behat/import_presets.feature
@@ -161,3 +161,20 @@ Feature: Users can import presets
     And I click on "Import preset and apply" "button"
     Then I should not see "Fields mappings"
     And I should see "Preset applied"
+
+  Scenario: Teacher can import from field page on a non-empty database and previous fields will be removed
+    Given the following "mod_data > fields" exist:
+      | database | type | name              | description              |
+      | data1    | text | Test field name   | Test field description   |
+    And I am on the "Mountain landscapes" "data activity" page logged in as teacher1
+    And I follow "Presets"
+    And I click on "Actions" "button"
+    And I choose "Import preset" in the open action menu
+    And I upload "mod/data/tests/fixtures/image_gallery_preset.zip" file to "Preset file" filemanager
+    When I click on "Import preset and apply" "button"
+    And I click on "Continue" "button"
+    Then I should see "Preset applied."
+    And I follow "Fields"
+    And I should see "image"
+    And I should see "title"
+    And I should not see "Test field name"
diff --git a/mod/data/tests/behat/use_presets.feature b/mod/data/tests/behat/use_presets.feature
index c67e9691793..3ce644e73c9 100644
--- a/mod/data/tests/behat/use_presets.feature
+++ b/mod/data/tests/behat/use_presets.feature
@@ -238,3 +238,17 @@ Feature: Users can use predefined presets
     And I should see "Existing fields to be deleted: number"
     And I should not see "If fields to be deleted are of the same type as fields to be created"
     And I should see "If fields to be deleted are of the same type as new fields in the preset"
+
+  Scenario: Teacher can use a preset on a non-empty database and previous fields will be removed
+    Given I am on the "Mountain landscapes" "data activity" page logged in as teacher1
+    And I follow "Fields"
+    And I should see "Test field name"
+    And I follow "Presets"
+    And I click on "fullname" "radio" in the "Image gallery" "table_row"
+    And I click on "Use this preset" "button"
+    And I should see "Existing fields to be deleted: Test field name"
+    When I click on "Apply preset" "button"
+    Then I should see "Preset applied."
+    And I should see "image"
+    And I should see "title"
+    And I should not see "Test field name"