Merge branch 'MDL-68636-master' of git://github.com/aanabit/moodle
authorAdrian Greeve <abgreeve@gmail.com>
Mon, 18 May 2020 00:21:53 +0000 (08:21 +0800)
committerAdrian Greeve <abgreeve@gmail.com>
Mon, 18 May 2020 00:21:53 +0000 (08:21 +0800)
12 files changed:
contentbank/classes/contenttype.php
contentbank/classes/output/bankcontent.php
contentbank/contenttype/h5p/classes/contenttype.php
contentbank/contenttype/h5p/tests/behat/admin_upload_content.feature
contentbank/contenttype/h5p/tests/contenttype_h5p_test.php
contentbank/templates/bankcontent.mustache
contentbank/tests/behat/delete_content.feature
contentbank/tests/contenttype_test.php
contentbank/tests/fixtures/testable_contenttype.php
contentbank/upload.php
contentbank/view.php
h5p/classes/api.php

index 7f7fbf5..79d658b 100644 (file)
@@ -159,37 +159,34 @@ abstract class contenttype {
     /**
      * Returns the URL where the content will be visualized.
      *
-     * @param stdClass $record  The content to be displayed.
+     * @param  content $content The content to be displayed.
      * @return string           URL where to visualize the given content.
      */
-    public function get_view_url(\stdClass $record): string {
-        return new moodle_url('/contentbank/view.php', ['id' => $record->id]);
+    public function get_view_url(content $content): string {
+        return new moodle_url('/contentbank/view.php', ['id' => $content->get_id()]);
     }
 
     /**
      * Returns the HTML content to add to view.php visualizer.
      *
-     * @param stdClass $record  The content to be displayed.
+     * @param  content $content The content to be displayed.
      * @return string           HTML code to include in view.php.
      */
-    public function get_view_content(\stdClass $record): string {
+    public function get_view_content(content $content): string {
         // Trigger an event for viewing this content.
         $event = contentbank_content_viewed::create_from_record($record);
         $event->trigger();
-
-        // Main contenttype class can visualize the content, but plugins could overwrite visualization.
-        return '';
     }
 
     /**
      * Returns the HTML code to render the icon for content bank contents.
      *
-     * @param string $contentname   The contentname to add as alt value to the icon.
+     * @param  content $content The content to be displayed.
      * @return string               HTML code to render the icon
      */
-    public function get_icon(string $contentname): string {
+    public function get_icon(content $content): string {
         global $OUTPUT;
-        return $OUTPUT->pix_icon('f/unknown-64', $contentname, 'moodle', ['class' => 'iconsize-big']);
+        return $OUTPUT->image_url('f/unknown-64', 'moodle')->out(false);
     }
 
     /**
index 471266e..2ea5c4d 100644 (file)
@@ -85,8 +85,8 @@ class bankcontent implements renderable, templatable {
             $name = $content->get_name();
             $contentdata[] = array(
                 'name' => $name,
-                'link' => $contenttype->get_view_url($record),
-                'icon' => $contenttype->get_icon($name)
+                'link' => $contenttype->get_view_url($content),
+                'icon' => $contenttype->get_icon($content)
             );
         }
         $data->contents = $contentdata;
index 806205d..d48941d 100644 (file)
@@ -25,7 +25,6 @@
 namespace contenttype_h5p;
 
 use core\event\contentbank_content_viewed;
-use stdClass;
 use html_writer;
 
 /**
@@ -57,15 +56,14 @@ class contenttype extends \core_contentbank\contenttype {
     /**
      * Returns the HTML content to add to view.php visualizer.
      *
-     * @param stdClass $record  Th content to be displayed.
+     * @param  content $content The content to be displayed.
      * @return string            HTML code to include in view.php.
      */
-    public function get_view_content(\stdClass $record): string {
+    public function get_view_content(\core_contentbank\content $content): string {
         // Trigger an event for viewing this content.
-        $event = contentbank_content_viewed::create_from_record($record);
+        $event = contentbank_content_viewed::create_from_record($content->get_content());
         $event->trigger();
 
-        $content = new content($record);
         $fileurl = $content->get_file_url();
         $html = html_writer::tag('h2', $content->get_name());
         $html .= \core_h5p\player::display($fileurl, new \stdClass(), true);
@@ -75,12 +73,32 @@ class contenttype extends \core_contentbank\contenttype {
     /**
      * Returns the HTML code to render the icon for H5P content types.
      *
-     * @param string $contentname   The contentname to add as alt value to the icon.
+     * @param  content $content The content to be displayed.
      * @return string            HTML code to render the icon
      */
-    public function get_icon(string $contentname): string {
-        global $OUTPUT;
-        return $OUTPUT->pix_icon('f/h5p-64', $contentname, 'moodle', ['class' => 'iconsize-big']);
+    public function get_icon(\core_contentbank\content $content): string {
+        global $OUTPUT, $DB;
+
+        $iconurl = $OUTPUT->image_url('f/h5p-64', 'moodle')->out(false);
+        $file = $content->get_file();
+        if (!empty($file)) {
+            $h5p = \core_h5p\api::get_content_from_pathnamehash($file->get_pathnamehash());
+            if (!empty($h5p)) {
+                \core_h5p\local\library\autoloader::register();
+                if ($h5plib = $DB->get_record('h5p_libraries', ['id' => $h5p->mainlibraryid])) {
+                    $h5pfilestorage = new \core_h5p\file_storage();
+                    $h5picon = $h5pfilestorage->get_icon_url(
+                            $h5plib->id,
+                            $h5plib->machinename,
+                            $h5plib->majorversion,
+                            $h5plib->minorversion);
+                    if (!empty($h5picon)) {
+                        $iconurl = $h5picon;
+                    }
+                }
+            }
+        }
+        return $iconurl;
     }
 
     /**
index ad9c5c5..8639d35 100644 (file)
@@ -34,7 +34,6 @@ Feature: H5P file upload to content bank for admins
     And I click on "Select this file" "button"
     And I click on "Save changes" "button"
     And I wait until the page is ready
-    And I click on "filltheblanks.h5p" "link"
     And I switch to "h5p-player" class iframe
     And I switch to "h5p-iframe" class iframe
     Then I should see "Of which countries"
index 3b9e3e3..c708516 100644 (file)
@@ -105,4 +105,46 @@ class contenttype_h5p_contenttype_plugin_testcase extends advanced_testcase {
         $this->assertFalse($coursetype->can_upload());
         $this->assertFalse($systemtype->can_upload());
     }
+
+    /**
+     * Tests get_icon result.
+     *
+     * @covers ::get_icon
+     */
+    public function test_get_icon() {
+        global $CFG;
+
+        $this->resetAfterTest();
+        $systemcontext = context_system::instance();
+        $this->setAdminUser();
+        $contenttype = new contenttype_h5p\contenttype($systemcontext);
+
+        // Add an H5P fill the blanks file to the content bank.
+        $filepath = $CFG->dirroot . '/h5p/tests/fixtures/filltheblanks.h5p';
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_contentbank');
+        $contents = $generator->generate_contentbank_data('contenttype_h5p', 1, 0, $systemcontext, true, $filepath);
+        $filltheblanks = array_shift($contents);
+
+        // Add an H5P find the words file to the content bank.
+        $filepath = $CFG->dirroot . '/h5p/tests/fixtures/find-the-words.h5p';
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_contentbank');
+        $contents = $generator->generate_contentbank_data('contenttype_h5p', 1, 0, $systemcontext, true, $filepath);
+        $findethewords = array_shift($contents);
+
+        // Check before deploying the icon for both contents is the same: default one.
+        // Because we don't know specific H5P content type yet.
+        $defaulticon = $contenttype->get_icon($filltheblanks);
+        $this->assertEquals($defaulticon, $contenttype->get_icon($findethewords));
+        $this->assertContains('h5p', $defaulticon);
+
+        // Deploy one of the contents though the player to create the H5P DB entries and know specific content type.
+        $h5pplayer = new \core_h5p\player($findethewords->get_file_url(), new \stdClass(), true);
+        $h5pplayer->add_assets_to_page();
+        $h5pplayer->output();
+
+        // Once the H5P has been deployed, we know the specific H5P content type, so the icon returned is not default one.
+        $findicon = $contenttype->get_icon($findethewords);
+        $this->assertNotEquals($defaulticon, $findicon);
+        $this->assertContains('find', $findicon, '', true);
+    }
 }
index 396c297..8c43626 100644 (file)
             {
                 "name": "accordion.h5p",
                 "link": "http://something/contentbank/contenttype/h5p/view.php?url=http://something/pluginfile.php/1/contentbank/public/accordion.h5p",
-                "icon" : "<img class='icon iconsize-big' alt='accordion.h5p' aria-hidden='true' src='http://something/theme/image.php/boost/core/1581597850/f/h5p-64'>"
+                "icon" : "http://something/theme/image.php/boost/core/1581597850/f/h5p-64"
             },
             {
                 "name": "resume.pdf",
-                "icon": "<img class='icon iconsize-big' alt='resume.pdf' aria-hidden='true' src='http://something/theme/image.php/boost/core/1584597850/f/pdf-64'>"
+                "icon": "http://something/theme/image.php/boost/core/1584597850/f/pdf-64"
             }
         ],
         "tools": [
@@ -65,7 +65,7 @@
             <div class="cb-file position-relative mb-2" data-file="{{{name}}}">
                 <div class="p-2">
                     <div class="cb-thumbnail mb-1 text-center">
-                        {{{ icon }}}
+                        <img class="icon iconsize-big" alt="{{{name}}}" title="{{{name}}}" src="{{{ icon }}}">
                     </div>
 
                     {{#link}}
index b49a79c..9b142b7 100644 (file)
@@ -26,14 +26,12 @@ Feature: Delete H5P file from the content bank
     And I click on "Save changes" "button"
 
   Scenario: Admins can delete content from the content bank
-    Given I should see "filltheblanks.h5p"
-    And I follow "filltheblanks.h5p"
-    When I open the action menu in "region-main-settings-menu" "region"
-    Then I should see "Delete"
-    And I choose "Delete" in the open action menu
+    Given I open the action menu in "region-main-settings-menu" "region"
+    And I should see "Delete"
+    When I choose "Delete" in the open action menu
     And I should see "Are you sure you want to delete the content 'filltheblanks.h5p'"
     And I click on "Cancel" "button" in the "Delete content" "dialogue"
-    And I should see "filltheblanks.h5p"
+    Then I should see "filltheblanks.h5p"
     And I open the action menu in "region-main-settings-menu" "region"
     And I choose "Delete" in the open action menu
     And I click on "Delete" "button" in the "Delete content" "dialogue"
@@ -68,8 +66,5 @@ Feature: Delete H5P file from the content bank
     And I click on "find-the-words.h5p" "link"
     And I click on "Select this file" "button"
     And I click on "Save changes" "button"
-    And I should see "filltheblanks.h5p"
-    And I should see "find-the-words.h5p"
-    And I follow "find-the-words.h5p"
     And I open the action menu in "region-main-settings-menu" "region"
     And I should see "Delete"
index d5cf377..982d2f1 100644 (file)
@@ -106,7 +106,10 @@ class core_contenttype_contenttype_testcase extends \advanced_testcase {
 
         $systemcontext = \context_system::instance();
         $testable = new contenttype($systemcontext);
-        $icon = $testable->get_icon('new content');
+        $record = new stdClass();
+        $record->name = 'New content';
+        $content = $testable->create_content($record);
+        $icon = $testable->get_icon($content);
         $this->assertContains('archive', $icon);
     }
 
index 850d4ef..f279481 100644 (file)
@@ -40,11 +40,11 @@ class contenttype extends \core_contentbank\contenttype {
     /**
      * Returns the URL where the content will be visualized.
      *
-     * @param stdClass $record  Th content to be displayed.
+     * @param  content $content The content to delete.
      * @return string            URL where to visualize the given content.
      */
-    public function get_view_url(\stdClass $record): string {
-        $fileurl = $this->get_file_url($record->id);
+    public function get_view_url(\core_contentbank\content $content): string {
+        $fileurl = $this->get_file_url($content->get_id());
         $url = $fileurl."?forcedownload=1";
 
         return $url;
@@ -53,13 +53,13 @@ class contenttype extends \core_contentbank\contenttype {
     /**
      * Returns the HTML code to render the icon for content bank contents.
      *
-     * @param string $contentname   The contentname to add as alt value to the icon.
+     * @param  content $content The content to delete.
      * @return string               HTML code to render the icon
      */
-    public function get_icon(string $contentname): string {
+    public function get_icon(\core_contentbank\content $content): string {
         global $OUTPUT;
 
-        return $OUTPUT->pix_icon('f/archive-64', $contentname, 'moodle', ['class' => 'iconsize-big']);
+        return $OUTPUT->image_url('f/archive-64', 'moodle')->out(false);
     }
 
     /**
index 0a88a69..371d27f 100644 (file)
@@ -80,6 +80,8 @@ if ($mform->is_cancelled()) {
         $file = reset($files);
         $content = $cb->create_content_from_file($context, $USER->id, $file);
         file_save_draft_area_files($formdata->file, $contextid, 'contentbank', 'public', $content->get_id());
+        $viewurl = new \moodle_url('/contentbank/view.php', ['id' => $content->get_id(), 'contextid' => $contextid]);
+        redirect($viewurl);
     }
     redirect($returnurl);
 }
index d34205c..c95d7fd 100644 (file)
@@ -118,7 +118,7 @@ if ($errormsg !== '') {
     echo $OUTPUT->notification($statusmsg, 'notifysuccess');
 }
 if ($contenttype->can_access()) {
-    echo $contenttype->get_view_content($record);
+    echo $contenttype->get_view_content($content);
 }
 
 echo $OUTPUT->box_end();
index b86acad..81c7625 100644 (file)
@@ -487,4 +487,19 @@ class api {
         $pathnamehash = $fs->get_pathname_hash($contextid, $component, $filearea, $itemid, $filepath, $filename);
         return $pathnamehash;
     }
+
+    /**
+     * Returns the H5P content object corresponding to an H5P content file.
+     *
+     * @param string $pathnamehash The pathnamehash of the file associated to an H5P content.
+     *
+     * @return null|\stdClass H5P content object or null if not found.
+     */
+    public static function get_content_from_pathnamehash(string $pathnamehash): ?\stdClass {
+        global $DB;
+
+        $h5p = $DB->get_record('h5p', ['pathnamehash' => $pathnamehash]);
+
+        return ($h5p) ? $h5p : null;
+    }
 }