From 0ead4cefd20eec243e2f399ca0092c62fff1aba2 Mon Sep 17 00:00:00 2001
From: Paul Holden <paulh@moodle.com>
Date: Wed, 23 Feb 2022 18:08:51 +0000
Subject: [PATCH] MDL-73983 reportbuilder: fix pre-defined action title
 attribute.

Since switching to report actions being displayed via action menus
in 48a6e927, pre-defined action titles weren't displayed.
---
 reportbuilder/classes/local/report/action.php | 14 ++++--
 .../classes/table/system_report_table.php     |  1 +
 .../tests/local/report/action_test.php        | 46 +++++++++++++++++++
 3 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/reportbuilder/classes/local/report/action.php b/reportbuilder/classes/local/report/action.php
index 9519612f066..526325f9905 100644
--- a/reportbuilder/classes/local/report/action.php
+++ b/reportbuilder/classes/local/report/action.php
@@ -94,9 +94,8 @@ final class action {
     }
 
     /**
-     * Return renderer action icon suitable for output
-     *
-     * @uses core_renderer::action_icon()
+     * Return rendered action link suitable for output, or null if the action cannot be displayed (because one of it's callbacks
+     * returned false, {@see add_callback})
      *
      * @param stdClass $row
      * @return string|null
@@ -130,7 +129,14 @@ final class action {
             $this->attributes['data-popup-action'] = json_encode(new popup_action('click', $url));
         }
 
-        return $OUTPUT->action_link($url, $this->title, null, self::replace_placeholders($this->attributes, $row), $this->icon);
+        // Interpolate any placeholders with correct values.
+        $attributes = self::replace_placeholders($this->attributes, $row);
+
+        // Ensure title attribute isn't duplicated.
+        $title = $attributes['title'];
+        unset($attributes['title']);
+
+        return $OUTPUT->action_link($url, $title, null, $attributes, $this->icon);
     }
 
     /**
diff --git a/reportbuilder/classes/table/system_report_table.php b/reportbuilder/classes/table/system_report_table.php
index 2513defcdc5..0e35ac8b19b 100644
--- a/reportbuilder/classes/table/system_report_table.php
+++ b/reportbuilder/classes/table/system_report_table.php
@@ -224,6 +224,7 @@ class system_report_table extends base_report_table {
         $menu = new action_menu();
         $menu->set_menu_trigger($OUTPUT->pix_icon('a/setting', get_string('actions', 'core_reportbuilder')));
         foreach ($this->report->get_actions() as $action) {
+            // Ensure the action link can be displayed for the current row.
             $actionlink = $action->get_action_link($row);
             if ($actionlink) {
                 $menu->add($actionlink);
diff --git a/reportbuilder/tests/local/report/action_test.php b/reportbuilder/tests/local/report/action_test.php
index c6871d4819a..f1d0969e082 100644
--- a/reportbuilder/tests/local/report/action_test.php
+++ b/reportbuilder/tests/local/report/action_test.php
@@ -19,6 +19,7 @@ declare(strict_types=1);
 namespace core_reportbuilder\local\report;
 
 use advanced_testcase;
+use lang_string;
 use moodle_url;
 use pix_icon;
 use stdClass;
@@ -57,6 +58,51 @@ class action_test extends advanced_testcase {
         $this->assertNull($action->get_action_link(new stdClass()));
     }
 
+    /**
+     * Data provider for {@see test_action_title}
+     *
+     * @return array[]
+     */
+    public function action_title_provider(): array {
+        $title = new lang_string('yes');
+        return [
+            'Specified via constructor' => ['', [], $title],
+            'Specified via pix icon' => [(string) $title],
+            'Specified via attributes' => ['', ['title' => $title]],
+            'Specified via attributes placeholder' => ['', ['title' => ':title'], null, ['title' => $title]],
+        ];
+    }
+
+    /**
+     * Test action title is correct
+     *
+     * @param string $pixiconalt
+     * @param array $attributes
+     * @param lang_string|null $title
+     * @param array $row
+     *
+     * @dataProvider action_title_provider
+     */
+    public function test_action_title(
+        string $pixiconalt,
+        array $attributes = [],
+        ?lang_string $title = null,
+        array $row = []
+    ): void {
+
+        $action = new action(
+            new moodle_url('#'),
+            new pix_icon('t/edit', $pixiconalt),
+            $attributes,
+            false,
+            $title
+        );
+
+        // Assert correct title appears inside action link, after the icon.
+        $actionlink = $action->get_action_link((object) $row);
+        $this->assertStringEndsWith('</i>Yes</a>', $actionlink);
+    }
+
     /**
      * Test that action link URL parameters have placeholders replaced
      */