Merge pull request #133 from catalyst/issue132-ophaned-records

Issue #132: ensure child records are deleted with their parents

Author: matthewhilton

classes/observation_manager.php

@@ -216,6 +216,7 @@ public static function delete_observation_point(int $observationid, int $obpoint
 
         $transaction = $DB->start_delegated_transaction();
 
+        $DB->delete_records('observation_point_responses', ['obs_pt_id' => $obpointid]);
         $DB->delete_records('observation_points', ['id' => $obpointid, 'obs_id' => $observationid]);
 
         // Shuffle those above down.

classes/session_manager.php

@@ -303,4 +303,16 @@ public static function cancel_session(int $sessionid) {
             'finish_time' => time()]);
         return;
     }
+
+    /**
+     * Deletes observation session
+     * @param int $observationid ID of the observation instance
+     * @param int $sessionid ID of the observation session to delete
+     */
+    public static function delete_observation_session(int $observationid, int $sessionid) {
+        global $DB;
+
+        $DB->delete_records('observation_point_responses', ['obs_ses_id' => $sessionid]);
+        $DB->delete_records('observation_sessions', ['id' => $sessionid, 'obs_id' => $observationid]);
+    }
 }

classes/timeslot_manager.php

@@ -155,6 +155,7 @@ public static function delete_time_slot(int $observationid, int $slotid, int $ac
             self::send_cancellation_message($observationid, $slotid, $timeslot->observee_id, $actioninguserid);
         }
 
+        $DB->delete_records('observation_notifications', ['timeslot_id' => $slotid]);
         $DB->delete_records('observation_timeslots', ['id' => $slotid, 'obs_id' => $observationid]);
     }
 

db/upgrade.php

@@ -108,5 +108,31 @@ function xmldb_observation_upgrade($oldversion) {
         upgrade_mod_savepoint(true, 2021052533, 'observation');
     }
 
+    if ($oldversion < 2024052801) {
+        // Clean up any orphan records.
+        $obssqlwhere = 'obs_id NOT IN (SELECT id FROM {observation})';
+
+        // Time slots.
+        $DB->delete_records_select('observation_timeslots', $obssqlwhere);
+
+        // Notifications.
+        $sqlwhere = 'timeslot_id NOT IN (SELECT id FROM {observation_timeslots})';
+        $DB->delete_records_select('observation_notifications', $sqlwhere);
+
+        // Sessions.
+        $DB->delete_records_select('observation_sessions', $obssqlwhere);
+
+        // Points.
+        $DB->delete_records_select('observation_points', $obssqlwhere);
+
+        // Point responses.
+        $sqlwhere = 'obs_pt_id NOT IN (SELECT id FROM {observation_points})
+                     OR obs_ses_id NOT IN (SELECT id FROM {observation_sessions})';
+        $DB->delete_records_select('observation_point_responses', $sqlwhere);
+
+        // Observation savepoint reached.
+        upgrade_mod_savepoint(true, 2024052801, 'observation');
+    }
+
     return true;
 }

lib.php

@@ -106,9 +106,29 @@ function observation_update_instance($data): bool {
  * @return bool true
  */
 function observation_delete_instance($id) {
-    global $DB;
+    global $DB, $USER;
+
+    if (!$observation = $DB->get_record('observation', ['id' => $id])) {
+        return false;
+    }
+
+    $timeslots = \mod_observation\timeslot_manager::get_time_slots($observation->id);
+    foreach ($timeslots as $timeslot) {
+        \mod_observation\timeslot_manager::delete_time_slot($observation->id, $timeslot->id, $USER->id);
+    }
+
+    $sessions = \mod_observation\session_manager::get_sessions($observation->id);
+    foreach ($sessions as $session) {
+        \mod_observation\session_manager::delete_observation_session($observation->id, $session->id);
+    }
+
+    $points = \mod_observation\observation_manager::get_observation_points($observation->id);
+    foreach ($points as $point) {
+        \mod_observation\observation_manager::delete_observation_point($observation->id, $point->id);
+    }
+
+    $DB->delete_records('observation', ['id' => $observation->id]);
 
-    $DB->delete_records('observation', array('id' => $id));
     return true;
 }
 

tests/observation_point_test.php

@@ -90,6 +90,8 @@ private static function create_valid_point($observationid) {
      * Tests CRUD operations for observation point with expected data.
      */
     public function test_crud_expected () {
+        global $DB;
+
         $data = self::VALID_DATA;
         $data['obs_id'] = $this->instance->id;
 
@@ -137,6 +139,9 @@ public function test_crud_expected () {
         $this->assertTrue(in_array($editeddata->title, array_column($returndata, 'title')));
         $this->assertFalse(in_array($data['title'], array_column($returndata, 'title')));
 
+        $responses = $DB->get_records('observation_point_responses', ['obs_pt_id' => $returnedpoint->id]);
+        $this->assertCount(1, $responses);
+
         // Delete point.
         \mod_observation\observation_manager::delete_observation_point($this->instance->id, $returnedpoint->id);
 
@@ -145,6 +150,10 @@ public function test_crud_expected () {
 
         $this->assertEmpty($returndata);
 
+        // Confirm response also deleted.
+        $responses = $DB->get_records('observation_point_responses', ['obs_pt_id' => $returnedpoint->id]);
+        $this->assertCount(0, $responses);
+
         // Cannot access point as no longer exists (throws exception).
         $this->expectException('dml_exception');
         \mod_observation\observation_manager::get_existing_point_data($this->instance->id, $returnedpoint->id);

tests/observation_session_test.php

@@ -285,4 +285,39 @@ public function test_start_session_lockout_neg() {
         \mod_observation\session_manager::start_session($obid, $oberid, $this->observee->id);
         \mod_observation\session_manager::start_session($obid, $oberid, $this->observee2->id);
     }
+
+    /**
+     * Tests CRUD operations for observation point with expected data.
+     */
+    public function test_crud_expected() {
+        global $DB;
+
+        // Create session and submit point response.
+        $sessionid = $this->create_session();
+        $response = (object)self::VALID_RESPONSE;
+        \mod_observation\observation_manager::submit_point_response($sessionid, $this->pointid1, $response);
+
+        $responses = $DB->get_records('observation_point_responses', ['obs_pt_id' => $this->pointid1]);
+        $this->assertCount(1, $responses);
+
+        // Delete session and test response is also deleted.
+        \mod_observation\session_manager::delete_observation_session($this->instance->id, $sessionid);
+
+        $responses = $DB->get_records('observation_point_responses', ['obs_pt_id' => $this->pointid1]);
+        $this->assertCount(0, $responses);
+
+        // Create another session and submit point response.
+        $sessionid = $this->create_session();
+        $response = (object)self::VALID_RESPONSE;
+        \mod_observation\observation_manager::submit_point_response($sessionid, $this->pointid1, $response);
+
+        $responses = $DB->get_records('observation_point_responses', ['obs_pt_id' => $this->pointid1]);
+        $this->assertCount(1, $responses);
+
+        // Delete point and test related responses are deleted.
+        \mod_observation\observation_manager::delete_observation_point($this->instance->id, $this->pointid1);
+
+        $responses = $DB->get_records('observation_point_responses', ['obs_pt_id' => $this->pointid1]);
+        $this->assertCount(0, $responses);
+    }
 }

tests/timeslot_notification_test.php

@@ -111,6 +111,19 @@ public function test_notification_crud() {
 
         $notifys = \mod_observation\timeslot_manager::get_users_notifications($this->instance->id, $this->observee->id);
         $this->assertEquals(0, count($notifys));
+
+        // Create a different notification.
+        \mod_observation\timeslot_manager::create_notification($this->instance->id, $this->slot2id, $this->observee2->id,
+            (object) self::NOTIFY_DATA);
+
+        $notifys = \mod_observation\timeslot_manager::get_users_notifications($this->instance->id, $this->observee2->id);
+        $this->assertEquals(1, count($notifys));
+
+        // Delete timeslot 2 and check notification is also deleted.
+        \mod_observation\timeslot_manager::delete_time_slot($this->instance->id, $this->slot2id, $this->observer->id);
+
+        $notifys = \mod_observation\timeslot_manager::get_users_notifications($this->instance->id, $this->observee2->id);
+        $this->assertEquals(0, count($notifys));
     }
 
     /**

version.php

@@ -25,8 +25,8 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$plugin->version   = 2022041900;
-$plugin->release   = 2022041900;
+$plugin->version   = 2024052801;
+$plugin->release   = 2024052801;
 $plugin->requires  = 2022041900;
 $plugin->component = 'mod_observation';
 $plugin->maturity = MATURITY_STABLE;