MDL-69529 contentbank: Remove redundant code.
authorIlya Tregubov <ilya@moodle.com>
Mon, 7 Dec 2020 13:32:55 +0000 (15:32 +0200)
committerIlya Tregubov <ilya@moodle.com>
Mon, 4 Jan 2021 07:28:17 +0000 (09:28 +0200)
contentbank/classes/contentbank.php
contentbank/classes/external/delete_content.php
contentbank/classes/external/rename_content.php
contentbank/view.php

index 9152067..6374055 100644 (file)
@@ -215,8 +215,7 @@ class contentbank {
 
         $records = $DB->get_records_select('contentbank_content', $sql, $params, 'name ASC');
         foreach ($records as $record) {
-            $contentclass = "\\$record->contenttype\\content";
-            $content = new $contentclass($record);
+            $content = $this->get_content_from_id($record->id);
             if ($content->is_view_allowed()) {
                 $contents[] = $content;
             }
@@ -267,14 +266,10 @@ class contentbank {
         $result = true;
         $records = $DB->get_records('contentbank_content', ['contextid' => $context->id]);
         foreach ($records as $record) {
-            $contenttypeclass = "\\$record->contenttype\\contenttype";
-            if (class_exists($contenttypeclass)) {
-                $contenttype = new $contenttypeclass($context);
-                $contentclass = "\\$record->contenttype\\content";
-                $content = new $contentclass($record);
-                if (!$contenttype->delete_content($content)) {
-                    $result = false;
-                }
+            $content = $this->get_content_from_id($record->id);
+            $contenttype = $content->get_content_type_instance();
+            if (!$contenttype->delete_content($content)) {
+                $result = false;
             }
         }
         return $result;
@@ -293,14 +288,10 @@ class contentbank {
         $result = true;
         $records = $DB->get_records('contentbank_content', ['contextid' => $from->id]);
         foreach ($records as $record) {
-            $contenttypeclass = "\\$record->contenttype\\contenttype";
-            if (class_exists($contenttypeclass)) {
-                $contenttype = new $contenttypeclass($from);
-                $contentclass = "\\$record->contenttype\\content";
-                $content = new $contentclass($record);
-                if (!$contenttype->move_content($content, $to)) {
-                    $result = false;
-                }
+            $content = $this->get_content_from_id($record->id);
+            $contenttype = $content->get_content_type_instance();
+            if (!$contenttype->move_content($content, $to)) {
+                $result = false;
             }
         }
         return $result;
index 65846aa..caf18c2 100644 (file)
@@ -30,6 +30,7 @@ defined('MOODLE_INTERNAL') || die();
 global $CFG;
 require_once($CFG->libdir . '/externallib.php');
 
+use core_contentbank\contentbank;
 use external_api;
 use external_function_parameters;
 use external_multiple_structure;
@@ -70,34 +71,31 @@ class delete_content extends external_api {
         $warnings = [];
 
         $params = self::validate_parameters(self::execute_parameters(), ['contentids' => $contentids]);
+        $cb = new contentbank();
         foreach ($params['contentids'] as $contentid) {
             try {
                 $record = $DB->get_record('contentbank_content', ['id' => $contentid], '*', MUST_EXIST);
-                $contenttypeclass = "\\$record->contenttype\\contenttype";
-                if (class_exists($contenttypeclass)) {
-                    $context = \context::instance_by_id($record->contextid, MUST_EXIST);
-                    self::validate_context($context);
-                    $contenttype = new $contenttypeclass($context);
-                    $contentclass = "\\$record->contenttype\\content";
-                    $content = new $contentclass($record);
-                    // Check capability.
-                    if ($contenttype->can_delete($content)) {
-                        // This content can be deleted.
-                        if (!$contenttype->delete_content($content)) {
-                            $warnings[] = [
-                                'item' => $contentid,
-                                'warningcode' => 'contentnotdeleted',
-                                'message' => get_string('contentnotdeleted', 'core_contentbank')
-                            ];
-                        }
-                    } else {
-                        // The user has no permission to delete this content.
+                $content = $cb->get_content_from_id($record->id);
+                $contenttype = $content->get_content_type_instance();
+                $context = \context::instance_by_id($record->contextid, MUST_EXIST);
+                self::validate_context($context);
+                // Check capability.
+                if ($contenttype->can_delete($content)) {
+                    // This content can be deleted.
+                    if (!$contenttype->delete_content($content)) {
                         $warnings[] = [
                             'item' => $contentid,
-                            'warningcode' => 'nopermissiontodelete',
-                            'message' => get_string('nopermissiontodelete', 'core_contentbank')
+                            'warningcode' => 'contentnotdeleted',
+                            'message' => get_string('contentnotdeleted', 'core_contentbank')
                         ];
                     }
+                } else {
+                    // The user has no permission to delete this content.
+                    $warnings[] = [
+                        'item' => $contentid,
+                        'warningcode' => 'nopermissiontodelete',
+                        'message' => get_string('nopermissiontodelete', 'core_contentbank')
+                    ];
                 }
             } catch (\moodle_exception $e) {
                 // The content or the context don't exist.
index 5cd9d75..1374aae 100644 (file)
@@ -29,6 +29,7 @@ defined('MOODLE_INTERNAL') || die();
 global $CFG;
 require_once($CFG->libdir . '/externallib.php');
 
+use core_contentbank\contentbank;
 use external_api;
 use external_function_parameters;
 use external_single_structure;
@@ -76,35 +77,33 @@ class rename_content extends external_api {
             'name' => $name,
         ]);
         $params['name'] = clean_param($params['name'], PARAM_TEXT);
-        try {
-            $record = $DB->get_record('contentbank_content', ['id' => $contentid], '*', MUST_EXIST);
-            $contenttypeclass = "\\$record->contenttype\\contenttype";
-            if (class_exists($contenttypeclass)) {
+
+        // If name is empty don't try to rename and return a more detailed message.
+        if (empty(trim($params['name']))) {
+            $warnings[] = [
+                'item' => $contentid,
+                'warningcode' => 'emptynamenotallowed',
+                'message' => get_string('emptynamenotallowed', 'core_contentbank')
+            ];
+        } else {
+            try {
+                $record = $DB->get_record('contentbank_content', ['id' => $contentid], '*', MUST_EXIST);
+                $cb = new contentbank();
+                $content = $cb->get_content_from_id($record->id);
+                $contenttype = $content->get_content_type_instance();
                 $context = \context::instance_by_id($record->contextid, MUST_EXIST);
                 self::validate_context($context);
-                $contenttype = new $contenttypeclass($context);
-                $contentclass = "\\$record->contenttype\\content";
-                $content = new $contentclass($record);
                 // Check capability.
                 if ($contenttype->can_manage($content)) {
-                    if (empty(trim($name))) {
-                        // If name is empty don't try to rename and return a more detailed message.
+                    // This content can be renamed.
+                    if ($contenttype->rename_content($content, $params['name'])) {
+                        $result = true;
+                    } else {
                         $warnings[] = [
                             'item' => $contentid,
-                            'warningcode' => 'emptynamenotallowed',
-                            'message' => get_string('emptynamenotallowed', 'core_contentbank')
+                            'warningcode' => 'contentnotrenamed',
+                            'message' => get_string('contentnotrenamed', 'core_contentbank')
                         ];
-                    } else {
-                        // This content can be renamed.
-                        if ($contenttype->rename_content($content, $params['name'])) {
-                            $result = true;
-                        } else {
-                            $warnings[] = [
-                                'item' => $contentid,
-                                'warningcode' => 'contentnotrenamed',
-                                'message' => get_string('contentnotrenamed', 'core_contentbank')
-                            ];
-                        }
                     }
                 } else {
                     // The user has no permission to manage this content.
@@ -114,14 +113,14 @@ class rename_content extends external_api {
                         'message' => get_string('nopermissiontomanage', 'core_contentbank')
                     ];
                 }
+            } catch (\moodle_exception $e) {
+                // The content or the context don't exist.
+                $warnings[] = [
+                    'item' => $contentid,
+                    'warningcode' => 'exception',
+                    'message' => $e->getMessage()
+                ];
             }
-        } catch (\moodle_exception $e) {
-            // The content or the context don't exist.
-            $warnings[] = [
-                'item' => $contentid,
-                'warningcode' => 'exception',
-                'message' => $e->getMessage()
-            ];
         }
 
         return [
index 4c84b1b..46daffb 100644 (file)
@@ -58,13 +58,9 @@ $title .= ": ".$record->name;
 $PAGE->set_title($title);
 $PAGE->set_pagetype('contentbank');
 
-$contenttypeclass = "\\$record->contenttype\\contenttype";
-$contentclass = "\\$record->contenttype\\content";
-if (!class_exists($contenttypeclass) || !class_exists($contentclass)) {
-    print_error('contenttypenotfound', 'error', $returnurl, $record->contenttype);
-}
-$contenttype = new $contenttypeclass($context);
-$content = new $contentclass($record);
+$cb = new \core_contentbank\contentbank();
+$content = $cb->get_content_from_id($record->id);
+$contenttype = $content->get_content_type_instance();
 
 // Create the cog menu with all the secondary actions, such as delete, rename...
 $actionmenu = new action_menu();