Skip to content

Commit

Permalink
add dry run mode to upload_course endpoint to check permissions befor…
Browse files Browse the repository at this point in the history 
…e actually uploading a potentially large mbz file
  • Loading branch information
@Glutamat42
Glutamat42 committed Jul 17, 2023
1 parent 851f270 commit b8c6d1b
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 13 deletions.
20 changes: 17 additions & 3 deletions classes/external/upload_course.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ public static function execute_parameters(): external_function_parameters {
return new external_function_parameters(
array(
'category_id' => new external_value(PARAM_INT, 'ID of the category in which the course should be created. If null, the course will be created in the category with the lowest available ID. Please note that even if a user has restore permissions for a category other than the one with the lowest ID, the course restoration process will not be successful.', VALUE_OPTIONAL),
'mbz' => new external_value(PARAM_FILE, 'Required (moodle tag "optional" is due to moodle limitations). MBZ as file upload. Upload the file in this field. Moodle external_api wont recognize it / this field will be empty but it can be loaded from this field via plain PHP code.', VALUE_OPTIONAL),
'only_check_permissions' => new external_value(PARAM_BOOL, 'Check only if user has the permissions for restore. No mbz needed. Will return generic data for course name and id.', VALUE_OPTIONAL),
'mbz' => new external_value(PARAM_FILE, 'Required (moodle tag "optional" is due to moodle limitations), except if only_check_permissions is true. MBZ as file upload. Upload the file in this field. Moodle external_api wont recognize it / this field will be empty but it can be loaded from this field via plain PHP code.', VALUE_OPTIONAL),
)
);
}
Expand All @@ -53,9 +54,15 @@ public static function execute_returns(): external_function_parameters {
* @throws moodle_exception
* @throws Exception
*/
public static function execute(int $category_id=null): array {
public static function execute(int $category_id=null, $only_check_permissions=false): array {
global $USER;

// param validation except mbz
$params = self::validate_parameters(self::execute_parameters(), array('category_id' => $category_id, 'only_check_permissions' => $only_check_permissions));
$category_id = $params['category_id'];
$only_check_permissions = $params['only_check_permissions'];


// get category id (if parameter is set, use this id, otherwise the first the user is allowed to restore courses in)
if (empty($category_id)) {
$categories = core_course_category::make_categories_list('moodle/restore:restorecourse');
Expand All @@ -69,7 +76,14 @@ public static function execute(int $category_id=null): array {
$context = context_coursecat::instance($category_id);
require_capability('moodle/restore:restorecourse', $context);

// Moodle parameter validation not needed because moodle is too stupid to support direct file upload
if ($only_check_permissions) {
return array('data' => array(
'course_id' => -1,
'course_fullname' => ''
));
}

// Additional check for mbz parameter, because Moodle is too stupid to support direct file upload
// instead manual validation is needed
if (!isset($_FILES['mbz'])) {
throw new invalid_parameter_exception('mbz is missing');
Expand Down
61 changes: 51 additions & 10 deletions tests/external/upload_course_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,49 +57,63 @@ public function provide_test_execute_data() {
'fail_validation' => false,
'valid_user' => true,
'specify_course_cat' => false,
'dry_run' => false,
],
'fail_validation' => [
'is_adler_course' => true,
'upload_error' => UPLOAD_ERR_OK,
'fail_validation' => true,
'valid_user' => true,
'specify_course_cat' => false,
'dry_run' => false,
],
'mbz_upload_failed' => [
'is_adler_course' => true,
'upload_error' => UPLOAD_ERR_NO_FILE,
'fail_validation' => false,
'valid_user' => true,
'specify_course_cat' => false,
'dry_run' => false,
],
'not_adler_course' => [
'is_adler_course' => false,
'upload_error' => UPLOAD_ERR_OK,
'fail_validation' => false,
'valid_user' => true,
'specify_course_cat' => false,
'dry_run' => false,
],
'user_not_allowed' => [
'is_adler_course' => true,
'upload_error' => UPLOAD_ERR_OK,
'fail_validation' => false,
'valid_user' => false,
'specify_course_cat' => false,
'dry_run' => false,
],
'specified_course_cat' => [
'is_adler_course' => true,
'upload_error' => UPLOAD_ERR_OK,
'fail_validation' => false,
'valid_user' => true,
'specify_course_cat' => true,
'dry_run' => false,
],
'dry_run' => [
'is_adler_course' => true,
'upload_error' => UPLOAD_ERR_OK,
'fail_validation' => false,
'valid_user' => true,
'specify_course_cat' => true,
'dry_run' => true,
]
];
}

/**
* @dataProvider provide_test_execute_data
*/
public function test_execute($is_adler_course, $upload_error, $fail_validation, $valid_user, $specify_course_cat) {
public function test_execute($is_adler_course, $upload_error, $fail_validation, $valid_user, $specify_course_cat, $dry_run) {
$test_course_filepath = $this->generate_mbz($is_adler_course);

global $DB;
Expand Down Expand Up @@ -135,17 +149,22 @@ public function test_execute($is_adler_course, $upload_error, $fail_validation,
$this->expectExceptionMessage('not_allowed');
}

if($specify_course_cat) {
$course_cat = $this->getDataGenerator()->create_category();
upload_course::execute($course_cat->id);
} else {
upload_course::execute();
}
// case specify_course_cat
$course_cat = $this->getDataGenerator()->create_category();
$param_course_cat = $specify_course_cat ? $course_cat->id : null;

// case dry_run
$param_dry_run = $dry_run ? true : false;
upload_course::execute($param_course_cat, $param_dry_run);


$course_count_after = $DB->count_records('course');

$this->assertEquals($course_count_before + 1, $course_count_after);
if ($dry_run) {
$this->assertEquals($course_count_before, $course_count_after);
} else {
$this->assertEquals($course_count_before + 1, $course_count_after);
}
}


Expand Down Expand Up @@ -175,7 +194,29 @@ public function test_execute_returns($success) {
}
}

public function test_execute_parameters() {
upload_course::validate_parameters(upload_course::execute_parameters(), []);
public function provide_test_execute_parameters_data() {
return [
'1' => [
'data' => [
'category_id' => 7,

]
],
'2' => [
'data' => [
'only_check_permissions' => true
],
],
'3' => [
'data' => []
]
];
}

/**
* @dataProvider provide_test_execute_parameters_data
*/
public function test_execute_parameters($data) {
upload_course::validate_parameters(upload_course::execute_parameters(), $data);
}
}

0 comments on commit b8c6d1b

Please sign in to comment.