MDL-68483 contentbank: improve search API
authorSara Arjona <sara@moodle.com>
Tue, 28 Apr 2020 16:27:16 +0000 (18:27 +0200)
committerSara Arjona <sara@moodle.com>
Wed, 13 May 2020 11:31:30 +0000 (13:31 +0200)
These improvements were suggested by Eloy when MDL-67797 was integrated.

contentbank/classes/contentbank.php
contentbank/classes/output/bankcontent.php
contentbank/index.php
contentbank/tests/contentbank_test.php

index e34dfa2..197a36c 100644 (file)
@@ -42,7 +42,7 @@ class contentbank {
      *
      * @return string[] Array of contentbank contenttypes.
      */
-    private function get_enabled_content_types(): array {
+    public function get_enabled_content_types(): array {
         $enabledtypes = \core\plugininfo\contenttype::get_enabled_plugins();
         $types = [];
         foreach ($enabledtypes as $name) {
@@ -159,20 +159,28 @@ class contentbank {
      * Find the contents with %$search% in the contextid defined.
      * If contextid and search are empty, all contents are returned.
      * In all the cases, only the contents for the enabled contentbank-type plugins are returned.
+     * No content-type permissions are validated here. It is the caller responsability to check that the user can access to them.
+     * The only validation done here is, for each content, a call to the method $content->is_view_allowed().
      *
      * @param  string|null $search Optional string to search (for now it will search only into the name).
      * @param  int $contextid Optional contextid to search.
+     * @param  array $contenttypenames Optional array with the list of content-type names to search.
      * @return array The contents for the enabled contentbank-type plugins having $search as name and placed in $contextid.
      */
-    public function search_contents(?string $search = null, ?int $contextid = 0): array {
+    public function search_contents(?string $search = null, ?int $contextid = 0, ?array $contenttypenames = null): array {
         global $DB;
 
         $contents = [];
 
         // Get only contents for enabled content-type plugins.
-        $contenttypes = array_map(function($contenttypename) {
-            return "contenttype_$contenttypename";
-        }, $this->get_enabled_content_types());
+        $contenttypes = [];
+        $enabledcontenttypes = $this->get_enabled_content_types();
+        foreach ($enabledcontenttypes as $contenttypename) {
+            if (empty($contenttypenames) || in_array($contenttypename, $contenttypenames)) {
+                $contenttypes[] = "contenttype_$contenttypename";
+            }
+        }
+
         if (empty($contenttypes)) {
             // Early return if there are no content-type plugins enabled.
             return $contents;
index fe76d0c..471266e 100644 (file)
@@ -80,18 +80,14 @@ class bankcontent implements renderable, templatable {
         $contentdata = array();
         foreach ($this->contents as $content) {
             $record = $content->get_content();
-            $managerclass = $content->get_content_type().'\\contenttype';
-            if (class_exists($managerclass)) {
-                $manager = new $managerclass($this->context);
-                if ($manager->can_access()) {
-                    $name = $content->get_name();
-                    $contentdata[] = array(
-                        'name' => $name,
-                        'link' => $manager->get_view_url($record),
-                        'icon' => $manager->get_icon($name)
-                    );
-                }
-            }
+            $contenttypeclass = $content->get_content_type().'\\contenttype';
+            $contenttype = new $contenttypeclass($this->context);
+            $name = $content->get_name();
+            $contentdata[] = array(
+                'name' => $name,
+                'link' => $contenttype->get_view_url($record),
+                'icon' => $contenttype->get_icon($name)
+            );
         }
         $data->contents = $contentdata;
         $data->tools = $this->toolbar;
index bbf9786..f4f101b 100644 (file)
@@ -46,9 +46,19 @@ $PAGE->set_title($title);
 $PAGE->set_heading($title);
 $PAGE->set_pagetype('contenbank');
 
-// Get all contents managed by active plugins to render.
+// Get all contents managed by active plugins where the user has permission to render them.
 $cb = new \core_contentbank\contentbank();
-$foldercontents = $cb->search_contents($search, $contextid);
+$contenttypes = [];
+$enabledcontenttypes = $cb->get_enabled_content_types();
+foreach ($enabledcontenttypes as $contenttypename) {
+    $contenttypeclass = "\\contenttype_$contenttypename\\contenttype";
+    $contenttype = new $contenttypeclass($context);
+    if ($contenttype->can_access()) {
+        $contenttypes[] = $contenttypename;
+    }
+}
+
+$foldercontents = $cb->search_contents($search, $contextid, $contenttypes);
 
 // Get the toolbar ready.
 $toolbar = array ();
index e420456..90893aa 100644 (file)
@@ -185,7 +185,8 @@ class core_contentbank_testcase extends advanced_testcase {
      * @param  int $expectedresult Expected result.
      * @param  array $contexts List of contexts where to create content.
      */
-    public function test_search_contents(?string $search, string $where, int $expectedresult, array $contexts = []): void {
+    public function test_search_contents(?string $search, string $where, int $expectedresult, array $contexts = [],
+            array $contenttypes = null): void {
         global $DB;
 
         $this->resetAfterTest();
@@ -219,7 +220,7 @@ class core_contentbank_testcase extends advanced_testcase {
 
         // Search for some content.
         $cb = new \core_contentbank\contentbank();
-        $contents = $cb->search_contents($search, $contextid);
+        $contents = $cb->search_contents($search, $contextid, $contenttypes);
 
         $this->assertCount($expectedresult, $contents);
         if (!empty($contents) && !empty($search)) {
@@ -321,6 +322,13 @@ class core_contentbank_testcase extends advanced_testcase {
                 0,
                 []
             ],
+            'Search with unexisting content-type' => [
+                null,
+                'course',
+                0,
+                ['system', 'category', 'course'],
+                ['contenttype_unexisting'],
+            ],
         ];
     }