MDL-63671 tool_cohortroles: Limit data to the system context in provider
authorMihail Geshoski <mihail@moodle.com>
Thu, 8 Nov 2018 08:06:43 +0000 (16:06 +0800)
committerMihail Geshoski <mihail@moodle.com>
Thu, 8 Nov 2018 08:06:43 +0000 (16:06 +0800)
admin/tool/cohortroles/classes/privacy/provider.php
admin/tool/cohortroles/tests/privacy_test.php

index 2cdfca6..3cd324e 100644 (file)
@@ -80,15 +80,25 @@ class provider implements
     public static function get_contexts_for_userid(int $userid) : contextlist {
         $contextlist = new contextlist();
 
-        // Retrieve the User context associated with tool_cohortroles records.
-        $sql = "SELECT DISTINCT c.id
-                  FROM {context} c
-                  JOIN {tool_cohortroles} cr ON cr.userid = c.instanceid AND c.contextlevel = :contextuser
-                 WHERE cr.userid = :userid";
+        // When we process user deletions and expiries, we always delete from the user context.
+        // As a result the cohort role assignments would be deleted, which has a knock-on effect with courses
+        // as roles may change and data may be removed earlier than it should be.
+
+        // Retrieve the context associated with tool_cohortroles records.
+        $sql = "SELECT DISTINCT c.contextid
+                  FROM {tool_cohortroles} tc
+                  JOIN {cohort} c
+                       ON tc.cohortid = c.id
+                  JOIN {context} ctx
+                       ON ctx.id = c.contextid
+                 WHERE tc.userid = :userid
+                       AND (ctx.contextlevel = :contextlevel1
+                           OR ctx.contextlevel = :contextlevel2)";
 
         $params = [
-            'contextuser' => CONTEXT_USER,
-            'userid'       => $userid
+            'userid'        => $userid,
+            'contextlevel1' => CONTEXT_SYSTEM,
+            'contextlevel2' => CONTEXT_COURSECAT
         ];
 
         $contextlist->add_from_sql($sql, $params);
@@ -104,24 +114,29 @@ class provider implements
     public static function get_users_in_context(userlist $userlist) {
         $context = $userlist->get_context();
 
-        // We should process user data from the system context.
         // When we process user deletions and expiries, we always delete from the user context.
         // As a result the cohort role assignments would be deleted, which has a knock-on effect with courses
         // as roles may change and data may be removed earlier than it should be.
-        if (!$context instanceof \context_system) {
-            return;
-        }
 
-        $params = [
-            'contextid' => $context->id
+        $allowedcontextlevels = [
+            CONTEXT_SYSTEM,
+            CONTEXT_COURSECAT
         ];
 
+        if (!in_array($context->contextlevel, $allowedcontextlevels)) {
+            return;
+        }
+
         $sql = "SELECT tc.userid as userid
                   FROM {tool_cohortroles} tc
                   JOIN {cohort} c
                        ON tc.cohortid = c.id
                  WHERE c.contextid = :contextid";
 
+        $params = [
+            'contextid' => $context->id
+        ];
+
         $userlist->add_from_sql('userid', $sql, $params);
     }
 
@@ -133,24 +148,28 @@ class provider implements
     public static function export_user_data(approved_contextlist $contextlist) {
         global $DB;
 
-        // If the user has tool_cohortroles data, then only the User context should be present so get the first context.
-        $contexts = $contextlist->get_contexts();
-        if (count($contexts) == 0) {
-            return;
-        }
-        $context = reset($contexts);
+        // Remove contexts different from SYSTEM or COURSECAT.
+        $contextids = array_reduce($contextlist->get_contexts(), function($carry, $context) {
+            if ($context->contextlevel == CONTEXT_SYSTEM || $context->contextlevel == CONTEXT_COURSECAT) {
+                $carry[] = $context->id;
+            }
+            return $carry;
+        }, []);
 
-        // Sanity check that context is at the User context level, then get the userid.
-        if ($context->contextlevel !== CONTEXT_USER) {
+        if (empty($contextids)) {
             return;
         }
-        $userid = $context->instanceid;
+
+        $userid = $contextlist->get_user()->id;
+
+        list($contextsql, $contextparams) = $DB->get_in_or_equal($contextids, SQL_PARAMS_NAMED);
 
         // Retrieve the tool_cohortroles records created for the user.
-        $sql = 'SELECT cr.id as cohortroleid,
+        $sql = "SELECT cr.id as cohortroleid,
                        c.name as cohortname,
                        c.idnumber as cohortidnumber,
                        c.description as cohortdescription,
+                       c.contextid as contextid,
                        r.shortname as roleshortname,
                        cr.userid as userid,
                        cr.timecreated as timecreated,
@@ -158,13 +177,13 @@ class provider implements
                   FROM {tool_cohortroles} cr
                   JOIN {cohort} c ON c.id = cr.cohortid
                   JOIN {role} r ON r.id = cr.roleid
-                 WHERE cr.userid = :userid';
+                 WHERE cr.userid = :userid
+                       AND c.contextid {$contextsql}";
 
-        $params = [
-            'userid' => $userid
-        ];
+        $params = ['userid' => $userid] + $contextparams;
 
         $cohortroles = $DB->get_records_sql($sql, $params);
+
         foreach ($cohortroles as $cohortrole) {
             // The tool_cohortroles data export is organised in:
             // {User Context}/Cohort roles management/{cohort name}/{role shortname}/data.json.
@@ -184,6 +203,8 @@ class provider implements
                 'timemodified' => transform::datetime($cohortrole->timemodified)
             ];
 
+            $context = \context::instance_by_id($cohortrole->contextid);
+
             writer::with_context($context)->export_data($subcontext, $data);
         }
     }
@@ -196,14 +217,24 @@ class provider implements
     public static function delete_data_for_all_users_in_context(\context $context) {
         global $DB;
 
-        // Sanity check that context is at the User context level, then get the userid.
-        if ($context->contextlevel !== CONTEXT_USER) {
+        // When we process user deletions and expiries, we always delete from the user context.
+        // As a result the cohort role assignments would be deleted, which has a knock-on effect with courses
+        // as roles may change and data may be removed earlier than it should be.
+
+        $allowedcontextlevels = [
+            CONTEXT_SYSTEM,
+            CONTEXT_COURSECAT
+        ];
+
+        if (!in_array($context->contextlevel, $allowedcontextlevels)) {
             return;
         }
-        $userid = $context->instanceid;
 
-        // Delete the tool_cohortroles records created for the userid.
-        $DB->delete_records('tool_cohortroles', ['userid' => $userid]);
+        $cohortids = $DB->get_fieldset_select('cohort', 'id', 'contextid = :contextid',
+            ['contextid' => $context->id]);
+
+        // Delete the tool_cohortroles records created in the specific context.
+        $DB->delete_records_list('tool_cohortroles', 'cohortid', $cohortids);
     }
 
     /**
@@ -214,15 +245,42 @@ class provider implements
     public static function delete_data_for_users(approved_userlist $userlist) {
         global $DB;
 
-        $context = $userlist->get_context();
-
-        // We should process user data from the system context.
         // When we process user deletions and expiries, we always delete from the user context.
         // As a result the cohort role assignments would be deleted, which has a knock-on effect with courses
         // as roles may change and data may be removed earlier than it should be.
-        if ($context instanceof \context_system) {
-            $DB->delete_records_list('tool_cohortroles', 'userid', $userlist->get_userids());
+
+        $userids = $userlist->get_userids();
+
+        if (empty($userids)) {
+            return;
         }
+
+        $context = $userlist->get_context();
+
+        $allowedcontextlevels = [
+            CONTEXT_SYSTEM,
+            CONTEXT_COURSECAT
+        ];
+
+        if (!in_array($context->contextlevel, $allowedcontextlevels)) {
+            return;
+        }
+
+        $cohortids = $DB->get_fieldset_select('cohort', 'id', 'contextid = :contextid',
+            ['contextid' => $context->id]);
+
+        if (empty($cohortids)) {
+            return;
+        }
+
+        list($cohortsql, $cohortparams) = $DB->get_in_or_equal($cohortids, SQL_PARAMS_NAMED);
+        list($usersql, $userparams) = $DB->get_in_or_equal($userids, SQL_PARAMS_NAMED);
+
+        $params = $cohortparams + $userparams;
+        $select = "cohortid {$cohortsql} AND userid {$usersql}";
+
+        // Delete the tool_cohortroles records created in the specific context for an approved list of users.
+        $DB->delete_records_select('tool_cohortroles', $select, $params);
     }
 
     /**
@@ -233,21 +291,38 @@ class provider implements
     public static function delete_data_for_user(approved_contextlist $contextlist) {
         global $DB;
 
-        // If the user has tool_cohortroles data, then only the User context should be present so get the first context.
-        $contexts = $contextlist->get_contexts();
-        if (count($contexts) == 0) {
+        // When we process user deletions and expiries, we always delete from the user context.
+        // As a result the cohort role assignments would be deleted, which has a knock-on effect with courses
+        // as roles may change and data may be removed earlier than it should be.
+
+        // Remove contexts different from SYSTEM or COURSECAT.
+        $contextids = array_reduce($contextlist->get_contexts(), function($carry, $context) {
+            if ($context->contextlevel == CONTEXT_SYSTEM || $context->contextlevel == CONTEXT_COURSECAT) {
+                $carry[] = $context->id;
+            }
+            return $carry;
+        }, []);
+
+        if (empty($contextids)) {
             return;
         }
-        $context = reset($contexts);
 
-        // Sanity check that context is at the User context level, then get the userid.
-        if ($context->contextlevel !== CONTEXT_USER) {
+        $userid = $contextlist->get_user()->id;
+
+        list($contextsql, $contextparams) =  $DB->get_in_or_equal($contextids, SQL_PARAMS_NAMED);
+        $selectcontext = "contextid {$contextsql}";
+        // Get the cohorts in the specified contexts.
+        $cohortids = $DB->get_fieldset_select('cohort', 'id', $selectcontext, $contextparams);
+
+        if (empty($cohortids)) {
             return;
         }
-        $userid = $context->instanceid;
+
+        list($cohortsql, $cohortparams) =  $DB->get_in_or_equal($cohortids, SQL_PARAMS_NAMED);
+        $selectcohort = "cohortid {$cohortsql} AND userid = :userid";
+        $params = ['userid' => $userid] + $cohortparams;
 
         // Delete the tool_cohortroles records created for the userid.
-        $DB->delete_records('tool_cohortroles', ['userid' => $userid]);
+        $DB->delete_records_select('tool_cohortroles', $selectcohort, $params);
     }
-
 }
index 26f99d3..6399314 100644 (file)
@@ -58,22 +58,36 @@ class tool_cohortroles_privacy_testcase extends \core_privacy\tests\provider_tes
         $this->setUser($user);
         $this->setAdminUser();
 
-        $nocohortroles = 3;
-        $this->setup_test_scenario_data($user->id, $nocohortroles);
+        // Create course category.
+        $coursecategory = $this->getDataGenerator()->create_category();
+        $coursecategoryctx = \context_coursecat::instance($coursecategory->id);
+        $systemctx = context_system::instance();
+        // Create course.
+        $course = $this->getDataGenerator()->create_course();
+        $coursectx = \context_course::instance($course->id);
+
+        $this->setup_test_scenario_data($user->id, $systemctx, 1);
+        $this->setup_test_scenario_data($user->id, $coursecategoryctx, 1, 'Sausage roll 2',
+            'sausageroll2');
+        $this->setup_test_scenario_data($user->id, $coursectx, 1, 'Sausage roll 3',
+            'sausageroll3');
 
         // Test the User's assigned cohortroles matches 3.
         $cohortroles = $DB->get_records('tool_cohortroles', ['userid' => $user->id]);
-        $this->assertCount($nocohortroles, $cohortroles);
+        $this->assertCount(3, $cohortroles);
 
-        // Test the User's retrieved contextlist contains only one context.
+        // Test the User's retrieved contextlist returns only the system and course category context.
         $contextlist = provider::get_contexts_for_userid($user->id);
         $contexts = $contextlist->get_contexts();
-        $this->assertCount(1, $contexts);
-
-        // Test the User's contexts equal the User's own context.
-        $context = reset($contexts);
-        $this->assertEquals(CONTEXT_USER, $context->contextlevel);
-        $this->assertEquals($user->id, $context->instanceid);
+        $this->assertCount(2, $contexts);
+
+        $contextlevels = array_column($contexts, 'contextlevel');
+        $expected = [
+            CONTEXT_SYSTEM,
+            CONTEXT_COURSECAT
+        ];
+        // Test the User's contexts equal the system and course category context.
+        $this->assertEquals($expected, $contextlevels, '', 0, 10, true);
     }
 
     /**
@@ -85,26 +99,45 @@ class tool_cohortroles_privacy_testcase extends \core_privacy\tests\provider_tes
         $this->setUser($user);
         $this->setAdminUser();
 
-        $nocohortroles = 3;
-        $this->setup_test_scenario_data($user->id, $nocohortroles);
-
-        // Test the User's retrieved contextlist contains only one context.
+        // Create course category.
+        $coursecategory = $this->getDataGenerator()->create_category();
+        $coursecategoryctx = \context_coursecat::instance($coursecategory->id);
+        $systemctx = context_system::instance();
+        // Create course.
+        $course = $this->getDataGenerator()->create_course();
+        $coursectx = \context_course::instance($course->id);
+
+        $this->setup_test_scenario_data($user->id, $systemctx, 1);
+        $this->setup_test_scenario_data($user->id, $coursecategoryctx, 1, 'Sausage roll 2',
+            'sausageroll2');
+        $this->setup_test_scenario_data($user->id, $coursectx, 1, 'Sausage roll 3',
+            'sausageroll3');
+
+        // Test the User's retrieved contextlist contains two contexts.
         $contextlist = provider::get_contexts_for_userid($user->id);
         $contexts = $contextlist->get_contexts();
-        $this->assertCount(1, $contexts);
+        $this->assertCount(2, $contexts);
 
-        // Test the User's contexts equal the User's own context.
-        $context = reset($contexts);
-        $this->assertEquals(CONTEXT_USER, $context->contextlevel);
-        $this->assertEquals($user->id, $context->instanceid);
+        // Add a system, course category and course context to the approved context list.
+        $approvedcontextids = [
+            $systemctx->id,
+            $coursecategoryctx->id,
+            $coursectx->id
+        ];
 
         // Retrieve the User's tool_cohortroles data.
-        $approvedcontextlist = new approved_contextlist($user, 'tool_cohortroles', $contextlist->get_contextids());
+        $approvedcontextlist = new approved_contextlist($user, 'tool_cohortroles', $approvedcontextids);
         provider::export_user_data($approvedcontextlist);
 
-        // Test the tool_cohortroles data is exported at the User context level.
-        $writer = writer::with_context($context);
+        // Test the tool_cohortroles data is exported at the system context level.
+        $writer = writer::with_context($systemctx);
+        $this->assertTrue($writer->has_any_data());
+        // Test the tool_cohortroles data is exported at the course category context level.
+        $writer = writer::with_context($coursecategoryctx);
         $this->assertTrue($writer->has_any_data());
+        // Test the tool_cohortroles data is not exported at the course context level.
+        $writer = writer::with_context($coursectx);
+        $this->assertFalse($writer->has_any_data());
     }
 
     /**
@@ -118,29 +151,52 @@ class tool_cohortroles_privacy_testcase extends \core_privacy\tests\provider_tes
         $this->setUser($user);
         $this->setAdminUser();
 
-        $nocohortroles = 4;
-        $this->setup_test_scenario_data($user->id, $nocohortroles);
+        // Create course category.
+        $coursecategory = $this->getDataGenerator()->create_category();
+        $coursecategoryctx = \context_coursecat::instance($coursecategory->id);
+        $systemctx = context_system::instance();
 
-        // Test the User's assigned cohortroles matches 4.
+        $this->setup_test_scenario_data($user->id, $systemctx, 1);
+        $this->setup_test_scenario_data($user->id, $coursecategoryctx, 1, 'Sausage roll 2',
+            'sausageroll2');
+
+        // Test the User's assigned cohortroles matches 2.
         $cohortroles = $DB->get_records('tool_cohortroles', ['userid' => $user->id]);
-        $this->assertCount($nocohortroles, $cohortroles);
+        $this->assertCount(2, $cohortroles);
 
-        // Test the User's retrieved contextlist contains only one context.
+        // Test the User's retrieved contextlist contains two contexts.
         $contextlist = provider::get_contexts_for_userid($user->id);
         $contexts = $contextlist->get_contexts();
-        $this->assertCount(1, $contexts);
+        $this->assertCount(2, $contexts);
 
-        // Test the User's contexts equal the User's own context.
-        $context = reset($contexts);
-        $this->assertEquals(CONTEXT_USER, $context->contextlevel);
-        $this->assertEquals($user->id, $context->instanceid);
+        // Make sure the user data is only being deleted in within the system and course category context.
+        $usercontext = context_user::instance($user->id);
+        // Delete all the User's records in mdl_tool_cohortroles table by the user context.
+        provider::delete_data_for_all_users_in_context($usercontext);
+
+        // Test the cohort roles records in mdl_tool_cohortroles table is still present.
+        $cohortroles = $DB->get_records('tool_cohortroles', ['userid' => $user->id]);
+        $this->assertCount(2, $cohortroles);
+
+        // Delete all the User's records in mdl_tool_cohortroles table by the specified system context.
+        provider::delete_data_for_all_users_in_context($systemctx);
+
+        // The user data in the system context should be deleted.
+        // Test the User's retrieved contextlist contains one context (course category).
+        $contextlist = provider::get_contexts_for_userid($user->id);
+        $contexts = $contextlist->get_contexts();
+        $this->assertCount(1, $contexts);
 
-        // Delete all the User's records in mdl_tool_cohortroles table by the specified User context.
-        provider::delete_data_for_all_users_in_context($context);
+        // Delete all the User's records in mdl_tool_cohortroles table by the specified course category context.
+        provider::delete_data_for_all_users_in_context($coursecategoryctx);
 
         // Test the cohort roles records in mdl_tool_cohortroles table is equals zero.
         $cohortroles = $DB->get_records('tool_cohortroles', ['userid' => $user->id]);
         $this->assertCount(0, $cohortroles);
+
+        $contextlist = provider::get_contexts_for_userid($user->id);
+        $contexts = $contextlist->get_contexts();
+        $this->assertCount(0, $contexts);
     }
 
     /**
@@ -154,24 +210,35 @@ class tool_cohortroles_privacy_testcase extends \core_privacy\tests\provider_tes
         $this->setUser($user);
         $this->setAdminUser();
 
-        $nocohortroles = 4;
-        $this->setup_test_scenario_data($user->id, $nocohortroles);
+        // Create course category.
+        $coursecategory = $this->getDataGenerator()->create_category();
+        $coursecategoryctx = \context_coursecat::instance($coursecategory->id);
+        $systemctx = context_system::instance();
+
+        $this->setup_test_scenario_data($user->id, $systemctx, 1);
+        $this->setup_test_scenario_data($user->id, $coursecategoryctx, 1, 'Sausage roll 2',
+            'sausageroll2');
 
-        // Test the User's assigned cohortroles matches 4.
+        // Test the User's assigned cohortroles matches 2.
         $cohortroles = $DB->get_records('tool_cohortroles', ['userid' => $user->id]);
-        $this->assertCount($nocohortroles, $cohortroles);
+        $this->assertCount(2, $cohortroles);
 
-        // Test the User's retrieved contextlist contains only one context.
+        // Test the User's retrieved contextlist contains two contexts.
         $contextlist = provider::get_contexts_for_userid($user->id);
         $contexts = $contextlist->get_contexts();
-        $this->assertCount(1, $contexts);
+        $this->assertCount(2, $contexts);
 
-        // Test the User's contexts equal the User's own context.
-        $context = reset($contexts);
-        $this->assertEquals(CONTEXT_USER, $context->contextlevel);
-        $this->assertEquals($user->id, $context->instanceid);
+        // Make sure the user data is only being deleted in within the system and the course category contexts.
+        $usercontext = context_user::instance($user->id);
+        // Delete all the User's records in mdl_tool_cohortroles table by the specified approved context list.
+        $approvedcontextlist = new approved_contextlist($user, 'tool_cohortroles', [$usercontext->id]);
+        provider::delete_data_for_user($approvedcontextlist);
 
-        // Delete all the User's records in mdl_tool_cohortroles table by the specified User approved context list.
+        // Test the cohort roles records in mdl_tool_cohortroles table are still present.
+        $cohortroles = $DB->get_records('tool_cohortroles', ['userid' => $user->id]);
+        $this->assertCount(2, $cohortroles);
+
+        // Delete all the User's records in mdl_tool_cohortroles table by the specified approved context list.
         $approvedcontextlist = new approved_contextlist($user, 'tool_cohortroles', $contextlist->get_contextids());
         provider::delete_data_for_user($approvedcontextlist);
 
@@ -190,22 +257,32 @@ class tool_cohortroles_privacy_testcase extends \core_privacy\tests\provider_tes
         $user = $this->getDataGenerator()->create_user();
         $usercontext = context_user::instance($user->id);
 
-        $systemcontext = context_system::instance();
+        // Create course category.
+        $coursecategory = $this->getDataGenerator()->create_category();
+        $coursecategoryctx = \context_coursecat::instance($coursecategory->id);
+        $systemctx = context_system::instance();
 
         $this->setAdminUser();
 
-        $userlist = new \core_privacy\local\request\userlist($systemcontext, $component);
+        $userlist = new \core_privacy\local\request\userlist($systemctx, $component);
         provider::get_users_in_context($userlist);
         $this->assertCount(0, $userlist);
 
-        $nocohortroles = 3;
-        $this->setup_test_scenario_data($user->id, $nocohortroles);
+        $this->setup_test_scenario_data($user->id, $systemctx, 1);
+        $this->setup_test_scenario_data($user->id, $coursecategoryctx, 1, 'Sausage roll 2',
+            'sausageroll2');
 
         // The list of users within the system context should contain user.
         provider::get_users_in_context($userlist);
         $this->assertCount(1, $userlist);
         $this->assertTrue(in_array($user->id, $userlist->get_userids()));
 
+        // The list of users within the course category context should contain user.
+        $userlist = new \core_privacy\local\request\userlist($coursecategoryctx, $component);
+        provider::get_users_in_context($userlist);
+        $this->assertCount(1, $userlist);
+        $this->assertTrue(in_array($user->id, $userlist->get_userids()));
+
         // The list of users within the user context should be empty.
         $userlist2 = new \core_privacy\local\request\userlist($usercontext, $component);
         provider::get_users_in_context($userlist2);
@@ -226,43 +303,44 @@ class tool_cohortroles_privacy_testcase extends \core_privacy\tests\provider_tes
         $user3 = $this->getDataGenerator()->create_user();
         $usercontext3 = context_user::instance($user3->id);
 
-        $systemcontext = context_system::instance();
+        // Create course category.
+        $coursecategory = $this->getDataGenerator()->create_category();
+        $coursecategoryctx = \context_coursecat::instance($coursecategory->id);
+        $systemctx = context_system::instance();
 
         $this->setAdminUser();
 
-        $nocohortroles = 3;
-        $this->setup_test_scenario_data($user1->id, $nocohortroles);
-        $this->setup_test_scenario_data($user2->id, $nocohortroles, 'Sausage roll 2',
+        $this->setup_test_scenario_data($user1->id, $systemctx, 1);
+        $this->setup_test_scenario_data($user2->id, $systemctx, 1, 'Sausage roll 2',
                 'sausageroll2');
-        $this->setup_test_scenario_data($user3->id, $nocohortroles, 'Sausage roll 3',
+        $this->setup_test_scenario_data($user3->id, $coursecategoryctx, 1, 'Sausage roll 3',
                 'sausageroll3');
 
-        $userlist1 = new \core_privacy\local\request\userlist($systemcontext, $component);
+        $userlist1 = new \core_privacy\local\request\userlist($systemctx, $component);
         provider::get_users_in_context($userlist1);
-        $this->assertCount(3, $userlist1);
+        $this->assertCount(2, $userlist1);
         $this->assertTrue(in_array($user1->id, $userlist1->get_userids()));
         $this->assertTrue(in_array($user2->id, $userlist1->get_userids()));
-        $this->assertTrue(in_array($user3->id, $userlist1->get_userids()));
 
         // Convert $userlist1 into an approved_contextlist.
-        $approvedlist1 = new approved_userlist($systemcontext, $component, [$user1->id, $user2->id]);
+        $approvedlist1 = new approved_userlist($systemctx, $component, [$user1->id]);
         // Delete using delete_data_for_user.
         provider::delete_data_for_users($approvedlist1);
 
         // Re-fetch users in systemcontext.
-        $userlist1 = new \core_privacy\local\request\userlist($systemcontext, $component);
+        $userlist1 = new \core_privacy\local\request\userlist($systemctx, $component);
         provider::get_users_in_context($userlist1);
-        // The user data of user1 and user2 in systemcontext should be deleted.
-        // The user data of user3 in systemcontext should be still present.
+        // The user data of user1in systemcontext should be deleted.
+        // The user data of user2 in systemcontext should be still present.
         $this->assertCount(1, $userlist1);
-        $this->assertTrue(in_array($user3->id, $userlist1->get_userids()));
+        $this->assertTrue(in_array($user2->id, $userlist1->get_userids()));
 
         // Convert $userlist1 into an approved_contextlist in the user context.
         $approvedlist2 = new approved_userlist($usercontext3, $component, $userlist1->get_userids());
         // Delete using delete_data_for_user.
         provider::delete_data_for_users($approvedlist2);
         // Re-fetch users in systemcontext.
-        $userlist1 = new \core_privacy\local\request\userlist($systemcontext, $component);
+        $userlist1 = new \core_privacy\local\request\userlist($systemctx, $component);
         provider::get_users_in_context($userlist1);
         // The user data in systemcontext should not be deleted.
         $this->assertCount(1, $userlist1);
@@ -278,12 +356,15 @@ class tool_cohortroles_privacy_testcase extends \core_privacy\tests\provider_tes
      * @throws \core_competency\invalid_persistent_exception
      * @throws coding_exception
      */
-    protected function setup_test_scenario_data($userid, $nocohortroles, $rolename = 'Sausage Roll',
+    protected function setup_test_scenario_data($userid, $context, $nocohortroles, $rolename = 'Sausage Roll',
                                                 $roleshortname = 'sausageroll') {
         $roleid = create_role($rolename, $roleshortname, 'mmmm');
 
+        $result = new \stdClass();
+        $result->contextid = $context->id;
+
         for ($c = 0; $c < $nocohortroles; $c++) {
-            $cohort = $this->getDataGenerator()->create_cohort();
+            $cohort = $this->getDataGenerator()->create_cohort($result);
 
             $params = (object)array(
                 'userid' => $userid,