From 13e814bcb1df7cf2aa9638847fc24f8bdff36e88 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 49575b9a3ac..f348ac3cb5a 100644 --- a/mod/assign/externallib.php +++ b/mod/assign/externallib.php @@ -1308,9 +1308,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(); } /** @@ -1372,9 +1370,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(); } /** @@ -1436,9 +1432,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(); } /** @@ -1498,9 +1492,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(); } /** @@ -1578,9 +1570,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(); } /** @@ -1634,9 +1624,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(); } /** @@ -1712,9 +1700,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(); } /** @@ -2062,8 +2048,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 7f45111e36de33e8f81395c23acfc75e1e377f2b 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 039c0c7b335..a6e9426e610 100644 --- a/mod/assign/tests/externallib_test.php +++ b/mod/assign/tests/externallib_test.php @@ -327,6 +327,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'])); @@ -528,6 +529,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)); @@ -592,11 +594,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)); @@ -652,11 +656,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)); @@ -707,28 +713,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); } @@ -769,11 +781,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()); @@ -788,6 +802,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()); @@ -833,12 +848,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::revert_submissions_to_draft_returns(), $result); // Check for 0 warnings. $this->assertEquals(0, count($result)); @@ -925,6 +942,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)); @@ -999,11 +1017,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'); } @@ -1149,8 +1167,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), @@ -1262,6 +1279,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(); @@ -1282,6 +1300,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)); @@ -1343,6 +1362,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. @@ -1360,15 +1380,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);