From 6856da7f094cfccf2b2c020ab861f260332a4144 Mon Sep 17 00:00:00 2001
From: Paul Holden <paulh@moodle.com>
Date: Fri, 20 Oct 2023 09:01:16 +0100
Subject: [PATCH] MDL-79801 tag: fix report entity flagged column/filter query.

The "flag" field isn't a boolean state (1/0), it's a cumulative
count of how many times the tag has been flagged by users. This
should be accounted for to ensure that column aggregation and
filtering performs correctly.
---
 tag/classes/reportbuilder/local/entities/tag.php |  6 +++---
 tag/tests/reportbuilder/datasource/tags_test.php | 10 ++++++----
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/tag/classes/reportbuilder/local/entities/tag.php b/tag/classes/reportbuilder/local/entities/tag.php
index ea93d2a46b6..ebc9395e5cf 100644
--- a/tag/classes/reportbuilder/local/entities/tag.php
+++ b/tag/classes/reportbuilder/local/entities/tag.php
@@ -171,8 +171,8 @@ class tag extends base {
         ))
             ->add_joins($this->get_joins())
             ->set_type(column::TYPE_BOOLEAN)
-            ->add_fields("{$tagalias}.flag")
-            ->set_is_sortable(true)
+            ->add_field("CASE WHEN {$tagalias}.flag > 0 THEN 1 ELSE {$tagalias}.flag END", 'flag')
+            ->set_is_sortable(true, ["{$tagalias}.flag"])
             ->add_callback([format::class, 'boolean_as_text']);
 
         // Time modified.
@@ -224,7 +224,7 @@ class tag extends base {
             'flagged',
             new lang_string('flagged', 'core_tag'),
             $this->get_entity_name(),
-            "{$tagalias}.flag"
+            "CASE WHEN {$tagalias}.flag > 0 THEN 1 ELSE {$tagalias}.flag END"
         ))
             ->add_joins($this->get_joins());
 
diff --git a/tag/tests/reportbuilder/datasource/tags_test.php b/tag/tests/reportbuilder/datasource/tags_test.php
index e727560c013..feceadccbea 100644
--- a/tag/tests/reportbuilder/datasource/tags_test.php
+++ b/tag/tests/reportbuilder/datasource/tags_test.php
@@ -79,6 +79,7 @@ class tags_test extends core_reportbuilder_testcase {
     public function test_datasource_non_default_columns(): void {
         $this->resetAfterTest();
 
+        $this->getDataGenerator()->create_tag(['name' => 'Horses', 'description' => 'Neigh', 'flag' => 2]);
         $course = $this->getDataGenerator()->create_course(['tags' => ['Horses']]);
         $coursecontext = context_course::instance($course->id);
 
@@ -122,8 +123,8 @@ class tags_test extends core_reportbuilder_testcase {
 
         // Tag.
         $this->assertEquals('Horses', $courserow[4]);
-        $this->assertEmpty($courserow[5]);
-        $this->assertEquals('No', $courserow[6]);
+        $this->assertEquals('<div class="text_to_html">Neigh</div>', $courserow[5]);
+        $this->assertEquals('Yes', $courserow[6]);
         $this->assertNotEmpty($courserow[7]);
 
         // Instance.
@@ -184,10 +185,10 @@ class tags_test extends core_reportbuilder_testcase {
                 'tag:standard_operator' => boolean_select::CHECKED,
             ], false],
             'Filter tag flagged' => ['tag:flagged', [
-                'tag:flagged_operator' => boolean_select::NOT_CHECKED,
+                'tag:flagged_operator' => boolean_select::CHECKED,
             ], true],
             'Filter tag flagged (no match)' => ['tag:flagged', [
-                'tag:flagged_operator' => boolean_select::CHECKED,
+                'tag:flagged_operator' => boolean_select::NOT_CHECKED,
             ], false],
             'Filter tag time modified' => ['tag:timemodified', [
                 'tag:timemodified_operator' => date::DATE_RANGE,
@@ -242,6 +243,7 @@ class tags_test extends core_reportbuilder_testcase {
     ): void {
         $this->resetAfterTest();
 
+        $this->getDataGenerator()->create_tag(['name' => 'Horses', 'flag' => 2]);
         $this->getDataGenerator()->create_course(['tags' => ['Horses']]);
 
         /** @var core_reportbuilder_generator $generator */