From 16dde93127e2d22daa148acbba23e24429863482 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Mon, 13 Apr 2015 17:34:35 +0200 Subject: [PATCH 1/2] MDL-49837 assign: Fix external functions return declaration Incorrect use of external_warnings function --- mod/assign/externallib.php | 32 ++++++++------------------------ 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/mod/assign/externallib.php b/mod/assign/externallib.php index 7c12410f633..2d06fef1dcd 100644 --- a/mod/assign/externallib.php +++ b/mod/assign/externallib.php @@ -1263,9 +1263,7 @@ class mod_assign_external extends external_api { * @since Moodle 2.6 */ public static function lock_submissions_returns() { - return new external_multiple_structure( - new external_warnings() - ); + return new external_warnings(); } /** @@ -1327,9 +1325,7 @@ class mod_assign_external extends external_api { * @since Moodle 2.6 */ public static function revert_submissions_to_draft_returns() { - return new external_multiple_structure( - new external_warnings() - ); + return new external_warnings(); } /** @@ -1391,9 +1387,7 @@ class mod_assign_external extends external_api { * @since Moodle 2.6 */ public static function unlock_submissions_returns() { - return new external_multiple_structure( - new external_warnings() - ); + return new external_warnings(); } /** @@ -1453,9 +1447,7 @@ class mod_assign_external extends external_api { * @since Moodle 2.6 */ public static function submit_for_grading_returns() { - return new external_multiple_structure( - new external_warnings() - ); + return new external_warnings(); } /** @@ -1533,9 +1525,7 @@ class mod_assign_external extends external_api { * @since Moodle 2.6 */ public static function save_user_extensions_returns() { - return new external_multiple_structure( - new external_warnings() - ); + return new external_warnings(); } /** @@ -1589,9 +1579,7 @@ class mod_assign_external extends external_api { * @since Moodle 2.6 */ public static function reveal_identities_returns() { - return new external_multiple_structure( - new external_warnings() - ); + return new external_warnings(); } /** @@ -1667,9 +1655,7 @@ class mod_assign_external extends external_api { * @since Moodle 2.6 */ public static function save_submission_returns() { - return new external_multiple_structure( - new external_warnings() - ); + return new external_warnings(); } /** @@ -2017,8 +2003,6 @@ class mod_assign_external extends external_api { * @since Moodle 2.6 */ public static function copy_previous_attempt_returns() { - return new external_multiple_structure( - new external_warnings() - ); + return new external_warnings(); } } From 24eae8af65ae4aca0d310b5f8691468073159b72 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Mon, 13 Apr 2015 17:35:20 +0200 Subject: [PATCH 2/2] MDL-49837 assign: Use correct assertions and fix return params cleaning --- mod/assign/tests/externallib_test.php | 31 +++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/mod/assign/tests/externallib_test.php b/mod/assign/tests/externallib_test.php index f764d23574c..97236bbc67c 100644 --- a/mod/assign/tests/externallib_test.php +++ b/mod/assign/tests/externallib_test.php @@ -284,6 +284,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { $assignmentids[] = $assign1->id; $result = mod_assign_external::get_submissions($assignmentids); + $result = external_api::clean_returnvalue(mod_assign_external::get_submissions_returns(), $result); // Check the online text submission is returned. $this->assertEquals(1, count($result['assignments'])); @@ -485,6 +486,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { $this->setUser($teacher); $students = array($student1->id, $student2->id); $result = mod_assign_external::lock_submissions($instance->id, $students); + $result = external_api::clean_returnvalue(mod_assign_external::lock_submissions_returns(), $result); // Check for 0 warnings. $this->assertEquals(0, count($result)); @@ -549,11 +551,13 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { $this->setUser($teacher); $students = array($student1->id, $student2->id); $result = mod_assign_external::lock_submissions($instance->id, $students); + $result = external_api::clean_returnvalue(mod_assign_external::lock_submissions_returns(), $result); // Check for 0 warnings. $this->assertEquals(0, count($result)); $result = mod_assign_external::unlock_submissions($instance->id, $students); + $result = external_api::clean_returnvalue(mod_assign_external::unlock_submissions_returns(), $result); // Check for 0 warnings. $this->assertEquals(0, count($result)); @@ -609,11 +613,13 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { $plugin->save($submission, $data); $result = mod_assign_external::submit_for_grading($instance->id, false); + $result = external_api::clean_returnvalue(mod_assign_external::submit_for_grading_returns(), $result); // Should be 1 fail because the submission statement was not aceptted. $this->assertEquals(1, count($result)); $result = mod_assign_external::submit_for_grading($instance->id, true); + $result = external_api::clean_returnvalue(mod_assign_external::submit_for_grading_returns(), $result); // Check for 0 warnings. $this->assertEquals(0, count($result)); @@ -664,28 +670,34 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { $this->setUser($student1); $result = mod_assign_external::submit_for_grading($instance->id, true); + $result = external_api::clean_returnvalue(mod_assign_external::submit_for_grading_returns(), $result); // Check for 0 warnings. $this->assertEquals(1, count($result)); $this->setUser($teacher); $result = mod_assign_external::save_user_extensions($instance->id, array($student1->id), array($now, $tomorrow)); + $result = external_api::clean_returnvalue(mod_assign_external::save_user_extensions_returns(), $result); $this->assertEquals(1, count($result)); $this->setUser($teacher); $result = mod_assign_external::save_user_extensions($instance->id, array($student1->id), array($yesterday - 10)); + $result = external_api::clean_returnvalue(mod_assign_external::save_user_extensions_returns(), $result); $this->assertEquals(1, count($result)); $this->setUser($teacher); $result = mod_assign_external::save_user_extensions($instance->id, array($student1->id), array($tomorrow)); + $result = external_api::clean_returnvalue(mod_assign_external::save_user_extensions_returns(), $result); $this->assertEquals(0, count($result)); $this->setUser($student1); $result = mod_assign_external::submit_for_grading($instance->id, true); + $result = external_api::clean_returnvalue(mod_assign_external::submit_for_grading_returns(), $result); $this->assertEquals(0, count($result)); $this->setUser($student1); $result = mod_assign_external::save_user_extensions($instance->id, array($student1->id), array($now, $tomorrow)); + $result = external_api::clean_returnvalue(mod_assign_external::save_user_extensions_returns(), $result); } @@ -726,11 +738,13 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { $this->setUser($student1); $this->setExpectedException('required_capability_exception'); $result = mod_assign_external::reveal_identities($instance->id); + $result = external_api::clean_returnvalue(mod_assign_external::reveal_identities_returns(), $result); $this->assertEquals(1, count($result)); $this->assertEquals(true, $assign->is_blind_marking()); $this->setUser($teacher); $result = mod_assign_external::reveal_identities($instance->id); + $result = external_api::clean_returnvalue(mod_assign_external::reveal_identities_returns(), $result); $this->assertEquals(0, count($result)); $this->assertEquals(false, $assign->is_blind_marking()); @@ -745,6 +759,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { $assign = new assign($context, $cm, $course); $result = mod_assign_external::reveal_identities($instance->id); + $result = external_api::clean_returnvalue(mod_assign_external::reveal_identities_returns(), $result); $this->assertEquals(1, count($result)); $this->assertEquals(false, $assign->is_blind_marking()); @@ -790,12 +805,14 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { // Simulate a submission. $this->setUser($student1); $result = mod_assign_external::submit_for_grading($instance->id, true); + $result = external_api::clean_returnvalue(mod_assign_external::submit_for_grading_returns(), $result); $this->assertEquals(0, count($result)); // Ready to test. $this->setUser($teacher); $students = array($student1->id, $student2->id); $result = mod_assign_external::revert_submissions_to_draft($instance->id, array($student1->id)); + $result = external_api::clean_returnvalue(mod_assign_external::submit_for_grading_returns(), $result); // Check for 0 warnings. $this->assertEquals(0, count($result)); @@ -882,6 +899,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { 'itemid'=>$draftidonlinetext); $submissionpluginparams['onlinetext_editor'] = $onlinetexteditorparams; $result = mod_assign_external::save_submission($instance->id, $submissionpluginparams); + $result = external_api::clean_returnvalue(mod_assign_external::save_submission_returns(), $result); $this->assertEquals(0, count($result)); @@ -956,11 +974,11 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { 'released', false, $feedbackpluginparams); - // No warnings. - $this->assertEquals(0, count($result)); + $this->assertNull($result); $result = mod_assign_external::get_grades(array($instance->id)); + $result = external_api::clean_returnvalue(mod_assign_external::get_grades_returns(), $result); $this->assertEquals($result['assignments'][0]['grades'][0]['grade'], '50.0'); } @@ -1106,8 +1124,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { $grades[] = $student2gradeinfo; $result = mod_assign_external::save_grades($instance->id, false, $grades); - // No warnings. - $this->assertEquals(0, count($result)); + $this->assertNull($result); $student1grade = $DB->get_record('assign_grades', array('userid' => $student1->id, 'assignment' => $instance->id), @@ -1219,6 +1236,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { $this->setExpectedException('invalid_parameter_exception'); // Expect an exception since 2 grades have been submitted for the same team. $result = mod_assign_external::save_grades($instance->id, true, $grades1); + $result = external_api::clean_returnvalue(mod_assign_external::save_grades_returns(), $result); $grades2 = array(); $student3gradeinfo = array(); @@ -1239,6 +1257,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { $student4gradeinfo['plugindata'] = $feedbackpluginparams; $grades2[] = $student4gradeinfo; $result = mod_assign_external::save_grades($instance->id, true, $grades2); + $result = external_api::clean_returnvalue(mod_assign_external::save_grades_returns(), $result); // There should be no warnings. $this->assertEquals(0, count($result)); @@ -1300,6 +1319,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { $submissionpluginparams['onlinetext_editor'] = $onlinetexteditorparams; $submissionpluginparams['files_filemanager'] = file_get_unused_draft_itemid(); $result = mod_assign_external::save_submission($instance->id, $submissionpluginparams); + $result = external_api::clean_returnvalue(mod_assign_external::save_submission_returns(), $result); $this->setUser($teacher); // Add a grade and reopen the attempt. @@ -1317,15 +1337,18 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { 'released', false, $feedbackpluginparams); + $this->assertNull($result); $this->setUser($student1); // Now copy the previous attempt. $result = mod_assign_external::copy_previous_attempt($instance->id); + $result = external_api::clean_returnvalue(mod_assign_external::copy_previous_attempt_returns(), $result); // No warnings. $this->assertEquals(0, count($result)); $this->setUser($teacher); $result = mod_assign_external::get_submissions(array($instance->id)); + $result = external_api::clean_returnvalue(mod_assign_external::get_submissions_returns(), $result); // Check we are now on the second attempt. $this->assertEquals($result['assignments'][0]['submissions'][0]['attemptnumber'], 1);