MDL-65313 core_favourites: component scoped deletion now requires itemid

This now requires itemid, which cannot be null. This is safer. The
context can still be passed in as an optional parameter.

favourites/classes/local/service/component_favourite_service.php

@@ -61,15 +61,17 @@ public function __construct(string $component, favourite_repository_interface $r
 
 
     /**
-     * Delete a collection of favourites by type, and optionally for a given context.
+     * Delete a collection of favourites by type and item, and optionally for a given context.
      *
-     * E.g. delete all favourites of type 'message_conversations' and for a specific CONTEXT_COURSE context.
+     * E.g. delete all favourites of type 'message_conversations' for the conversation '11' and in the CONTEXT_COURSE context.
      *
      * @param string $itemtype the type of the favourited items.
+     * @param int $itemid the id of the item to which the favourites relate
      * @param \context $context the context of the items which were favourited.
      */
-    public function delete_favourites_by_type(string $itemtype, \context $context = null) {
-        $criteria = ['component' => $this->component, 'itemtype' => $itemtype] + ($context ? ['contextid' => $context->id] : []);
+    public function delete_favourites_by_type_and_item(string $itemtype, int $itemid, \context $context = null) {
+        $criteria = ['component' => $this->component, 'itemtype' => $itemtype, 'itemid' => $itemid] +
+            ($context ? ['contextid' => $context->id] : []);
         $this->repo->delete_by($criteria);
     }
 }

favourites/tests/component_favourite_service_test.php

@@ -180,9 +180,9 @@ protected function get_mock_repository(array $mockstore) {
     }
 
     /**
-     * Test confirming the deletion of favourites by type, but with no optional context filter provided.
+     * Test confirming the deletion of favourites by type and item, but with no optional context filter provided.
      */
-    public function test_delete_favourites_by_type() {
+    public function test_delete_favourites_by_type_and_item() {
         list($user1context, $user2context, $course1context, $course2context) = $this->setup_users_and_courses();
 
         // Get a user_favourite_service for each user.
@@ -207,8 +207,11 @@ public function test_delete_favourites_by_type() {
         // Get a component_favourite_service to perform the type based deletion.
         $service = new \core_favourites\local\service\component_favourite_service('core_course', $repo);
 
-        // Delete all 'course' type favourites (for all users at ANY context).
-        $service->delete_favourites_by_type('course');
+        // Delete all 'course' type favourites (for all users who have favourited course1).
+        $service->delete_favourites_by_type_and_item('course', $course1context->instanceid);
+
+        // Delete all 'course' type favourites (for all users who have favourited course2).
+        $service->delete_favourites_by_type_and_item('course', $course2context->instanceid);
 
         // Verify the favourites don't exist.
         $this->assertFalse($repo->exists($fav1->id));
@@ -221,13 +224,13 @@ public function test_delete_favourites_by_type() {
         $this->assertTrue($repo->exists($fav6->id));
 
         // Try to delete favourites for a type which we know doesn't exist. Verify no exception.
-        $this->assertNull($service->delete_favourites_by_type('course'));
+        $this->assertNull($service->delete_favourites_by_type_and_item('course', $course1context->instanceid));
     }
 
     /**
-     * Test confirming the deletion of favourites by type and with the optional context filter provided.
+     * Test confirming the deletion of favourites by type and item and with the optional context filter provided.
      */
-    public function test_delete_favourites_by_type_with_context() {
+    public function test_delete_favourites_by_type_and_item_with_context() {
         list($user1context, $user2context, $course1context, $course2context) = $this->setup_users_and_courses();
 
         // Get a user_favourite_service for each user.
@@ -249,11 +252,17 @@ public function test_delete_favourites_by_type_with_context() {
         $fav5 = $user2service->create_favourite('core_user', 'course', $course1context->instanceid, $course1context);
         $fav6 = $user2service->create_favourite('core_course', 'whatnow', $course1context->instanceid, $course1context);
 
+        // Favourite the courses again, but this time in another context.
+        $fav7 = $user1service->create_favourite('core_course', 'course', $course1context->instanceid, context_system::instance());
+        $fav8 = $user2service->create_favourite('core_course', 'course', $course1context->instanceid, context_system::instance());
+        $fav9 = $user1service->create_favourite('core_course', 'course', $course2context->instanceid, context_system::instance());
+        $fav10 = $user2service->create_favourite('core_course', 'course', $course2context->instanceid, context_system::instance());
+
         // Get a component_favourite_service to perform the type based deletion.
         $service = new \core_favourites\local\service\component_favourite_service('core_course', $repo);
 
         // Delete all 'course' type favourites (for all users at ONLY the course 1 context).
-        $service->delete_favourites_by_type('course', $course1context);
+        $service->delete_favourites_by_type_and_item('course', $course1context->instanceid, $course1context);
 
         // Verify the favourites for course 1 context don't exist.
         $this->assertFalse($repo->exists($fav1->id));
@@ -267,7 +276,13 @@ public function test_delete_favourites_by_type_with_context() {
         $this->assertTrue($repo->exists($fav5->id));
         $this->assertTrue($repo->exists($fav6->id));
 
+        // Verify the course favourite at the system context are unaffected.
+        $this->assertTrue($repo->exists($fav7->id));
+        $this->assertTrue($repo->exists($fav8->id));
+        $this->assertTrue($repo->exists($fav9->id));
+        $this->assertTrue($repo->exists($fav10->id));
+
         // Try to delete favourites for a type which we know doesn't exist. Verify no exception.
-        $this->assertNull($service->delete_favourites_by_type('course', $course1context));
+        $this->assertNull($service->delete_favourites_by_type_and_item('course', $course1context->instanceid, $course1context));
     }
 }

message/classes/api.php

@@ -3338,7 +3338,7 @@ public static function delete_all_conversation_data(int $conversationid) {
 
         // Delete all favourite records for all users relating to this conversation.
         $service = \core_favourites\service_factory::get_service_for_component('core_message');
-        $service->delete_favourites_by_type('message_conversations', $convcontext);
+        $service->delete_favourites_by_type_and_item('message_conversations', $conversationid, $convcontext);
     }
 
     /**