From b0da618b61a90d51b7249c5c3308537e1786e4f2 Mon Sep 17 00:00:00 2001
From: Damyon Wiese <damyon@moodle.com>
Date: Tue, 8 Jan 2013 15:49:03 +0800
Subject: [PATCH] MDL-37337 Assignment: Restructure assignment settings screen
 to make it "wieldy"

---
 mod/assign/externallib.php              |  2 +-
 mod/assign/lang/en/assign.php           |  3 ++
 mod/assign/locallib.php                 | 35 ++++++++++--------
 mod/assign/mod_form.php                 | 48 ++++++++++++++-----------
 mod/assign/submission/file/locallib.php |  4 +--
 mod/assign/tests/lib_test.php           |  1 -
 mod/assign/tests/locallib_test.php      |  6 ++--
 7 files changed, 58 insertions(+), 41 deletions(-)

diff --git a/mod/assign/externallib.php b/mod/assign/externallib.php
index a5ad1c4a1f5..58b80836394 100644
--- a/mod/assign/externallib.php
+++ b/mod/assign/externallib.php
@@ -185,7 +185,7 @@ class mod_assign_external extends external_api {
             array(
                 'assignments' => new external_multiple_structure(self::assign_grades(), 'list of assignment grade information'),
                 'warnings'      => new external_warnings('item is always \'assignment\'',
-                    'when errorcode is 3 then itemid is an assignment id. When errorcode is 1, itemid is a course module instance id',
+                    'when errorcode is 3 then itemid is an assignment id. When errorcode is 1, itemid is a course module id',
                     'errorcode can be 3 (no grades found) or 1 (no permission to get grades)')
             )
         );
diff --git a/mod/assign/lang/en/assign.php b/mod/assign/lang/en/assign.php
index ef1c7edcf10..a189bae5e9f 100644
--- a/mod/assign/lang/en/assign.php
+++ b/mod/assign/lang/en/assign.php
@@ -324,4 +324,7 @@ $string['viewfull'] = 'View full';
 $string['viewsummary'] = 'View summary';
 $string['viewsubmissiongradingtable'] = 'View submission grading table.';
 $string['viewrevealidentitiesconfirm'] = 'View reveal student identities confirmation page.';
+$string['submissiontypes'] = 'Submission types';
+$string['feedbacktypes'] = 'Feedback types';
+$string['groupsubmissionsettings'] = 'Group submission settings';
 
diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php
index e28ba439eed..e428889dd20 100644
--- a/mod/assign/locallib.php
+++ b/mod/assign/locallib.php
@@ -657,7 +657,7 @@ class assign {
     protected function update_plugin_instance(assign_plugin $plugin, stdClass $formdata) {
         if ($plugin->is_visible()) {
             $enabledname = $plugin->get_subtype() . '_' . $plugin->get_type() . '_enabled';
-            if ($formdata->$enabledname) {
+            if (!empty($formdata->$enabledname)) {
                 $plugin->enable();
                 if (!$plugin->save_settings($formdata)) {
                     print_error($plugin->get_error());
@@ -843,17 +843,18 @@ class assign {
      * @param assign_plugin $plugin The plugin to add the settings from
      * @param MoodleQuickForm $mform The form to add the configuration settings to.
      *                               This form is modified directly (not returned).
+     * @param array $pluginsenabled A list of form elements to be added to a group.
+     *                              The new element is added to this array by this function.
      * @return void
      */
-    protected function add_plugin_settings(assign_plugin $plugin, MoodleQuickForm $mform) {
+    protected function add_plugin_settings(assign_plugin $plugin, MoodleQuickForm $mform, & $pluginsenabled) {
         global $CFG;
         if ($plugin->is_visible()) {
-            $mform->addElement('selectyesno',
-                               $plugin->get_subtype() . '_' . $plugin->get_type() . '_enabled',
-                               $plugin->get_name());
-            $mform->addHelpButton($plugin->get_subtype() . '_' . $plugin->get_type() . '_enabled',
-                                  'enabled',
-                                  $plugin->get_subtype() . '_' . $plugin->get_type());
+
+            $name = $plugin->get_subtype() . '_' . $plugin->get_type() . '_enabled';
+            $label = $plugin->get_name();
+            $label .= ' ' . $this->get_renderer()->help_icon('enabled', $plugin->get_subtype() . '_' . $plugin->get_type());
+            $pluginsenabled[] = $mform->createElement('checkbox', $name, '', $label);
 
             $default = get_config($plugin->get_subtype() . '_' . $plugin->get_type(), 'default');
             if ($plugin->get_config('enabled') !== false) {
@@ -874,16 +875,23 @@ class assign {
      * @return void
      */
     public function add_all_plugin_settings(MoodleQuickForm $mform) {
-        $mform->addElement('header', 'general', get_string('submissionsettings', 'assign'));
+        $mform->addElement('header', 'submissiontypes', get_string('submissionsettings', 'assign'));
 
+        $submissionpluginsenabled = array();
+        $group = $mform->addGroup(array(), 'submissionplugins', get_string('submissiontypes', 'assign'), array(' '), false);
         foreach ($this->submissionplugins as $plugin) {
-            $this->add_plugin_settings($plugin, $mform);
+            $this->add_plugin_settings($plugin, $mform, $submissionpluginsenabled);
+        }
+        $group->setElements($submissionpluginsenabled);
 
-        }
-        $mform->addElement('header', 'general', get_string('feedbacksettings', 'assign'));
+        $mform->addElement('header', 'feedbacktypes', get_string('feedbacksettings', 'assign'));
+        $feedbackpluginsenabled = array();
+        $group = $mform->addGroup(array(), 'feedbackplugins', get_string('feedbacktypes', 'assign'), array(' '), false);
         foreach ($this->feedbackplugins as $plugin) {
-            $this->add_plugin_settings($plugin, $mform);
+            $this->add_plugin_settings($plugin, $mform, $feedbackpluginsenabled);
         }
+        $group->setElements($feedbackpluginsenabled);
+        $mform->setExpanded('submissiontypes');
     }
 
     /**
@@ -3112,7 +3120,6 @@ class assign {
                     }
                 }
 
-
                 $feedbackstatus = new assign_feedback_status($gradefordisplay,
                                                       $gradeddate,
                                                       $grader,
diff --git a/mod/assign/mod_form.php b/mod/assign/mod_form.php
index 70bc77b4412..998d2e77d38 100644
--- a/mod/assign/mod_form.php
+++ b/mod/assign/mod_form.php
@@ -73,7 +73,9 @@ class mod_assign_mod_form extends moodleform_mod {
 
         $config = get_config('assign');
 
-        $mform->addElement('header', 'general', get_string('settings', 'assign'));
+        $assignment->add_all_plugin_settings($mform);
+
+        $mform->addElement('header', 'availability', get_string('availability', 'assign'));
 
         $name = get_string('allowsubmissionsfromdate', 'assign');
         $options = array('optional'=>true);
@@ -92,10 +94,12 @@ class mod_assign_mod_form extends moodleform_mod {
         $mform->setDefault('cutoffdate', time()+7*24*3600);
 
         $name = get_string('alwaysshowdescription', 'assign');
-        $mform->addElement('selectyesno', 'alwaysshowdescription', $name);
+        $mform->addElement('checkbox', 'alwaysshowdescription', $name);
         $mform->addHelpButton('alwaysshowdescription', 'alwaysshowdescription', 'assign');
         $mform->setDefault('alwaysshowdescription', 1);
 
+        $mform->addElement('header', 'submissionsettings', get_string('submissionsettings', 'assign'));
+
         $name = get_string('submissiondrafts', 'assign');
         $mform->addElement('selectyesno', 'submissiondrafts', $name);
         $mform->addHelpButton('submissiondrafts', 'submissiondrafts', 'assign');
@@ -114,16 +118,7 @@ class mod_assign_mod_form extends moodleform_mod {
             $mform->addElement('hidden', 'requiresubmissionstatement', 1);
         }
 
-        $name = get_string('sendnotifications', 'assign');
-        $mform->addElement('selectyesno', 'sendnotifications', $name);
-        $mform->addHelpButton('sendnotifications', 'sendnotifications', 'assign');
-        $mform->setDefault('sendnotifications', 1);
-
-        $name = get_string('sendlatenotifications', 'assign');
-        $mform->addElement('selectyesno', 'sendlatenotifications', $name);
-        $mform->addHelpButton('sendlatenotifications', 'sendlatenotifications', 'assign');
-        $mform->setDefault('sendlatenotifications', 1);
-        $mform->disabledIf('sendlatenotifications', 'sendnotifications', 'eq', 1);
+        $mform->addElement('header', 'groupsubmissionsettings', get_string('groupsubmissionsettings', 'assign'));
 
         $name = get_string('teamsubmission', 'assign');
         $mform->addElement('selectyesno', 'teamsubmission', $name);
@@ -150,6 +145,26 @@ class mod_assign_mod_form extends moodleform_mod {
         $mform->setDefault('teamsubmissiongroupingid', 0);
         $mform->disabledIf('teamsubmissiongroupingid', 'teamsubmission', 'eq', 0);
 
+        $mform->addElement('header', 'notifications', get_string('notifications', 'assign'));
+
+        $name = get_string('sendnotifications', 'assign');
+        $mform->addElement('selectyesno', 'sendnotifications', $name);
+        $mform->addHelpButton('sendnotifications', 'sendnotifications', 'assign');
+        $mform->setDefault('sendnotifications', 1);
+
+        $name = get_string('sendlatenotifications', 'assign');
+        $mform->addElement('selectyesno', 'sendlatenotifications', $name);
+        $mform->addHelpButton('sendlatenotifications', 'sendlatenotifications', 'assign');
+        $mform->setDefault('sendlatenotifications', 1);
+        $mform->disabledIf('sendlatenotifications', 'sendnotifications', 'eq', 1);
+
+        // Plagiarism enabling form.
+        if (!empty($CFG->enableplagiarism)) {
+            require_once($CFG->libdir . '/plagiarismlib.php');
+            plagiarism_get_form_elements_module($mform, $ctx->get_course_context(), 'mod_assign');
+        }
+
+        $this->standard_grading_coursemodule_elements();
         $name = get_string('blindmarking', 'assign');
         $mform->addElement('selectyesno', 'blindmarking', $name);
         $mform->addHelpButton('blindmarking', 'blindmarking', 'assign');
@@ -158,14 +173,6 @@ class mod_assign_mod_form extends moodleform_mod {
             $mform->freeze('blindmarking');
         }
 
-        // Plagiarism enabling form.
-        if (!empty($CFG->enableplagiarism)) {
-            require_once($CFG->libdir . '/plagiarismlib.php');
-            plagiarism_get_form_elements_module($mform, $ctx->get_course_context(), 'mod_assign');
-        }
-
-        $assignment->add_all_plugin_settings($mform);
-        $this->standard_grading_coursemodule_elements();
         $this->standard_coursemodule_elements();
 
         $this->add_action_buttons();
@@ -177,6 +184,7 @@ class mod_assign_mod_form extends moodleform_mod {
                                                   'assignment = ? AND grade <> -1',
                                                   array($this->_instance));
         }
+
         if ($mform->elementExists('grade') && $hasgrade) {
             $module = array(
                 'name' => 'mod_assign',
diff --git a/mod/assign/submission/file/locallib.php b/mod/assign/submission/file/locallib.php
index 3d160006e8c..bd089245806 100644
--- a/mod/assign/submission/file/locallib.php
+++ b/mod/assign/submission/file/locallib.php
@@ -85,7 +85,7 @@ class assign_submission_file extends assign_submission_plugin {
                               'maxfilessubmission',
                               'assignsubmission_file');
         $mform->setDefault('assignsubmission_file_maxfiles', $defaultmaxfilesubmissions);
-        $mform->disabledIf('assignsubmission_file_maxfiles', 'assignsubmission_file_enabled', 'eq', 0);
+        $mform->disabledIf('assignsubmission_file_maxfiles', 'assignsubmission_file_enabled', 'notchecked');
 
         $choices = get_max_upload_sizes($CFG->maxbytes,
                                         $COURSE->maxbytes,
@@ -105,7 +105,7 @@ class assign_submission_file extends assign_submission_plugin {
         $mform->setDefault('assignsubmission_file_maxsizebytes', $defaultmaxsubmissionsizebytes);
         $mform->disabledIf('assignsubmission_file_maxsizebytes',
                            'assignsubmission_file_enabled',
-                           'eq', 0);
+                           'notchecked');
     }
 
     /**
diff --git a/mod/assign/tests/lib_test.php b/mod/assign/tests/lib_test.php
index 11a3de41b0f..19d049c6fc2 100644
--- a/mod/assign/tests/lib_test.php
+++ b/mod/assign/tests/lib_test.php
@@ -181,7 +181,6 @@ class mod_assign_lib_testcase extends advanced_testcase {
 
         $submission = $assign->get_user_submission($this->students[0]->id, true);
 
-
         $this->expectOutputRegex('/Draft/');
         assign_user_complete($this->course, $this->students[0], $assign->get_course_module(), $assign->get_instance());
 
diff --git a/mod/assign/tests/locallib_test.php b/mod/assign/tests/locallib_test.php
index 6f533b8c9e1..ae2164c2cf4 100644
--- a/mod/assign/tests/locallib_test.php
+++ b/mod/assign/tests/locallib_test.php
@@ -674,7 +674,7 @@ class mod_assign_locallib_testcase extends advanced_testcase {
         $data->grade = '50.0';
         $assign->testable_apply_grade_to_user($data, $this->students[0]->id);
 
-        // Now we should see the feedback
+        // Now we should see the feedback.
         $this->setUser($this->students[0]);
         $output = $assign->view_student_summary($this->students[0], true);
         $this->assertNotEquals(false, strpos($output, 'Feedback'), 'Show feedback if there is a grade');
@@ -695,7 +695,7 @@ class mod_assign_locallib_testcase extends advanced_testcase {
         $output = $assign->view_student_summary($this->students[0], true);
         $this->assertEquals(false, strpos($output, 'Feedback'), 'Do not show feedback if the grade is hidden in the gradebook');
 
-        // Do the same but add feedback
+        // Do the same but add feedback.
         $assign = $this->create_instance(array('assignfeedback_comments_enabled' => 1));
 
         $this->setUser($this->teachers[0]);
@@ -706,7 +706,7 @@ class mod_assign_locallib_testcase extends advanced_testcase {
         $plugin = $assign->get_feedback_plugin_by_type('comments');
         $plugin->save($grade, $data);
 
-        // Should have feedback but no grade
+        // Should have feedback but no grade.
         $this->setUser($this->students[0]);
         $output = $assign->view_student_summary($this->students[0], true);
         $this->assertNotEquals(false, strpos($output, 'Tomato sauce'), 'Show feedback even if there is no grade');