WR422914 Peer review fixes.

Added following fixes for the peer review from Dale Davis:

* Update scope of __contruct classes as using Singleton
* Updated getter function names
* Re-added trailing comma to task file
* Removed additional ci.yml file
* Removed commented code
* Updated assign and quiz objects to use stdClass for dynamic properties
* Updated lib.php to use frankenstyle prefix

classes/report_base.php

@@ -39,9 +39,9 @@ abstract class report_base {
     /**
      * Contructor.
      */
-    public function __construct() {
-        $this->get_required_js();
-        $this->get_required_css();
+    private function __construct() {
+        $this->add_required_js();
+        $this->add_required_css();
     }
 
     /**
@@ -97,13 +97,13 @@ public function has_access(): bool {
      * Set up the required JS in the global $PAGE object.
      * @return void
      */
-    protected function get_required_js() {
+    protected function add_required_js() {
     }
 
     /**
      * Set up the required CSS in the global $PAGE object.
      * @return void
      */
-    protected function get_required_css() {
+    protected function add_required_css() {
     }
 }

classes/source_base.php

@@ -46,9 +46,9 @@ abstract class source_base {
     /**
      * Constructor.
      */
-    public function __construct() {
-        $this->get_required_js();
-        $this->get_required_css();
+    private function __construct() {
+        $this->add_required_js();
+        $this->add_required_css();
     }
 
     /**
@@ -121,14 +121,14 @@ abstract public function get_name(): string;
      * Set up the required JS in the global $PAGE object.
      * @return void
      */
-    protected function get_required_js() {
+    protected function add_required_js() {
     }
 
     /**
      * Set up the required CSS in the global $PAGE object.
      * @return void
      */
-    protected function get_required_css() {
+    protected function add_required_css() {
     }
 
     /**

lib.php

@@ -47,7 +47,7 @@ function local_assessfreq_extend_navigation_course(navigation_node $navigation,
 /**
  * Get all of the subplugin reports that are enabled and instantiate the class.
  *
- * @param bool $ignoreenabled
+ * @param bool $ignoreenabled Ignore the is_enabled check.
  * @return array
  */
 function local_assessfreq_get_reports(bool $ignoreenabled = false): array {
@@ -70,7 +70,8 @@ function local_assessfreq_get_reports(bool $ignoreenabled = false): array {
  * Get all of the subplugin sources that are enabled and instantiate the class.
  *
  * @param bool $ignoreenabled
- * @param string $requiredmethod
+ * @param string $requiredmethod    Only load sources that contain the required method.
+ *                                  Usesful for calling specific sources for a report.
  * @return array
  */
 function local_assessfreq_get_sources(bool $ignoreenabled = false, $requiredmethod = ''): array {
@@ -159,7 +160,8 @@ function local_assessfreq_get_years($preference): array {
  * This is based on which sources have been enabled.
  *
  * @param array $preferences
- * @param string $requiredmethod
+ * @param string $requiredmethod    Only load sources that contain the required method.
+ *                                  Usesful for calling specific sources for a report.
  * @return array $modules The enabled modules.
  * @throws coding_exception
  */

report/activities_in_progress/classes/report.php

@@ -117,7 +117,7 @@ public function get_contents(): string {
     /**
      * {@inheritDoc}
      */
-    protected function get_required_js(): void {
+    protected function add_required_js(): void {
         global $PAGE;
 
         $PAGE->requires->js_call_amd(
@@ -130,7 +130,7 @@ protected function get_required_js(): void {
     /**
      * {@inheritDoc}
      */
-    protected function get_required_css(): void {
+    protected function add_required_css(): void {
         global $PAGE;
 
         $PAGE->requires->css('/local/assessfreq/report/activities_in_progress/styles.css');

report/activity_dashboard/classes/report.php

@@ -86,7 +86,7 @@ public function get_contents(): string {
     /**
      * {@inheritDoc}
      */
-    protected function get_required_js(): void {
+    protected function add_required_js(): void {
         global $PAGE;
 
         $PAGE->requires->js_call_amd(
@@ -99,7 +99,7 @@ protected function get_required_js(): void {
     /**
      * {@inheritDoc}
      */
-    protected function get_required_css(): void {
+    protected function add_required_css(): void {
         global $PAGE;
 
         $PAGE->requires->css('/local/assessfreq/report/activity_dashboard/styles.css');

report/heatmap/classes/report.php

@@ -85,7 +85,7 @@ public function get_contents(): string {
     /**
      * {@inheritDoc}
      */
-    protected function get_required_js(): void {
+    protected function add_required_js(): void {
         global $PAGE;
 
         $PAGE->requires->js_call_amd('assessfreqreport_heatmap/heatmap', 'init', [$PAGE->course->id]);
@@ -94,7 +94,7 @@ protected function get_required_js(): void {
     /**
      * {@inheritDoc}
      */
-    protected function get_required_css(): void {
+    protected function add_required_css(): void {
         global $PAGE;
         // The CSS for the heatmap is based on plugin config. As such this needs to be in-line.
         $PAGE->requires->css('/local/assessfreq/report/heatmap/dynamic-styles.php');

report/student_search/classes/output/user_table.php

@@ -29,7 +29,7 @@
 
 require_once($CFG->libdir . '/tablelib.php');
 
-use assessfreqsource_quiz\Source;
+use assessfreqsource_quiz;
 use coding_exception;
 use html_writer;
 use local_assessfreq\frequency;
@@ -474,7 +474,7 @@ public function query_db($pagesize, $useinitialsbar = false): void {
         $capabilities = $frequency->get_module_capabilities('quiz');
 
         // Get the quizzes that we want users for.
-        $quizsource = new Source();
+        $quizsource = assessfreqsource_quiz\source::get_instance();
         $allquizzes = $quizsource->get_quiz_summaries($this->now, $this->hoursahead, $this->hoursbehind, false);
 
         $inprogressquizzes = $allquizzes['inprogress'];

report/student_search/classes/report.php

@@ -86,7 +86,7 @@ public function get_contents(): string {
     /**
      * {@inheritDoc}
      */
-    protected function get_required_js(): void {
+    protected function add_required_js(): void {
         global $PAGE;
 
         $PAGE->requires->js_call_amd(
@@ -99,7 +99,7 @@ protected function get_required_js(): void {
     /**
      * {@inheritDoc}
      */
-    protected function get_required_css(): void {
+    protected function add_required_css(): void {
         global $PAGE;
 
         $PAGE->requires->css('/local/assessfreq/report/student_search/styles.css');

report/summary_graphs/classes/report.php

@@ -102,7 +102,7 @@ public function get_contents(): string {
     /**
      * {@inheritDoc}
      */
-    protected function get_required_js(): void {
+    protected function add_required_js(): void {
         global $PAGE;
 
         $PAGE->requires->js_call_amd('assessfreqreport_summary_graphs/summary_graphs', 'init');

source/quiz/classes/task/quiz_tracking.php

@@ -23,7 +23,6 @@
  */
 namespace assessfreqsource_quiz\task;
 
-use assessfreqsource_quiz\source;
 use core\task\scheduled_task;
 use local_assessfreq\frequency;
 use stdClass;
@@ -66,7 +65,7 @@ public function execute(): void {
             $actionstart = 1594788000;
         }
 
-        $source = new source();
+        $source = \assessfreqsource_quiz\source::get_instance();
         $frequency = new frequency();
 
         $quizzes = $source->get_tracked_quizzes_with_overrides($actionstart);

templates/index.mustache

@@ -25,9 +25,9 @@
 
 <div id="local-assessfreq-index">
     <!-- Course select interface -->
-    <p>
+    <div class="mb-1">
         <select id="local-assessfreq-course-filter" class="icon-no-margin nav-course-autocomplete"></select>
-    </p>
+    </div>
     <!-- Tabs -->
     {{> local_assessfreq/tabs}}
 </div>

tests/quiz_test.php

@@ -601,7 +601,7 @@ public function setup_mock_quiz_data(): array {
      * Test getting quiz override info.
      */
     public function test_get_quiz_override_info(): void {
-        $quizdata = new \assessfreqsource_quiz\source();
+        $quizdata = \assessfreqsource_quiz\source::get_instance();
         $context = \context_module::instance($this->quiz1->cmid);
 
         // We're testing a private method, so we need to setup reflector magic.
@@ -620,7 +620,7 @@ public function test_get_quiz_override_info(): void {
      */
     public function test_get_quiz_questions(): void {
         global $DB;
-        $quizdata = new \assessfreqsource_quiz\source();
+        $quizdata = \assessfreqsource_quiz\source::get_instance();
 
         [$course, $cm] = get_course_and_cm_from_instance($this->quiz1->id, 'quiz');
 
@@ -645,7 +645,7 @@ public function test_get_quiz_questions(): void {
      */
     public function test_get_quiz_data(): void {
 
-        $quizdata = new \assessfreqsource_quiz\source();
+        $quizdata = \assessfreqsource_quiz\source::get_instance();
         $result = $quizdata->get_quiz_data($this->quiz1);
 
         $this->assertEquals('5 July 2020, 9:00 AM', $result->earlyopen);
@@ -661,7 +661,7 @@ public function test_get_quiz_data(): void {
      * Test quiz override tracking.
      */
     public function test_get_tracked_overrides(): void {
-        $quizdata = new \assessfreqsource_quiz\source();
+        $quizdata = \assessfreqsource_quiz\source::get_instance();
         $method = new \ReflectionMethod('\assessfreqsource_quiz\Source', 'get_tracked_overrides');
         $method->setAccessible(true); // Allow accessing of private method.
 
@@ -680,7 +680,7 @@ public function test_get_tracked_overrides(): void {
      * Test quiz tracking.
      */
     public function test_get_tracked_quizzes(): void {
-        $quizdata = new \assessfreqsource_quiz\source();
+        $quizdata = \assessfreqsource_quiz\source::get_instance();
         $method = new \ReflectionMethod('\assessfreqsource_quiz\Source', 'get_tracked_quizzes');
         $method->setAccessible(true); // Allow accessing of private method.
 
@@ -713,7 +713,7 @@ public function test_get_tracked_quizzes_with_overrides(): void {
 
         $DB->insert_record('quiz_overrides', $override);
 
-        $quizdata = new \assessfreqsource_quiz\source();
+        $quizdata = \assessfreqsource_quiz\source::get_instance();
         $method = new \ReflectionMethod('\assessfreqsource_quiz\Source', 'get_tracked_quizzes_with_overrides');
         $method->setAccessible(true); // Allow accessing of private method.
 
@@ -756,7 +756,7 @@ public function test_local_assessfreq_get_loggedin_users(): void {
      */
     public function test_get_quiz_attempts(): void {
 
-        $quizdata = new \assessfreqsource_quiz\source();
+        $quizdata = \assessfreqsource_quiz\source::get_instance();
 
         $method = new \ReflectionMethod('\assessfreqsource_quiz\Source', 'get_quiz_attempts');
         $method->setAccessible(true); // Allow accessing of private method.
@@ -795,7 +795,7 @@ public function test_get_quiz_tracking(): void {
         $now = 1594788000;
         $this->setup_quiz_tracking($now, $this->quiz1->id);
 
-        $quizdata = new \assessfreqsource_quiz\source();
+        $quizdata = \assessfreqsource_quiz\source::get_instance();
         $method = new \ReflectionMethod('\assessfreqsource_quiz\Source', 'get_tracking');
         $method->setAccessible(true); // Allow accessing of private method.
 
@@ -816,7 +816,7 @@ public function test_get_inprogress_counts(): void {
         $this->setup_quiz_tracking($now, $this->quiz3->id);
         $this->setup_quiz_tracking($now, $this->quiz4->id);
 
-        $quizdata = new \assessfreqsource_quiz\source();
+        $quizdata = \assessfreqsource_quiz\source::get_instance();
         $result = $quizdata->get_inprogress_count($now, 0, 0, false);
 
         $this->assertEquals(3, $result['assessments']);
@@ -827,7 +827,7 @@ public function test_get_inprogress_counts(): void {
      * Test getting finished quizzes.
      */
     public function test_get_quizzes_finished(): void {
-        $quizdata = new \assessfreqsource_quiz\source();
+        $quizdata = \assessfreqsource_quiz\source::get_instance();
 
         $now = 1594788000;
         $result = $quizdata->get_quiz_summaries($now, HOURSECS, HOURSECS);
@@ -843,7 +843,7 @@ public function test_get_quizzes_inprogress(): void {
         $this->setup_quiz_tracking($now, $this->quiz3->id);
         $this->setup_quiz_tracking($now, $this->quiz4->id);
 
-        $quizdata = new \assessfreqsource_quiz\source();
+        $quizdata = \assessfreqsource_quiz\source::get_instance();
         $result = $quizdata->get_quiz_summaries($now, HOURSECS, HOURSECS);
 
         $this->assertCount(3, $result['inprogress']);
@@ -857,7 +857,7 @@ public function test_get_quizzes_inprogress(): void {
      * Test getting upcomming quizzes.
      */
     public function test_get_quizzes_upcomming(): void {
-        $quizdata = new \assessfreqsource_quiz\source();
+        $quizdata = \assessfreqsource_quiz\source::get_instance();
 
         $now = 1594780800;
         $result = $quizdata->get_quiz_summaries($now, HOURSECS, HOURSECS);

version.php

@@ -29,5 +29,5 @@
 $plugin->release = 2025010300;
 $plugin->version = 2025010300;
 $plugin->requires = 2023100900; // Requires 4.3.
-$plugin->supported = [403, 405];
+$plugin->supported = [403, 404];
 $plugin->maturity = MATURITY_STABLE;